From a847487bbeee2b8971f59fce4facadc420334681 Mon Sep 17 00:00:00 2001 From: ZmnSCPxj jxPCSnmZ Date: Wed, 29 Jul 2020 19:24:07 +0800 Subject: [PATCH] lightningd/plugin.c: Add important plugins, which if they terminate, lightningd also terminates. Changelog-Added: New option `--important-plugin` loads a plugin is so important that if it dies, `lightningd` will exit rather than continue. You can still `--disable-plugin` it, however, which trumps `--important-plugin` and it will not be started at all. --- doc/lightningd-config.5 | 17 ++++++-- doc/lightningd-config.5.md | 16 ++++++-- lightningd/options.c | 19 ++++++++- lightningd/plugin.c | 80 ++++++++++++++++++++++++++++++------- lightningd/plugin.h | 11 ++++- lightningd/plugin_control.c | 2 +- tests/test_misc.py | 2 +- 7 files changed, 123 insertions(+), 24 deletions(-) diff --git a/doc/lightningd-config.5 b/doc/lightningd-config.5 index cb18a1b51..5f2a6d58b 100644 --- a/doc/lightningd-config.5 +++ b/doc/lightningd-config.5 @@ -534,10 +534,11 @@ add multiple directories\. \fBclear-plugins\fR -This option clears all \fIplugin\fR and \fIplugin-dir\fR options preceeding it, +This option clears all \fIplugin\fR, \fIimportant-plugin\fR, and \fIplugin-dir\fR options +preceeding it, including the default built-in plugin directory\. You can still add -\fIplugin-dir\fR and \fIplugin\fR options following this and they will have the -normal effect\. +\fIplugin-dir\fR, \fIplugin\fR, and \fIimportant-plugin\fR options following this +and they will have the normal effect\. \fBdisable-plugin\fR=\fIPLUGIN\fR @@ -547,6 +548,16 @@ be loaded at startup, whatever directory it is in\. This option is useful for disabling a single plugin inside a directory\. You can still explicitly load plugins which have been disabled, using \fBlightning-plugin\fR(7) \fBstart\fR\. + + \fBimportant-plugin\fR=\fIPLUGIN\fR +Speciy a plugin to run as part of C-lightning\. +This can be specified multiple times to add multiple plugins\. +Plugins specified via this option are considered so important, that if the +plugin stops for any reason (including via \fBlightning-plugin\fR(7) \fBstop\fR), +C-lightning will also stop running\. +This way, you can monitor crashes of important plugins by simply monitoring +if C-lightning terminates\. + .SH BUGS You should report bugs on our github issues page, and maybe submit a fix diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 39122ec9d..316a7ac15 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -439,10 +439,11 @@ loaded. *DIRECTORY* must exist; this can be specified multiple times to add multiple directories. **clear-plugins** -This option clears all *plugin* and *plugin-dir* options preceeding it, +This option clears all *plugin*, *important-plugin*, and *plugin-dir* options +preceeding it, including the default built-in plugin directory. You can still add -*plugin-dir* and *plugin* options following this and they will have the -normal effect. +*plugin-dir*, *plugin*, and *important-plugin* options following this +and they will have the normal effect. **disable-plugin**=*PLUGIN* If *PLUGIN* contains a /, plugins with the same path as *PLUGIN* will @@ -451,6 +452,15 @@ be loaded at startup, whatever directory it is in. This option is useful for disabling a single plugin inside a directory. You can still explicitly load plugins which have been disabled, using lightning-plugin(7) `start`. + **important-plugin**=*PLUGIN* +Speciy a plugin to run as part of C-lightning. +This can be specified multiple times to add multiple plugins. +Plugins specified via this option are considered so important, that if the +plugin stops for any reason (including via lightning-plugin(7) `stop`), +C-lightning will also stop running. +This way, you can monitor crashes of important plugins by simply monitoring +if C-lightning terminates. + BUGS ---- diff --git a/lightningd/options.c b/lightningd/options.c index e67bc7311..ba0099dd7 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -348,7 +348,7 @@ static char *opt_add_plugin(const char *arg, struct lightningd *ld) log_info(ld->log, "%s: disabled via disable-plugin", arg); return NULL; } - plugin_register(ld->plugins, arg, NULL); + plugin_register(ld->plugins, arg, NULL, false); return NULL; } @@ -369,6 +369,16 @@ static char *opt_clear_plugins(struct lightningd *ld) return NULL; } +static char *opt_important_plugin(const char *arg, struct lightningd *ld) +{ + if (plugin_blacklisted(ld->plugins, arg)) { + log_info(ld->log, "%s: disabled via disable-plugin", arg); + return NULL; + } + plugin_register(ld->plugins, arg, NULL, true); + return NULL; +} + /* Prompt the user to enter a password, from which will be derived the key used * for `hsm_secret` encryption. * The algorithm used to derive the key is Argon2(id), to which libsodium @@ -771,6 +781,10 @@ static void register_opts(struct lightningd *ld) NULL, ld, "Disable a particular plugin by filename/name"); + opt_register_early_arg("--important-plugin", opt_important_plugin, + NULL, ld, + "Add an important plugin to be run (can be used multiple times). Die if the plugin dies."); + /* Early, as it suppresses DNS lookups from cmdline too. */ opt_register_early_arg("--always-use-proxy", opt_set_bool_arg, opt_show_bool, @@ -1278,6 +1292,9 @@ static void add_config(struct lightningd *ld, json_add_opt_log_levels(response, ld->log); } else if (opt->cb_arg == (void *)opt_disable_plugin) { json_add_opt_disable_plugins(response, ld->plugins); + } else if (opt->cb_arg == (void *)opt_important_plugin) { + /* Do nothing, this is already handled by + * opt_add_plugin. */ } else if (opt->cb_arg == (void *)opt_add_plugin_dir || opt->cb_arg == (void *)plugin_opt_set || opt->cb_arg == (void *)plugin_opt_flag_set) { diff --git a/lightningd/plugin.c b/lightningd/plugin.c index dc8b577d1..5fc4f6454 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -53,6 +53,7 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->startup = true; p->json_cmds = tal_arr(p, struct command *, 0); p->blacklist = tal_arr(p, const char *, 0); + p->shutdown = false; uintmap_init(&p->pending_requests); memleak_add_helper(p, memleak_help_pending_requests); @@ -62,6 +63,9 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, void plugins_free(struct plugins *plugins) { struct plugin *p; + + plugins->shutdown = true; + /* Plugins are usually the unit of allocation, and they are internally * consistent, so let's free each plugin first. */ while (!list_empty(&plugins->plugins)) { @@ -105,6 +109,7 @@ struct command_result *plugin_register_all_complete(struct lightningd *ld, static void destroy_plugin(struct plugin *p) { struct plugin_rpccall *call; + plugin_hook_unregister_all(p); list_del(&p->list); @@ -118,10 +123,23 @@ static void destroy_plugin(struct plugin *p) /* Don't call this if we're still parsing options! */ if (p->plugin_state != UNCONFIGURED) check_plugins_resolved(p->plugins); + + /* If we are shutting down, do not continue to checking if + * the dying plugin is important. */ + if (p->plugins->shutdown) + return; + + /* Now check if the dying plugin is important. */ + if (p->important) { + log_broken(p->log, + "Plugin marked as important, " + "shutting down lightningd!"); + lightningd_exit(p->plugins->ld, 1); + } } struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, - struct command *start_cmd) + struct command *start_cmd, bool important) { struct plugin *p, *p_temp; @@ -130,6 +148,9 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, if (streq(path, p_temp->cmd)) { if (taken(path)) tal_free(path); + /* If added as "important", upgrade to "important". */ + if (important) + p_temp->important = true; return NULL; } } @@ -153,6 +174,8 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES, list_add_tail(&plugins->plugins, &p->list); tal_add_destructor(p, destroy_plugin); list_head_init(&p->pending_rpccalls); + + p->important = important; return p; } @@ -186,6 +209,8 @@ void plugin_blacklist(struct plugins *plugins, const char *name) log_info(plugins->log, "%s: disabled via disable-plugin", p->cmd); list_del_from(&plugins->plugins, &p->list); + /* disable-plugin overrides important-plugin. */ + p->important = false; tal_free(p); } } @@ -1188,7 +1213,7 @@ char *add_plugin_dir(struct plugins *plugins, const char *dir, bool error_ok) log_info(plugins->log, "%s: disabled via disable-plugin", fullpath); } else { - p = plugin_register(plugins, fullpath, NULL); + p = plugin_register(plugins, fullpath, NULL, false); if (!p && !error_ok) return tal_fmt(NULL, "Failed to register %s: %s", fullpath, strerror(errno)); @@ -1396,24 +1421,35 @@ void plugins_config(struct plugins *plugins) plugins->startup = false; } -void json_add_opt_plugins(struct json_stream *response, - const struct plugins *plugins) +/** json_add_opt_plugins_array + * + * @brief add a named array of plugins to the given response, + * depending on whether it is important or not important. + * + * @param response - the `json_stream` to write into. + * @param name - the field name of the array. + * @param plugins - the plugins object to query. + * @param important - match the `important` setting of the + * plugins to be added. + */ +static +void json_add_opt_plugins_array(struct json_stream *response, + const char *name, + const struct plugins *plugins, + bool important) { struct plugin *p; - struct plugin_opt *opt; const char *plugin_name; + struct plugin_opt *opt; const char *opt_name; - /* DEPRECATED: duplicated JSON "plugin" entries */ - if (deprecated_apis) { - list_for_each(&plugins->plugins, p, list) { - json_add_string(response, "plugin", p->cmd); - } - } - /* we output 'plugins' and their options as an array of substructures */ - json_array_start(response, "plugins"); + json_array_start(response, name); list_for_each(&plugins->plugins, p, list) { + /* Skip if not matching. */ + if (p->important != important) + continue; + json_object_start(response, NULL); json_add_string(response, "path", p->cmd); @@ -1444,6 +1480,23 @@ void json_add_opt_plugins(struct json_stream *response, json_array_end(response); } +void json_add_opt_plugins(struct json_stream *response, + const struct plugins *plugins) +{ + struct plugin *p; + + json_add_opt_plugins_array(response, "plugins", plugins, false); + json_add_opt_plugins_array(response, "important-plugins", plugins, true); + + /* DEPRECATED: duplicated JSON "plugin" entries */ + if (deprecated_apis) { + list_for_each(&plugins->plugins, p, list) { + json_add_string(response, p->important ? "important-plugin" : "plugin", p->cmd); + } + } + +} + void json_add_opt_disable_plugins(struct json_stream *response, const struct plugins *plugins) { @@ -1546,7 +1599,6 @@ static void mark_plugin_destroyed(const struct plugin *unused, pd->plugin = NULL; } - struct plugin_destroyed *plugin_detect_destruction(const struct plugin *plugin) { struct plugin_destroyed *pd = tal(NULL, struct plugin_destroyed); diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 5dc98346f..5b4947460 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -79,6 +79,10 @@ struct plugin { /* An array of currently pending RPC method calls, to be killed if the * plugin exits. */ struct list_head pending_rpccalls; + + /* If set, the plugin is so important that if it terminates early, + * C-lightning should terminate as well. */ + bool important; }; /** @@ -103,6 +107,9 @@ struct plugins { /* Blacklist of plugins from --disable-plugin */ const char **blacklist; + + /* Whether we are shutting down (`plugins_free` is called) */ + bool shutdown; }; /* The value of a plugin option, which can have different types. @@ -175,13 +182,15 @@ 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. + * @param important: The plugin is important. * * 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 command *start_cmd); + struct command *start_cmd, + bool important); /** * Returns true if the provided name matches a plugin command diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 87ad14bf9..047122a05 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -58,7 +58,7 @@ struct command_result *plugin_cmd_all_complete(struct plugins *plugins, static struct command_result * plugin_dynamic_start(struct command *cmd, const char *plugin_path) { - struct plugin *p = plugin_register(cmd->ld->plugins, plugin_path, cmd); + struct plugin *p = plugin_register(cmd->ld->plugins, plugin_path, cmd, false); const char *err; if (!p) diff --git a/tests/test_misc.py b/tests/test_misc.py index ee7c11a44..162353d0b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -833,7 +833,7 @@ def test_listconfigs(node_factory, bitcoind, chainparams): # Test one at a time. for c in configs.keys(): - if c.startswith('#') or c.startswith('plugins'): + if c.startswith('#') or c.startswith('plugins') or c == 'important-plugins': continue oneconfig = l1.rpc.listconfigs(config=c) assert(oneconfig[c] == configs[c])