lightningd: attach plugins natively to the command which started it.

This will let us unify the startup and runtime-started infrastructure.

Note that there are two kinds of notifications:
1. Starting a single plugin (i.e. `plugin start`)
2. Starting multiple plugins (i.e. `plugin rescan` or `plugin startdir`).

In the latter case, we want the command to complete only once *all*
the plugins are dead/finished.

We also call plugin_kill() in all cases, and correctly return afterwards
(it matters once we use the same paths for dynamic plugins, which don't
cause a fatal error if they don't startup).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/options.c

@@ -343,7 +343,7 @@ static char *opt_add_proxy_addr(const char *arg, struct lightningd *ld)
 
 static char *opt_add_plugin(const char *arg, struct lightningd *ld)
 {
-	plugin_register(ld->plugins, arg);
+	plugin_register(ld->plugins, arg, NULL);
 	return NULL;
 }
 

lightningd/plugin.c

@@ -10,6 +10,7 @@
 #include <lightningd/notification.h>
 #include <lightningd/options.h>
 #include <lightningd/plugin.h>
+#include <lightningd/plugin_control.h>
 #include <lightningd/plugin_hook.h>
 #include <signal.h>
 #include <sys/stat.h>
@@ -50,6 +51,7 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book,
 	p->log = new_log(p, log_book, NULL, "plugin-manager");
 	p->ld = ld;
 	p->startup = true;
+	p->json_cmds = tal_arr(p, struct command *, 0);
 	uintmap_init(&p->pending_requests);
 	memleak_add_helper(p, memleak_help_pending_requests);
 
@@ -69,6 +71,36 @@ void plugins_free(struct plugins *plugins)
 	tal_free(plugins);
 }
 
+static void check_plugins_resolved(struct plugins *plugins)
+{
+	/* As startup, we break out once all getmanifest are returned */
+	if (plugins->startup) {
+		if (!plugins_any_in_state(plugins, AWAITING_GETMANIFEST_RESPONSE))
+			io_break(plugins);
+	/* Otherwise we wait until all finished. */
+	} else if (plugins_all_in_state(plugins, INIT_COMPLETE)) {
+		struct command **json_cmds;
+
+		/* Clear commands first, in case callbacks add new ones.
+		 * Paranoia, but wouldn't that be a nasty bug to find? */
+		json_cmds = plugins->json_cmds;
+		plugins->json_cmds = tal_arr(plugins, struct command *, 0);
+		for (size_t i = 0; i < tal_count(json_cmds); i++)
+			plugin_cmd_all_complete(plugins, json_cmds[i]);
+		tal_free(json_cmds);
+	}
+}
+
+struct command_result *plugin_register_all_complete(struct lightningd *ld,
+						    struct command *cmd)
+{
+	if (plugins_all_in_state(ld->plugins, INIT_COMPLETE))
+		return plugin_cmd_all_complete(ld->plugins, cmd);
+
+	tal_arr_expand(&ld->plugins->json_cmds, cmd);
+	return NULL;
+}
+
 static void destroy_plugin(struct plugin *p)
 {
 	struct plugin_rpccall *call;
@@ -83,7 +115,8 @@ static void destroy_plugin(struct plugin *p)
 	}
 }
 
-struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES)
+struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES,
+			       struct command *start_cmd)
 {
 	struct plugin *p, *p_temp;
 
@@ -99,6 +132,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES)
 	p = tal(plugins, struct plugin);
 	p->plugins = plugins;
 	p->cmd = tal_strdup(p, path);
+	p->start_cmd = start_cmd;
 
 	p->plugin_state = UNCONFIGURED;
 	p->js_arr = tal_arr(p, struct json_stream *, 0);
@@ -166,6 +200,11 @@ void plugin_kill(struct plugin *plugin, char *fmt, ...)
 	io_wake(plugin);
 	kill(plugin->pid, SIGKILL);
 	list_del(&plugin->list);
+
+	if (plugin->start_cmd)
+		plugin_cmd_killed(plugin->start_cmd, plugin, msg);
+
+	check_plugins_resolved(plugin->plugins);
 }
 
 /**
@@ -456,7 +495,12 @@ static struct io_plan *plugin_write_json(struct io_conn *conn,
 static void plugin_conn_finish(struct io_conn *conn, struct plugin *plugin)
 {
 	plugin->stdout_conn = NULL;
+	if (plugin->start_cmd) {
+		plugin_cmd_succeeded(plugin->start_cmd, plugin);
+		plugin->start_cmd = NULL;
+	}
 	tal_free(plugin);
+	check_plugins_resolved(plugin->plugins);
 }
 
 struct io_plan *plugin_stdin_conn_init(struct io_conn *conn,
@@ -892,7 +936,11 @@ static bool plugin_hooks_add(struct plugin *plugin, const char *buffer,
 
 static void plugin_manifest_timeout(struct plugin *plugin)
 {
-	log_broken(plugin->log, "The plugin failed to respond to \"getmanifest\" in time, terminating.");
+	plugin_kill(plugin,
+		    "failed to respond to \"%s\" in time, terminating.",
+		    plugin->plugin_state == AWAITING_GETMANIFEST_RESPONSE
+		    ? "getmanifest" : "init");
+	if (plugin->plugins->startup)
 		fatal("Can't recover from plugin failure, terminating.");
 }
 
@@ -908,10 +956,12 @@ bool plugin_parse_getmanifest_response(const char *buffer,
 		return false;
 
 	dynamictok = json_get_member(buffer, resulttok, "dynamic");
-	if (dynamictok && !json_to_bool(buffer, dynamictok, &plugin->dynamic))
+	if (dynamictok && !json_to_bool(buffer, dynamictok, &plugin->dynamic)) {
 		plugin_kill(plugin, "Bad 'dynamic' field ('%.*s')",
 			    json_tok_full_len(dynamictok),
 			    json_tok_full(buffer, dynamictok));
+		return false;
+	}
 
 	featurestok = json_get_member(buffer, resulttok, "featurebits");
 
@@ -995,6 +1045,9 @@ bool plugins_all_in_state(const struct plugins *plugins, enum plugin_state state
 	return true;
 }
 
+/* FIXME: Forward declaration to reduce patch noise */
+static void plugin_config(struct plugin *plugin);
+
 /**
  * Callback for the plugin_manifest request.
  */
@@ -1003,16 +1056,25 @@ static void plugin_manifest_cb(const char *buffer,
 			       const jsmntok_t *idtok,
 			       struct plugin *plugin)
 {
-	if (!plugin_parse_getmanifest_response(buffer, toks, idtok, plugin))
+	if (!plugin_parse_getmanifest_response(buffer, toks, idtok, plugin)) {
 		plugin_kill(plugin, "%s: Bad response to getmanifest.", plugin->cmd);
+		return;
+	}
 
-	/* Reset timer, it'd kill us otherwise. */
-	tal_free(plugin->timeout_timer);
+	/* At startup, we want to io_break once all getmanifests are done */
+	check_plugins_resolved(plugin->plugins);
 
-	/* Check if all plugins have replied to getmanifest, and break
-	 * if they have */
-	if (!plugins_any_in_state(plugin->plugins, AWAITING_GETMANIFEST_RESPONSE))
-		io_break(plugin->plugins);
+	if (plugin->plugins->startup) {
+		/* Reset timer, it'd kill us otherwise. */
+		plugin->timeout_timer = tal_free(plugin->timeout_timer);
+	} else {
+		/* Note: here 60 second timer continues through init */
+		/* After startup, automatically call init after getmanifest */
+		if (!plugin->dynamic)
+			plugin_kill(plugin, "Not a dynamic plugin");
+		else
+			plugin_config(plugin);
+	}
 }
 
 /* If this is a valid plugin return full path name, otherwise NULL */
@@ -1089,7 +1151,7 @@ char *add_plugin_dir(struct plugins *plugins, const char *dir, bool error_ok)
 			continue;
 		fullpath = plugin_fullpath(tmpctx, dir, di->d_name);
 		if (fullpath) {
-			p = plugin_register(plugins, fullpath);
+			p = plugin_register(plugins, fullpath, NULL);
 			if (!p && !error_ok)
 				return tal_fmt(NULL, "Failed to register %s: %s",
 				               fullpath, strerror(errno));
@@ -1189,6 +1251,12 @@ static void plugin_config_cb(const char *buffer,
 			     struct plugin *plugin)
 {
 	plugin->plugin_state = INIT_COMPLETE;
+	plugin->timeout_timer = tal_free(plugin->timeout_timer);
+	if (plugin->start_cmd) {
+		plugin_cmd_succeeded(plugin->start_cmd, plugin);
+		plugin->start_cmd = NULL;
+	}
+	check_plugins_resolved(plugin->plugins);
 }
 
 void

lightningd/plugin.h

@@ -46,6 +46,8 @@ struct plugin {
 	bool stop;
 	struct plugins *plugins;
 	const char **plugin_path;
+	/* If there's a json command which ordered this to start */
+	struct command *start_cmd;
 
 	enum plugin_state plugin_state;
 
@@ -96,6 +98,9 @@ struct plugins {
 
 	struct lightningd *ld;
 	const char *default_dir;
+
+	/* If there are json commands waiting for plugin resolutions. */
+	struct command **json_cmds;
 };
 
 /* The value of a plugin option, which can have different types.
@@ -169,8 +174,14 @@ void plugins_free(struct plugins *plugins);
  *
  * @param plugins: Plugin context
  * @param path: The path of the executable for this plugin
+ * @param start_cmd: The optional JSON command which caused this.
+ *
+ * If @start_cmd, then plugin_cmd_killed or plugin_cmd_succeeded will be called
+ * on it eventually.
  */
-struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES);
+struct plugin *plugin_register(struct plugins *plugins,
+			       const char* path TAKES,
+			       struct command *start_cmd);
 
 /**
  * Returns true if the provided name matches a plugin command
@@ -197,6 +208,15 @@ struct plugin *find_plugin_for_command(struct lightningd *ld,
 				       const char *cmd_name);
 
 
+/**
+ * Call plugin_cmd_all_complete once all plugins are init or killed.
+ *
+ * Returns NULL if it's still pending. otherwise, returns
+ * plugin_cmd_all_complete().
+ */
+struct command_result *plugin_register_all_complete(struct lightningd *ld,
+						    struct command *cmd);
+
 /**
  * Send the configure message to all plugins.
  *

lightningd/plugin_control.c

@@ -14,14 +14,15 @@ struct dynamic_plugin {
 /**
  * Returned by all subcommands on success.
  */
-static struct command_result *plugin_dynamic_list_plugins(struct command *cmd)
+static struct command_result *plugin_dynamic_list_plugins(struct command *cmd,
+							  const struct plugins *plugins)
 {
 	struct json_stream *response;
-	struct plugin *p;
+	const struct plugin *p;
 
 	response = json_stream_success(cmd);
 	json_array_start(response, "plugins");
-	list_for_each(&cmd->ld->plugins->plugins, p, list) {
+	list_for_each(&plugins->plugins, p, list) {
 		json_object_start(response, NULL);
 		json_add_string(response, "name", p->cmd);
 		json_add_bool(response, "active",
@@ -52,6 +53,24 @@ plugin_dynamic_error(struct dynamic_plugin *dp, const char *error)
 	                    error);
 }
 
+struct command_result *plugin_cmd_killed(struct command *cmd,
+					 struct plugin *plugin, const char *msg)
+{
+	return command_fail(cmd, PLUGIN_ERROR, "%s: %s", plugin->cmd, msg);
+}
+
+struct command_result *plugin_cmd_succeeded(struct command *cmd,
+					    struct plugin *plugin)
+{
+	return plugin_dynamic_list_plugins(cmd, plugin->plugins);
+}
+
+struct command_result *plugin_cmd_all_complete(struct plugins *plugins,
+					       struct command *cmd)
+{
+	return plugin_dynamic_list_plugins(cmd, plugins);
+}
+
 static void plugin_dynamic_timeout(struct dynamic_plugin *dp)
 {
 	plugin_dynamic_error(dp, "Timed out while waiting for plugin response");
@@ -81,7 +100,7 @@ static void plugin_dynamic_config_callback(const char *buffer,
 	}
 
 	/* No plugin unconfigured left, return the plugin list */
-	was_pending(plugin_dynamic_list_plugins(dp->cmd));
+	was_pending(plugin_dynamic_list_plugins(dp->cmd, dp->plugin->plugins));
 }
 
 /**
@@ -184,7 +203,7 @@ plugin_dynamic_start(struct command *cmd, const char *plugin_path)
 
 	dp = tal(cmd, struct dynamic_plugin);
 	dp->cmd = cmd;
-	dp->plugin = plugin_register(cmd->ld->plugins, plugin_path);
+	dp->plugin = plugin_register(cmd->ld->plugins, plugin_path, NULL);
 	if (!dp->plugin)
 		return plugin_dynamic_error(dp, "Is already registered");
 
@@ -218,7 +237,7 @@ plugin_dynamic_startdir(struct command *cmd, const char *dir_path)
 		}
 	}
 	if (!found)
-		plugin_dynamic_list_plugins(cmd);
+		plugin_dynamic_list_plugins(cmd, cmd->ld->plugins);
 
 	return command_still_pending(cmd);
 }
@@ -289,7 +308,7 @@ plugin_dynamic_rescan_plugins(struct command *cmd)
 	}
 
 	if (!found)
-		return plugin_dynamic_list_plugins(cmd);
+		return plugin_dynamic_list_plugins(cmd, cmd->ld->plugins);
 	return command_still_pending(cmd);
 }
 
@@ -361,7 +380,7 @@ static struct command_result *json_plugin_control(struct command *cmd,
 			   NULL))
 			return command_param_failed();
 
-		return plugin_dynamic_list_plugins(cmd);
+		return plugin_dynamic_list_plugins(cmd, cmd->ld->plugins);
 	}
 
 	/* subcmd must be one of the above: param_subcommand checked it! */

lightningd/plugin_control.h

@@ -3,5 +3,16 @@
 #include "config.h"
 #include <lightningd/plugin.h>
 
+/* Plugin startup failed */
+struct command_result *plugin_cmd_killed(struct command *cmd,
+					 struct plugin *plugin, const char *msg);
+
+/* Plugin startup succeeded */
+struct command_result *plugin_cmd_succeeded(struct command *cmd,
+					    struct plugin *plugin);
+
+/* All plugins succeeded/failed */
+struct command_result *plugin_cmd_all_complete(struct plugins *plugins,
+					       struct command *cmd);
 
 #endif /* LIGHTNING_LIGHTNINGD_PLUGIN_CONTROL_H */