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/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 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 ------ diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 036fc57a8899..eba3c1826362 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 @@ -50,6 +38,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); @@ -61,7 +50,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 +59,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 +96,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) @@ -262,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); } @@ -435,8 +424,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 +434,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; @@ -813,31 +802,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)) @@ -847,14 +823,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; - /* 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); + 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 */ + plugin->plugins->pending_manifests--; + 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); /* Reset timer, it'd kill us otherwise. */ tal_free(plugin->timeout_timer); @@ -900,10 +889,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, @@ -918,7 +909,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)); @@ -930,9 +921,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; @@ -947,34 +942,39 @@ 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); } } -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->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); + /* 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); @@ -1013,18 +1013,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); @@ -1038,17 +1026,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"); @@ -1067,7 +1050,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); } @@ -1076,10 +1071,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; @@ -1124,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 */ diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 8787b62379f5..1130790e2e4f 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -3,14 +3,26 @@ #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, - CONFIGURING, CONFIGURED }; @@ -31,7 +43,6 @@ struct plugin { /* If this plugin can be restarted without restarting lightningd */ bool dynamic; - bool signal_startup; /* Stuff we read */ char *buffer; @@ -73,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. @@ -104,12 +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_start(struct plugins *plugins, const char *dev_plugin_debug); +void plugins_add_default_dir(struct plugins *plugins); /** * Initialize the registered plugins. @@ -130,7 +139,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 @@ -159,6 +168,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. * @@ -181,7 +204,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. @@ -203,6 +226,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. diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index ff5d49e78167..897ad99ac6f1 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -1,12 +1,245 @@ -#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); +} + +/** + * 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); +} + +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); +} + +/** + * 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); + + 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 @@ -19,16 +252,13 @@ 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; - bool plugin_found; if (!param(cmd, buffer, params, p_req("subcommand", param_ignore, cmd), @@ -36,23 +266,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; @@ -63,7 +277,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", @@ -78,7 +292,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); @@ -88,32 +302,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 */ - } - - /* 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) { - 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 = { 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 */ 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):