From f6e572c7aa7391b9189d8305cbcc79ebbe97eb9a Mon Sep 17 00:00:00 2001 From: darosior Date: Sun, 15 Sep 2019 23:45:33 +0200 Subject: [PATCH 01/14] plugins: return plugin in plugin_register, make conn initializers publics --- lightningd/plugin.c | 13 +++++++------ lightningd/plugin.h | 10 +++++++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 036fc57a8899..1c075b2af43c 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -61,7 +61,7 @@ static void destroy_plugin(struct plugin *p) list_del(&p->list); } -void plugin_register(struct plugins *plugins, const char* path TAKES) +struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES) { struct plugin *p, *p_temp; @@ -70,7 +70,7 @@ void plugin_register(struct plugins *plugins, const char* path TAKES) if (streq(path, p_temp->cmd)) { if (taken(path)) tal_free(path); - return; + return NULL; } } @@ -107,6 +107,7 @@ void plugin_register(struct plugins *plugins, const char* path TAKES) list_add_tail(&plugins->plugins, &p->list); tal_add_destructor(p, destroy_plugin); + return p; } bool plugin_paths_match(const char *cmd, const char *name) @@ -435,8 +436,8 @@ static void plugin_conn_finish(struct io_conn *conn, struct plugin *plugin) tal_free(plugin); } -static struct io_plan *plugin_stdin_conn_init(struct io_conn *conn, - struct plugin *plugin) +struct io_plan *plugin_stdin_conn_init(struct io_conn *conn, + struct plugin *plugin) { /* We write to their stdin */ /* We don't have anything queued yet, wait for notification */ @@ -445,8 +446,8 @@ static struct io_plan *plugin_stdin_conn_init(struct io_conn *conn, return io_wait(plugin->stdin_conn, plugin, plugin_write_json, plugin); } -static struct io_plan *plugin_stdout_conn_init(struct io_conn *conn, - struct plugin *plugin) +struct io_plan *plugin_stdout_conn_init(struct io_conn *conn, + struct plugin *plugin) { /* We read from their stdout */ plugin->stdout_conn = conn; diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 8787b62379f5..11c499acc708 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -130,7 +130,7 @@ void plugins_init(struct plugins *plugins, const char *dev_plugin_debug); * @param plugins: Plugin context * @param path: The path of the executable for this plugin */ -void plugin_register(struct plugins *plugins, const char* path TAKES); +struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES); /** * Returns true if the provided name matches a plugin command @@ -203,6 +203,14 @@ void plugin_request_send(struct plugin *plugin, */ char *plugin_opt_set(const char *arg, struct plugin_opt *popt); +/** + * Helpers to initialize a connection to a plugin; we read from their + * stdout, and write to their stdin. + */ +struct io_plan *plugin_stdin_conn_init(struct io_conn *conn, + struct plugin *plugin); +struct io_plan *plugin_stdout_conn_init(struct io_conn *conn, + struct plugin *plugin); /** * Needed for I/O logging for plugin messages. From 755e34d23d1f9130367db31fbac862384d7801c1 Mon Sep 17 00:00:00 2001 From: darosior Date: Tue, 10 Sep 2019 17:46:08 +0200 Subject: [PATCH 02/14] plugins: return also on register error in add_plugin_dir() --- lightningd/plugin.c | 16 +++++++++++----- lightningd/plugin.h | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 1c075b2af43c..9783fba41909 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -901,10 +901,12 @@ static const char *plugin_fullpath(const tal_t *ctx, const char *dir, return fullname; } -char *add_plugin_dir(struct plugins *plugins, const char *dir, bool nonexist_ok) +char *add_plugin_dir(struct plugins *plugins, const char *dir, bool error_ok) { struct dirent *di; DIR *d = opendir(dir); + struct plugin *p; + if (!d) { if (deprecated_apis && !path_is_abs(dir)) { dir = path_join(tmpctx, @@ -919,7 +921,7 @@ char *add_plugin_dir(struct plugins *plugins, const char *dir, bool nonexist_ok) } } if (!d) { - if (!nonexist_ok && errno == ENOENT) + if (!error_ok && errno == ENOENT) return NULL; return tal_fmt(NULL, "Failed to open plugin-dir %s: %s", dir, strerror(errno)); @@ -931,9 +933,13 @@ char *add_plugin_dir(struct plugins *plugins, const char *dir, bool nonexist_ok) if (streq(di->d_name, ".") || streq(di->d_name, "..")) continue; - fullpath = plugin_fullpath(NULL, dir, di->d_name); - if (fullpath) - plugin_register(plugins, take(fullpath)); + fullpath = plugin_fullpath(tmpctx, dir, di->d_name); + if (fullpath) { + p = plugin_register(plugins, fullpath); + if (!p && !error_ok) + return tal_fmt(NULL, "Failed to register %s: %s", + fullpath, strerror(errno)); + } } closedir(d); return NULL; diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 11c499acc708..f615026308bf 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -181,7 +181,7 @@ void *plugin_exclusive_loop(struct plugin *plugin); * Add a directory to the plugin path to automatically load plugins. */ char *add_plugin_dir(struct plugins *plugins, const char *dir, - bool nonexist_ok); + bool error_ok); /** * Clear all plugins registered so far. From c00e7ddc4a88fd35ed80a4d8f08dd6f2d6b52c43 Mon Sep 17 00:00:00 2001 From: darosior Date: Sun, 15 Sep 2019 15:34:09 +0200 Subject: [PATCH 03/14] plugins: split manifest_cb and plugin_config This reduces error details for getmanifest response, but avoids too much code duplication in the next commit. --- lightningd/plugin.c | 74 ++++++++++++++++++++++++++------------------- lightningd/plugin.h | 14 +++++++++ 2 files changed, 57 insertions(+), 31 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 9783fba41909..d2d58722493b 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -814,31 +814,18 @@ static void plugin_manifest_timeout(struct plugin *plugin) fatal("Can't recover from plugin failure, terminating."); } -/** - * Callback for the plugin_manifest request. - */ -static void plugin_manifest_cb(const char *buffer, - const jsmntok_t *toks, - const jsmntok_t *idtok, - struct plugin *plugin) + +bool plugin_parse_getmanifest_response(const char *buffer, + const jsmntok_t *toks, + const jsmntok_t *idtok, + struct plugin *plugin) { const jsmntok_t *resulttok, *dynamictok; bool dynamic_plugin; - /* Check if all plugins have replied to getmanifest, and break - * if they have and this is the startup init */ - plugin->plugins->pending_manifests--; - if (plugin->plugins->startup && plugin->plugins->pending_manifests == 0) - io_break(plugin->plugins); - resulttok = json_get_member(buffer, toks, "result"); - if (!resulttok || resulttok->type != JSMN_OBJECT) { - plugin_kill(plugin, - "\"getmanifest\" result is not an object: %.*s", - toks[0].end - toks[0].start, - buffer + toks[0].start); - return; - } + if (!resulttok || resulttok->type != JSMN_OBJECT) + return false; dynamictok = json_get_member(buffer, resulttok, "dynamic"); if (dynamictok && json_to_bool(buffer, dynamictok, &dynamic_plugin)) @@ -848,9 +835,27 @@ static void plugin_manifest_cb(const char *buffer, !plugin_rpcmethods_add(plugin, buffer, resulttok) || !plugin_subscriptions_add(plugin, buffer, resulttok) || !plugin_hooks_add(plugin, buffer, resulttok)) - plugin_kill( - plugin, - "Failed to register options, methods, hooks, or subscriptions."); + return false; + + return true; +} + +/** + * Callback for the plugin_manifest request. + */ +static void plugin_manifest_cb(const char *buffer, + const jsmntok_t *toks, + const jsmntok_t *idtok, + struct plugin *plugin) +{ + /* Check if all plugins have replied to getmanifest, and break + * if they have and this is the startup init */ + plugin->plugins->pending_manifests--; + if (plugin->plugins->startup && plugin->plugins->pending_manifests == 0) + io_break(plugin->plugins); + + if (!plugin_parse_getmanifest_response(buffer, toks, idtok, plugin)) + plugin_kill(plugin, "%s: Bad response to getmanifest.", plugin->cmd); /* If all plugins have replied to getmanifest and this is not * the startup init, configure them */ @@ -1045,17 +1050,12 @@ static void plugin_config_cb(const char *buffer, plugin->plugin_state = CONFIGURED; } -/* FIXME(cdecker) This just builds a string for the request because - * the json_stream is tightly bound to the command interface. It - * should probably be generalized and fixed up. */ -static void plugin_config(struct plugin *plugin) +void +plugin_populate_init_request(struct plugin *plugin, struct jsonrpc_request *req) { - struct plugin_opt *opt; const char *name; - struct jsonrpc_request *req; + struct plugin_opt *opt; struct lightningd *ld = plugin->plugins->ld; - req = jsonrpc_request_start(plugin, "init", plugin->log, - plugin_config_cb, plugin); /* Add .params.options */ json_object_start(req->stream, "options"); @@ -1074,7 +1074,19 @@ static void plugin_config(struct plugin *plugin) json_add_string(req->stream, "rpc-file", ld->rpc_filename); json_add_bool(req->stream, "startup", plugin->plugins->startup); json_object_end(req->stream); +} +/* FIXME(cdecker) This just builds a string for the request because + * the json_stream is tightly bound to the command interface. It + * should probably be generalized and fixed up. */ +static void +plugin_config(struct plugin *plugin) +{ + struct jsonrpc_request *req; + + req = jsonrpc_request_start(plugin, "init", plugin->log, + plugin_config_cb, plugin); + plugin_populate_init_request(plugin, req); jsonrpc_request_end(req); plugin_request_send(plugin, req); } diff --git a/lightningd/plugin.h b/lightningd/plugin.h index f615026308bf..78a59b704c54 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -159,6 +159,20 @@ void PRINTF_FMT(2,3) plugin_kill(struct plugin *plugin, char *fmt, ...); * incoming JSON-RPC calls and messages. */ void plugins_config(struct plugins *plugins); + +/** + * Read and treat (populate options, methods, ...) the `getmanifest` response. + */ +bool plugin_parse_getmanifest_response(const char *buffer, + const jsmntok_t *toks, + const jsmntok_t *idtok, + struct plugin *plugin); + +/** + * This populates the jsonrpc request with the plugin/lightningd specifications + */ +void plugin_populate_init_request(struct plugin *p, struct jsonrpc_request *req); + /** * Add the plugin option and their respective options to listconfigs. * From db07b93d633729ec81851e8814c4f0a88a375bc8 Mon Sep 17 00:00:00 2001 From: darosior Date: Thu, 12 Sep 2019 01:26:51 +0200 Subject: [PATCH 04/14] plugins: make 'plugin start' 's runtime path independant This remove the reliance on startup plugins' function "plugin_start" in order to use a distinct runtime path for a dynamically started plugin, which will allow startup plugins' code to be (almost) agnostic of dynamic plugins. This also makes the 'start' subcommand return only if the plugin is either started, or killed : no weird middle state where the plugin mishbehaving could crash lightningd. --- lightningd/plugin_control.c | 161 ++++++++++++++++++++++++++++++++++-- 1 file changed, 156 insertions(+), 5 deletions(-) diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index ff5d49e78167..3abde0abc569 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -1,13 +1,168 @@ +#include #include #include #include #include +#include #include #include +#include #include #include #include +/* A dummy structure used to give multiple arguments to callbacks. */ +struct dynamic_plugin { + struct plugin *plugin; + struct command *cmd; +}; + +/** + * Returned by all subcommands on success. + */ +static struct command_result *plugin_dynamic_list_plugins(struct command *cmd) +{ + struct json_stream *response; + struct plugin *p; + + response = json_stream_success(cmd); + json_array_start(response, "plugins"); + list_for_each(&cmd->ld->plugins->plugins, p, list) { + json_object_start(response, NULL); + json_add_string(response, "name", p->cmd); + json_add_bool(response, "active", + p->plugin_state == CONFIGURED); + json_object_end(response); + } + json_array_end(response); + return command_success(cmd, response); +} + +/** + * Returned by all subcommands on error. + */ +static struct command_result * +plugin_dynamic_error(struct dynamic_plugin *dp, const char *error) +{ + plugin_kill(dp->plugin, "%s: %s", dp->plugin->cmd, error); + return command_fail(dp->cmd, JSONRPC2_INVALID_PARAMS, + "%s: %s", dp->plugin->cmd, error); +} + +static void plugin_dynamic_timeout(struct dynamic_plugin *dp) +{ + plugin_dynamic_error(dp, "Timed out while waiting for plugin response"); +} + +static void plugin_dynamic_config_callback(const char *buffer, + const jsmntok_t *toks, + const jsmntok_t *idtok, + struct dynamic_plugin *dp) +{ + struct plugin *p; + + dp->plugin->plugin_state = CONFIGURED; + /* Reset the timer only now so that we are either configured, or + * killed. */ + tal_free(dp->plugin->timeout_timer); + + list_for_each(&dp->plugin->plugins->plugins, p, list) { + if (p->plugin_state != CONFIGURED) + return; + } + + /* No plugin unconfigured left, return the plugin list */ + was_pending(plugin_dynamic_list_plugins(dp->cmd)); +} + +/** + * Send the init message to the plugin. We don't care about its response, + * but it's considered the last part of the handshake : once it responds + * it is considered configured. + */ +static void plugin_dynamic_config(struct dynamic_plugin *dp) +{ + struct jsonrpc_request *req; + + req = jsonrpc_request_start(dp->plugin, "init", dp->plugin->log, + plugin_dynamic_config_callback, dp); + plugin_populate_init_request(dp->plugin, req); + jsonrpc_request_end(req); + plugin_request_send(dp->plugin, req); +} + +static void plugin_dynamic_manifest_callback(const char *buffer, + const jsmntok_t *toks, + const jsmntok_t *idtok, + struct dynamic_plugin *dp) +{ + if (!plugin_parse_getmanifest_response(buffer, toks, idtok, dp->plugin)) + return was_pending(plugin_dynamic_error(dp, "Gave a bad response to getmanifest")); + + if (!dp->plugin->dynamic) + return was_pending(plugin_dynamic_error(dp, "Not a dynamic plugin")); + + /* We got the manifest, now send the init message */ + plugin_dynamic_config(dp); +} + +/** + * This starts a plugin : spawns the process, connect its stdout and stdin, + * then sends it a getmanifest request. + */ +static struct command_result *plugin_start(struct dynamic_plugin *dp) +{ + int stdin, stdout; + char **p_cmd; + struct jsonrpc_request *req; + struct plugin *p = dp->plugin; + + p->dynamic = true; + p_cmd = tal_arrz(NULL, char *, 2); + p_cmd[0] = p->cmd; + p->pid = pipecmdarr(&stdin, &stdout, &pipecmd_preserve, p_cmd); + if (p->pid == -1) + return plugin_dynamic_error(dp, "Error running command"); + else + log_debug(dp->cmd->ld->plugins->log, "started(%u) %s", p->pid, p->cmd); + tal_free(p_cmd); + p->buffer = tal_arr(p, char, 64); + p->stop = false; + /* Give the plugin 20 seconds to respond to `getmanifest`, so we don't hang + * too long on the RPC caller. */ + p->timeout_timer = new_reltimer(dp->cmd->ld->timers, dp, + time_from_sec(20), + plugin_dynamic_timeout, dp); + + /* Create two connections, one read-only on top of the plugin's stdin, and one + * write-only on its stdout. */ + io_new_conn(p, stdout, plugin_stdout_conn_init, p); + io_new_conn(p, stdin, plugin_stdin_conn_init, p); + req = jsonrpc_request_start(p, "getmanifest", p->log, + plugin_dynamic_manifest_callback, dp); + jsonrpc_request_end(req); + plugin_request_send(p, req); + return command_still_pending(dp->cmd); +} + +/** + * Called when trying to start a plugin through RPC, it starts the plugin and + * will give a result 20 seconds later at the most. + */ +static struct command_result * +plugin_dynamic_start(struct command *cmd, const char *plugin_path) +{ + struct dynamic_plugin *dp; + + dp = tal(cmd, struct dynamic_plugin); + dp->cmd = cmd; + dp->plugin = plugin_register(cmd->ld->plugins, plugin_path); + if (!dp->plugin) + return plugin_dynamic_error(dp, "Is already registered"); + + return plugin_start(dp); +} + /** * A plugin command which permits to control plugins without restarting * lightningd. It takes a subcommand, and an optional subcommand parameter. @@ -63,7 +218,7 @@ static struct command_result *json_plugin_control(struct command *cmd, return command_param_failed(); if (access(plugin_path, X_OK) == 0) - plugin_register(cmd->ld->plugins, plugin_path); + return plugin_dynamic_start(cmd, plugin_path); else return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "%s is not executable: %s", @@ -98,10 +253,6 @@ static struct command_result *json_plugin_control(struct command *cmd, /* Don't do anything as we return the plugin list anyway */ } - /* The config function is called once we got the manifest, - * in 'plugin_manifest_cb'.*/ - plugins_start(cmd->ld->plugins, cmd->ld->dev_debug_subprocess); - response = json_stream_success(cmd); json_array_start(response, "plugins"); list_for_each(&cmd->ld->plugins->plugins, p, list) { From 3e01e48c7c2f9e9d6e06d36f41449d3d57d0818c Mon Sep 17 00:00:00 2001 From: darosior Date: Sat, 14 Sep 2019 17:35:00 +0200 Subject: [PATCH 05/14] plugins: make 'plugin startdir' 's runtime path independant This does the same as for the 'start' subcommand for the 'startdir' one. Note that we could fail to start the last plugin of a directory, but have succesfully started the precedent plugins. This will make us return an error to the user while some of the plugins have been started, but we still don't end up in a transient state with half-configured-half-errored plugins. --- lightningd/plugin_control.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 3abde0abc569..aa3e0f789059 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -163,6 +163,38 @@ plugin_dynamic_start(struct command *cmd, const char *plugin_path) return plugin_start(dp); } +/** + * Called when trying to start a plugin directory through RPC, it registers + * all contained plugins recursively and then starts them. + */ +static struct command_result * +plugin_dynamic_startdir(struct command *cmd, const char *dir_path) +{ + const char *err; + struct plugin *p; + /* If the directory is empty */ + bool found; + + err = add_plugin_dir(cmd->ld->plugins, dir_path, false); + if (err) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "%s", err); + + found = false; + list_for_each(&cmd->ld->plugins->plugins, p, list) { + if (p->plugin_state == UNCONFIGURED) { + found = true; + struct dynamic_plugin *dp = tal(cmd, struct dynamic_plugin); + dp->plugin = p; + dp->cmd = cmd; + plugin_start(dp); + } + } + if (!found) + plugin_dynamic_list_plugins(cmd); + + return command_still_pending(cmd); +} + /** * A plugin command which permits to control plugins without restarting * lightningd. It takes a subcommand, and an optional subcommand parameter. @@ -233,7 +265,7 @@ static struct command_result *json_plugin_control(struct command *cmd, return command_param_failed(); if (access(dir_path, F_OK) == 0) - add_plugin_dir(cmd->ld->plugins, dir_path, true); + return plugin_dynamic_startdir(cmd, dir_path); else return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Could not open %s", dir_path); From 15e0a260eed8db2032f6d54c9197b64cb4c0ec38 Mon Sep 17 00:00:00 2001 From: darosior Date: Sun, 15 Sep 2019 18:46:01 +0200 Subject: [PATCH 06/14] plugins: make 'plugin stop' a function To keep only subcommands logic into the main function. --- lightningd/plugin_control.c | 47 +++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index aa3e0f789059..9b0a50503d54 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -195,6 +195,34 @@ plugin_dynamic_startdir(struct command *cmd, const char *dir_path) return command_still_pending(cmd); } +static struct command_result * +plugin_dynamic_stop(struct command *cmd, const char *plugin_name) +{ + struct plugin *p; + struct json_stream *response; + + list_for_each(&cmd->ld->plugins->plugins, p, list) { + if (plugin_paths_match(p->cmd, plugin_name)) { + if (!p->dynamic) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "%s cannot be managed when " + "lightningd is up", + plugin_name); + plugin_hook_unregister_all(p); + plugin_kill(p, "%s stopped by lightningd via RPC", plugin_name); + tal_free(p); + response = json_stream_success(cmd); + json_add_string(response, "", + take(tal_fmt(NULL, "Successfully stopped %s.", + plugin_name))); + return command_success(cmd, response); + } + } + + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Could not find plugin %s", plugin_name); +} + /** * A plugin command which permits to control plugins without restarting * lightningd. It takes a subcommand, and an optional subcommand parameter. @@ -215,7 +243,6 @@ static struct command_result *json_plugin_control(struct command *cmd, if (streq(subcmd, "stop")) { const char *plugin_name; - bool plugin_found; if (!param(cmd, buffer, params, p_req("subcommand", param_ignore, cmd), @@ -223,23 +250,7 @@ static struct command_result *json_plugin_control(struct command *cmd, NULL)) return command_param_failed(); - plugin_found = false; - list_for_each(&cmd->ld->plugins->plugins, p, list) { - if (plugin_paths_match(p->cmd, plugin_name)) { - if (!p->dynamic) - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "%s plugin cannot be managed when lightningd is up", - plugin_name); - plugin_found = true; - plugin_hook_unregister_all(p); - plugin_kill(p, "%s stopped by lightningd via RPC", - plugin_name); - break; - } - } - if (!plugin_found) - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "Could not find plugin %s", plugin_name); + return plugin_dynamic_stop(cmd, plugin_name); } else if (streq(subcmd, "start")) { const char *plugin_path; From dbf936012c3ebd0dbd316517eba2bec311f8b0b4 Mon Sep 17 00:00:00 2001 From: darosior Date: Sun, 15 Sep 2019 18:51:41 +0200 Subject: [PATCH 07/14] plugins: make use of dynamic_plugin_list() in 'plugin list' And make a function for the 'rescan' subcommand so that we are consistent and have only subcommand logic in the main function, which return command_still_pending() in any case (but 'stop'). patched-by: @rustyrussell --- lightningd/plugin_control.c | 53 ++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 9b0a50503d54..dc06c01ccca2 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -223,6 +223,35 @@ plugin_dynamic_stop(struct command *cmd, const char *plugin_name) "Could not find plugin %s", plugin_name); } +/** + * Look for additions in the default plugin directory. + */ +static struct command_result * +plugin_dynamic_rescan_plugins(struct command *cmd) +{ + bool found; + struct plugin *p; + + /* This will not fail on "already registered" error. */ + plugins_add_default_dir(cmd->ld->plugins, + path_join(tmpctx, cmd->ld->config_dir, "plugins")); + + found = false; + list_for_each(&cmd->ld->plugins->plugins, p, list) { + if (p->plugin_state == UNCONFIGURED) { + struct dynamic_plugin *dp = tal(cmd, struct dynamic_plugin); + dp->plugin = p; + dp->cmd = cmd; + plugin_start(dp); + found = true; + } + } + + if (!found) + return plugin_dynamic_list_plugins(cmd); + return command_still_pending(cmd); +} + /** * A plugin command which permits to control plugins without restarting * lightningd. It takes a subcommand, and an optional subcommand parameter. @@ -234,13 +263,11 @@ static struct command_result *json_plugin_control(struct command *cmd, { const char *subcmd; subcmd = param_subcommand(cmd, buffer, params, - "start", "stop", "startdir", "rescan", "list", NULL); + "start", "stop", "startdir", + "rescan", "list", NULL); if (!subcmd) return command_param_failed(); - struct plugin *p; - struct json_stream *response; - if (streq(subcmd, "stop")) { const char *plugin_name; @@ -286,28 +313,18 @@ static struct command_result *json_plugin_control(struct command *cmd, NULL)) return command_param_failed(); - plugins_add_default_dir(cmd->ld->plugins, - path_join(tmpctx, cmd->ld->config_dir, "plugins")); + return plugin_dynamic_rescan_plugins(cmd); } else if (streq(subcmd, "list")) { if (!param(cmd, buffer, params, p_req("subcommand", param_ignore, cmd), NULL)) return command_param_failed(); - /* Don't do anything as we return the plugin list anyway */ - } - response = json_stream_success(cmd); - json_array_start(response, "plugins"); - list_for_each(&cmd->ld->plugins->plugins, p, list) { - json_object_start(response, NULL); - json_add_string(response, "name", p->cmd); - json_add_bool(response, "active", - p->plugin_state == CONFIGURED); - json_object_end(response); + return plugin_dynamic_list_plugins(cmd); } - json_array_end(response); - return command_success(cmd, response); + /* subcmd must be one of the above: param_subcommand checked it! */ + abort(); } static const struct json_command plugin_control_command = { From 5b7b3d1050173415be8881b0c220fd1c83d1bed5 Mon Sep 17 00:00:00 2001 From: darosior Date: Thu, 12 Sep 2019 01:17:57 +0200 Subject: [PATCH 08/14] plugins: remove dynamic plugins configuration code from lightningd/plugin This merges back plugins_init and plugins_start, removes conditions on startup, and removes the CONFIGURING state. --- lightningd/plugin.c | 38 ++++++++++++-------------------------- lightningd/plugin.h | 4 ---- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index d2d58722493b..01daa5850848 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -50,6 +50,7 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->log_book = log_book; p->log = new_log(p, log_book, "plugin-manager"); p->ld = ld; + p->startup = true; uintmap_init(&p->pending_requests); memleak_add_helper(p, memleak_help_pending_requests); @@ -849,19 +850,14 @@ static void plugin_manifest_cb(const char *buffer, struct plugin *plugin) { /* Check if all plugins have replied to getmanifest, and break - * if they have and this is the startup init */ + * if they have */ plugin->plugins->pending_manifests--; - if (plugin->plugins->startup && plugin->plugins->pending_manifests == 0) + if (plugin->plugins->pending_manifests == 0) io_break(plugin->plugins); if (!plugin_parse_getmanifest_response(buffer, toks, idtok, plugin)) plugin_kill(plugin, "%s: Bad response to getmanifest.", plugin->cmd); - /* If all plugins have replied to getmanifest and this is not - * the startup init, configure them */ - if (!plugin->plugins->startup && plugin->plugins->pending_manifests == 0) - plugins_config(plugin->plugins); - /* Reset timer, it'd kill us otherwise. */ tal_free(plugin->timeout_timer); } @@ -976,17 +972,21 @@ void plugins_add_default_dir(struct plugins *plugins, const char *default_dir) } } -void plugins_start(struct plugins *plugins, const char *dev_plugin_debug) +void plugins_init(struct plugins *plugins, const char *dev_plugin_debug) { struct plugin *p; char **cmd; int stdin, stdout; struct jsonrpc_request *req; - list_for_each(&plugins->plugins, p, list) { - if (p->plugin_state != UNCONFIGURED) - continue; + plugins->pending_manifests = 0; + plugins_add_default_dir(plugins, + path_join(tmpctx, plugins->ld->config_dir, "plugins")); + setenv("LIGHTNINGD_PLUGIN", "1", 1); + setenv("LIGHTNINGD_VERSION", version(), 1); + /* Spawn the plugin processes before entering the io_loop */ + list_for_each(&plugins->plugins, p, list) { bool debug; debug = dev_plugin_debug && strends(p->cmd, dev_plugin_debug); @@ -1025,18 +1025,6 @@ void plugins_start(struct plugins *plugins, const char *dev_plugin_debug) } tal_free(cmd); } -} - -void plugins_init(struct plugins *plugins, const char *dev_plugin_debug) -{ - plugins->pending_manifests = 0; - plugins_add_default_dir(plugins, - path_join(tmpctx, plugins->ld->config_dir, "plugins")); - - setenv("LIGHTNINGD_PLUGIN", "1", 1); - setenv("LIGHTNINGD_VERSION", version(), 1); - /* Spawn the plugin processes before entering the io_loop */ - plugins_start(plugins, dev_plugin_debug); if (plugins->pending_manifests > 0) io_loop_with_timers(plugins->ld); @@ -1095,10 +1083,8 @@ void plugins_config(struct plugins *plugins) { struct plugin *p; list_for_each(&plugins->plugins, p, list) { - if (p->plugin_state == UNCONFIGURED) { - p->plugin_state = CONFIGURING; + if (p->plugin_state == UNCONFIGURED) plugin_config(p); - } } plugins->startup = false; diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 78a59b704c54..828561c9df0d 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -10,7 +10,6 @@ enum plugin_state { UNCONFIGURED, - CONFIGURING, CONFIGURED }; @@ -31,7 +30,6 @@ struct plugin { /* If this plugin can be restarted without restarting lightningd */ bool dynamic; - bool signal_startup; /* Stuff we read */ char *buffer; @@ -109,8 +107,6 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, */ void plugins_add_default_dir(struct plugins *plugins, const char *default_dir); -void plugins_start(struct plugins *plugins, const char *dev_plugin_debug); - /** * Initialize the registered plugins. * From 85ead7b779f0ef06bda8af8d5408f373d0ad65cd Mon Sep 17 00:00:00 2001 From: darosior Date: Sun, 15 Sep 2019 15:17:46 +0200 Subject: [PATCH 09/14] contrib: Make cowsay answer 'init' __________________________________ < I was not plugin specs compliant > ---------------------------------- \ ^__^ \ (oo)\_______ (__)\ )\/\ ||----w | || || --- contrib/plugins/cowsay.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/plugins/cowsay.sh b/contrib/plugins/cowsay.sh index 71505fef600e..c995420392d0 100755 --- a/contrib/plugins/cowsay.sh +++ b/contrib/plugins/cowsay.sh @@ -24,6 +24,9 @@ echo '{"jsonrpc":"2.0","id":'"$id"',"result":{"dynamic":true,"options":[],"rpcme # Eg. {"jsonrpc":"2.0","id":5,"method":"init","params":{"options":{},"configuration":{"lightning-dir":"/home/rusty/.lightning","rpc-file":"lightning-rpc","startup":false}}}\n\n read -r JSON read -r _ +id=$(echo "$JSON" | sed 's/.*"id" *: *\([0-9]*\),.*/\1/') + +echo '{"jsonrpc":"2.0","id":'"$id"',"result":{}}' # eg. { "jsonrpc" : "2.0", "method" : "cowsay", "id" : 6, "params" :[ "hello"] } while read -r JSON; do From 359db2545eeff53cb107b95295c56b954c221dab Mon Sep 17 00:00:00 2001 From: darosior Date: Sun, 15 Sep 2019 15:19:28 +0200 Subject: [PATCH 10/14] plugins: refactor the tests for dynamic plugins This adapts the test to the new 'plugin' command: no more sleeping, since we are synchronous ! This tests the timeout by increasing the 'slowinit' plugin sleep duration at init reception. This adds a broken plugin to make sure we won't crash because of a misbehaving plugin (unmet dependency is the most common case). --- tests/plugins/broken.py | 19 +++++++++++++++++++ tests/plugins/slow_init.py | 2 +- tests/test_plugin.py | 34 +++++++++++++++++++--------------- 3 files changed, 39 insertions(+), 16 deletions(-) create mode 100755 tests/plugins/broken.py diff --git a/tests/plugins/broken.py b/tests/plugins/broken.py new file mode 100755 index 000000000000..28b871c88220 --- /dev/null +++ b/tests/plugins/broken.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python3 +"""Simple plugin to test that lightningd doesnt crash if it starts a +misbehaving plugin via RPC. +""" + +from lightning import Plugin +import an_unexistent_module_that_will_make_me_crash + +plugin = Plugin(dynamic=False) + + +@plugin.init() +def init(options, configuration, plugin): + plugin.log("broken.py initializing {}".format(configuration)) + # We need to actually use the import to pass source checks.. + an_unexistent_module_that_will_make_me_crash.hello() + + +plugin.run() diff --git a/tests/plugins/slow_init.py b/tests/plugins/slow_init.py index 4be95ced84da..91945d7dfe29 100755 --- a/tests/plugins/slow_init.py +++ b/tests/plugins/slow_init.py @@ -8,7 +8,7 @@ @plugin.init() def init(options, configuration, plugin): plugin.log("slow_init.py initializing {}".format(configuration)) - time.sleep(1) + time.sleep(2) plugin.run() diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 1e253ef911b9..a032fa6c9bc4 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -94,11 +94,10 @@ def test_plugin_dir(node_factory): def test_plugin_slowinit(node_factory): - """Tests the 'plugin' RPC command when init is slow""" + """Tests that the 'plugin' RPC command times out if plugin doesnt respond""" n = node_factory.get_node() n.rpc.plugin_start(os.path.join(os.getcwd(), "tests/plugins/slow_init.py")) - n.daemon.wait_for_log("slow_init.py initializing.* 'startup': False") # It's not actually configured yet, see what happens; # make sure 'rescan' and 'list' controls dont crash @@ -116,42 +115,47 @@ def test_plugin_command(node_factory): assert(len(cmd) == 0) # Add the 'contrib/plugins' test dir - time.sleep(2) n.rpc.plugin_startdir(directory=os.path.join(os.getcwd(), "contrib/plugins")) - n.daemon.wait_for_log(r"Plugin helloworld.py initialized") # Make sure that the 'hello' command from the helloworld.py plugin # is now available. cmd = [hlp for hlp in n.rpc.help()["help"] if "hello" in hlp["command"]] assert(len(cmd) == 1) - # Make sure 'rescan' and 'list' controls dont crash + # Make sure 'rescan' and 'list' subcommands dont crash n.rpc.plugin_rescan() n.rpc.plugin_list() - time.sleep(1) # Make sure the plugin behaves normally after stop and restart - n.rpc.plugin_stop(plugin="helloworld.py") + assert("Successfully stopped helloworld.py." == n.rpc.plugin_stop(plugin="helloworld.py")['']) n.daemon.wait_for_log(r"Killing plugin: helloworld.py") - time.sleep(1) n.rpc.plugin_start(plugin=os.path.join(os.getcwd(), "contrib/plugins/helloworld.py")) n.daemon.wait_for_log(r"Plugin helloworld.py initialized") assert("Hello world" == n.rpc.call(method="hello")) # Now stop the helloworld plugin - n.rpc.plugin_stop(plugin="helloworld.py") + assert("Successfully stopped helloworld.py." == n.rpc.plugin_stop(plugin="helloworld.py")['']) n.daemon.wait_for_log(r"Killing plugin: helloworld.py") - time.sleep(1) # Make sure that the 'hello' command from the helloworld.py plugin # is not available anymore. cmd = [hlp for hlp in n.rpc.help()["help"] if "hello" in hlp["command"]] assert(len(cmd) == 0) - # Test that we cannot stop a plugin with 'dynamic' set to False in + # Test that we cannot start a plugin with 'dynamic' set to False in # getmanifest - n.rpc.plugin_start(plugin=os.path.join(os.getcwd(), "tests/plugins/static.py")) - n.daemon.wait_for_log(r"Static plugin initialized.") - with pytest.raises(RpcError, match=r"plugin cannot be managed when lightningd is up"): - n.rpc.plugin_stop(plugin="static.py") + with pytest.raises(RpcError, match=r"Not a dynamic plugin"): + n.rpc.plugin_start(plugin=os.path.join(os.getcwd(), "tests/plugins/static.py")) + + # Test that we cannot stop a started plugin with 'dynamic' flag set to + # False + n2 = node_factory.get_node(options={ + "plugin": os.path.join(os.getcwd(), "tests/plugins/static.py") + }) + with pytest.raises(RpcError, match=r"static.py cannot be managed when lightningd is up"): + n2.rpc.plugin_stop(plugin="static.py") + + # Test that we don't crash when starting a broken plugin + with pytest.raises(RpcError, match=r"Timed out while waiting for plugin response"): + n2.rpc.plugin_start(plugin=os.path.join(os.getcwd(), "tests/plugins/broken.py")) def test_plugin_disable(node_factory): From a95eb72aa2a9155deac4d42e60dfc0e72cc62d02 Mon Sep 17 00:00:00 2001 From: darosior Date: Sat, 14 Sep 2019 17:35:28 +0200 Subject: [PATCH 11/14] plugins: cleanup shared headers between dynamic and static plugins --- lightningd/plugin.c | 12 ------------ lightningd/plugin.h | 13 +++++++++++++ lightningd/plugin_control.c | 10 ---------- lightningd/plugin_control.h | 2 +- 4 files changed, 14 insertions(+), 23 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 01daa5850848..f47b82b6283d 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -1,21 +1,10 @@ #include #include #include -#include -#include #include #include -#include -#include -#include -#include -#include #include -#include -#include -#include #include -#include #include #include #include @@ -23,7 +12,6 @@ #include #include #include -#include /* How many seconds may the plugin take to reply to the `getmanifest * call`? This is the maximum delay to `lightningd --help` and until diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 828561c9df0d..a322204aa921 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -3,10 +3,23 @@ #include "config.h" #include #include +#include #include +#include #include +#include +#include +#include +#include +#include +#include +#include +#include #include +#include #include +#include + enum plugin_state { UNCONFIGURED, diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index dc06c01ccca2..970c4966508d 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -1,15 +1,5 @@ -#include -#include -#include -#include -#include -#include -#include -#include -#include #include #include -#include /* A dummy structure used to give multiple arguments to callbacks. */ struct dynamic_plugin { diff --git a/lightningd/plugin_control.h b/lightningd/plugin_control.h index bcc74abfd86a..f63bdd552123 100644 --- a/lightningd/plugin_control.h +++ b/lightningd/plugin_control.h @@ -1,7 +1,7 @@ #ifndef LIGHTNING_LIGHTNINGD_PLUGIN_CONTROL_H #define LIGHTNING_LIGHTNINGD_PLUGIN_CONTROL_H #include "config.h" -#include +#include #endif /* LIGHTNING_LIGHTNINGD_PLUGIN_CONTROL_H */ From d846a554294aac18a44fb869c381dc1df2b2c8b5 Mon Sep 17 00:00:00 2001 From: darosior Date: Sun, 15 Sep 2019 22:41:19 +0200 Subject: [PATCH 12/14] plugins: make the default plugins directory a member of 'plugins' --- lightningd/plugin.c | 13 +++++++------ lightningd/plugin.h | 6 +++--- lightningd/plugin_control.c | 3 +-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index f47b82b6283d..22103ae21c34 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -943,18 +943,19 @@ void clear_plugins(struct plugins *plugins) tal_free(p); } -void plugins_add_default_dir(struct plugins *plugins, const char *default_dir) +void plugins_add_default_dir(struct plugins *plugins) { - DIR *d = opendir(default_dir); + DIR *d = opendir(plugins->default_dir); if (d) { struct dirent *di; /* Add this directory itself, and recurse down once. */ - add_plugin_dir(plugins, default_dir, true); + add_plugin_dir(plugins, plugins->default_dir, true); while ((di = readdir(d)) != NULL) { if (streq(di->d_name, ".") || streq(di->d_name, "..")) continue; - add_plugin_dir(plugins, path_join(tmpctx, default_dir, di->d_name), true); + add_plugin_dir(plugins, path_join(tmpctx, plugins->default_dir, + di->d_name), true); } closedir(d); } @@ -968,8 +969,8 @@ void plugins_init(struct plugins *plugins, const char *dev_plugin_debug) struct jsonrpc_request *req; plugins->pending_manifests = 0; - plugins_add_default_dir(plugins, - path_join(tmpctx, plugins->ld->config_dir, "plugins")); + plugins->default_dir = path_join(plugins, plugins->ld->config_dir, "plugins"); + plugins_add_default_dir(plugins); setenv("LIGHTNINGD_PLUGIN", "1", 1); setenv("LIGHTNINGD_VERSION", version(), 1); diff --git a/lightningd/plugin.h b/lightningd/plugin.h index a322204aa921..1130790e2e4f 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -84,6 +84,7 @@ struct plugins { struct log_book *log_book; struct lightningd *ld; + const char *default_dir; }; /* The value of a plugin option, which can have different types. @@ -115,10 +116,9 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, struct lightningd *ld); /** - * Search for `default_dir`, and if it exists add every directory it - * contains as a plugin dir. + * Recursively add all plugins from the default plugins directory. */ -void plugins_add_default_dir(struct plugins *plugins, const char *default_dir); +void plugins_add_default_dir(struct plugins *plugins); /** * Initialize the registered plugins. diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 970c4966508d..897ad99ac6f1 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -223,8 +223,7 @@ plugin_dynamic_rescan_plugins(struct command *cmd) struct plugin *p; /* This will not fail on "already registered" error. */ - plugins_add_default_dir(cmd->ld->plugins, - path_join(tmpctx, cmd->ld->config_dir, "plugins")); + plugins_add_default_dir(cmd->ld->plugins); found = false; list_for_each(&cmd->ld->plugins->plugins, p, list) { From 001c36b2fd9acbc7f9972e2046c470ba456b5d4c Mon Sep 17 00:00:00 2001 From: darosior Date: Mon, 16 Sep 2019 11:36:05 +0200 Subject: [PATCH 13/14] doc: Document the changes to the 'plugin' command --- CHANGELOG.md | 3 +++ doc/lightning-plugin.7 | 25 ++++++++++++++----------- doc/lightning-plugin.7.md | 18 ++++++++++++------ 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9f386e4734a..6deed42abae3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - JSON API: `fundchannel_cancel` is extended to work before funding broadcast. - JSON API: The parameter `exclude` of `getroute` now also support node-id. - JSON-API: `pay` can exclude error nodes if the failcode of `sendpay` has the NODE bit set +- JSON API: The `plugin` command now returns on error. A timeout of 20 seconds is added to `start` and `startdir` + subcommands at the end of which the plugin is errored if it did not complete the handshake with `lightningd`. +- JSON API: The `plugin` command does not allow to start static plugins after `lightningd` startup anymore. ### Deprecated diff --git a/doc/lightning-plugin.7 b/doc/lightning-plugin.7 index dcd6e0cc41a1..f1e86968f1fc 100644 --- a/doc/lightning-plugin.7 +++ b/doc/lightning-plugin.7 @@ -14,16 +14,18 @@ optionally one or two parameters which describes the plugin on which the action has to be taken\. -The \fIstart\fR command takes a path as parameter and will load the plugin -available from this path\. +The \fIstart\fR command takes a path as the first parameter and will load the +plugin available from this path\. It will wait for the plugin to complete +the handshake with \fBlightningd\fR for 20 seconds at the most\. -The \fIstop\fR command takes a plugin name as parameter and will kill and +The \fIstop\fR command takes a plugin name as parameter\. It will kill and unload the specified plugin\. -The \fIstartdir\fR command takes a directory path as parameter and will load -all plugins this directory contains\. +The \fIstartdir\fR command takes a directory path as first parameter and will +load all plugins this directory contains\. It will wait for each plugin to +complete the handshake with \fBlightningd\fR for 20 seconds at the most\. The \fIrescan\fR command starts all not-already-loaded plugins from the @@ -34,20 +36,21 @@ The \fIlist\fR command will return all the active plugins\. .SH RETURN VALUE -On success, this returns an array \fIplugins\fR of objects, one by plugin\. +On success, all subcommands but \fIstop\fR return an array \fIplugins\fR of +objects, one by plugin\. Each object contains the name of the plugin (\fIname\fR field) and its status (\fIactive\fR boolean field)\. Since plugins are configured asynchronously, a freshly started plugin may not appear immediately\. + +On error, the reason why the action could not be taken upon the +plugin is returned\. + .SH AUTHOR -Antoine Poinsot \fI is mainly responsible\. +Antoine Poinsot (\fIdarosior@protonmail.com\fR) is mainly responsible\. .SH RESOURCES Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -.HL - -Last updated 2019-07-29 12:57:57 CEST - diff --git a/doc/lightning-plugin.7.md b/doc/lightning-plugin.7.md index 467e083e9a99..1cec9a94bfa8 100644 --- a/doc/lightning-plugin.7.md +++ b/doc/lightning-plugin.7.md @@ -15,14 +15,16 @@ restart lightningd. It takes 1 to 3 parameters: a command optionally one or two parameters which describes the plugin on which the action has to be taken. -The *start* command takes a path as parameter and will load the plugin -available from this path. +The *start* command takes a path as the first parameter and will load the +plugin available from this path. It will wait for the plugin to complete +the handshake with `lightningd` for 20 seconds at the most. -The *stop* command takes a plugin name as parameter and will kill and +The *stop* command takes a plugin name as parameter. It will kill and unload the specified plugin. -The *startdir* command takes a directory path as parameter and will load -all plugins this directory contains. +The *startdir* command takes a directory path as first parameter and will +load all plugins this directory contains. It will wait for each plugin to +complete the handshake with `lightningd` for 20 seconds at the most. The *rescan* command starts all not-already-loaded plugins from the default plugins directory (by default *~/.lightning/plugins*). @@ -32,11 +34,15 @@ The *list* command will return all the active plugins. RETURN VALUE ------------ -On success, this returns an array *plugins* of objects, one by plugin. +On success, all subcommands but *stop* return an array *plugins* of +objects, one by plugin. Each object contains the name of the plugin (*name* field) and its status (*active* boolean field). Since plugins are configured asynchronously, a freshly started plugin may not appear immediately. +On error, the reason why the action could not be taken upon the +plugin is returned. + AUTHOR ------ From 84b5b364ecb1317c7b94ac202b17e40374c53524 Mon Sep 17 00:00:00 2001 From: darosior Date: Thu, 26 Sep 2019 17:00:18 +0200 Subject: [PATCH 14/14] plugins: Add a destructor to the plugin request, in case it dies Authored-by: @rustyrussell --- lightningd/plugin.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 22103ae21c34..eba3c1826362 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -252,7 +252,6 @@ static void plugin_response_handle(struct plugin *plugin, /* We expect the request->cb to copy if needed */ request->response_cb(plugin->buffer, toks, idtok, request->response_cb_arg); - uintmap_del(&plugin->plugins->pending_requests, id); tal_free(request); } @@ -1118,12 +1117,20 @@ void plugins_notify(struct plugins *plugins, tal_free(n); } +static void destroy_request(struct jsonrpc_request *req, + struct plugin *plugin) +{ + uintmap_del(&plugin->plugins->pending_requests, req->id); +} + void plugin_request_send(struct plugin *plugin, struct jsonrpc_request *req TAKES) { /* Add to map so we can find it later when routing the response */ tal_steal(plugin, req); uintmap_add(&plugin->plugins->pending_requests, req->id, req); + /* Add destructor in case plugin dies. */ + tal_add_destructor2(req, destroy_request, plugin); plugin_send(plugin, req->stream); /* plugin_send steals the stream, so remove the dangling * pointer here */