From 1d8aecb44fcbd6813dd85ef502333e4c9ab301bb Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 3 Sep 2021 19:46:18 +0930 Subject: [PATCH] lightningd: call "shutdown" notification on plugins at shutdown. Signed-off-by: Rusty Russell Changelog-Added: Plugins: `shutdown` notification for clean exits. --- lightningd/lightningd.c | 6 +- lightningd/notification.c | 11 ++++ lightningd/notification.h | 3 + lightningd/plugin.c | 93 ++++++++++++++++++++------- lightningd/plugin.h | 36 +++++------ lightningd/test/run-find_my_abspath.c | 6 +- 6 files changed, 107 insertions(+), 48 deletions(-) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 9282d9eab..83d6ed44b 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -1146,10 +1146,10 @@ int main(int argc, char *argv[]) /* We're not going to collect our children. */ remove_sigchild_handler(); - shutdown_subdaemons(ld); - /* Remove plugins. */ - plugins_free(ld->plugins); + /* Tell plugins we're shutting down. */ + shutdown_plugins(ld); + shutdown_subdaemons(ld); /* Clean up the JSON-RPC. This needs to happen in a DB transaction since * it might actually be touching the DB in some destructors, e.g., diff --git a/lightningd/notification.c b/lightningd/notification.c index df9b6deae..7f3ea216f 100644 --- a/lightningd/notification.c +++ b/lightningd/notification.c @@ -554,3 +554,14 @@ void notify_channel_open_failed(struct lightningd *ld, jsonrpc_notification_end(n); plugins_notify(ld->plugins, take(n)); } + +REGISTER_NOTIFICATION(plugin_shutdown, NULL); + +bool notify_plugin_shutdown(struct lightningd *ld, struct plugin *p) +{ + struct jsonrpc_notification *n = + jsonrpc_notification_start(NULL, "shutdown"); + + jsonrpc_notification_end(n); + return plugin_single_notify(p, take(n)); +} diff --git a/lightningd/notification.h b/lightningd/notification.h index 452926a37..ac27c8c14 100644 --- a/lightningd/notification.h +++ b/lightningd/notification.h @@ -104,4 +104,7 @@ void notify_openchannel_peer_sigs(struct lightningd *ld, void notify_channel_open_failed(struct lightningd *ld, const struct channel_id *cid); + +/* Tell this plugin to shutdown: returns true if it was subscribed. */ +bool notify_plugin_shutdown(struct lightningd *ld, struct plugin *p); #endif /* LIGHTNING_LIGHTNINGD_NOTIFICATION_H */ diff --git a/lightningd/plugin.c b/lightningd/plugin.c index fa1dfcf5e..e983ddb36 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -89,22 +89,6 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, return p; } -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)) { - p = list_pop(&plugins->plugins, struct plugin, list); - tal_free(p); - } - - tal_free(plugins); -} - /* Check that all the plugin's subscriptions are actually for known * notification topics. Emit a warning if that's not the case, but * don't kill the plugin. */ @@ -205,8 +189,12 @@ static void destroy_plugin(struct plugin *p) /* If we are shutting down, do not continue to checking if * the dying plugin is important. */ - if (p->plugins->shutdown) + if (p->plugins->shutdown) { + /* But return if this was the last plugin! */ + if (list_empty(&p->plugins->plugins)) + io_break(p->plugins); return; + } /* Now check if the dying plugin is important. */ if (p->important) { @@ -1958,21 +1946,36 @@ static bool plugin_subscriptions_contains(struct plugin *plugin, return false; } +bool plugin_single_notify(struct plugin *p, + const struct jsonrpc_notification *n TAKES) +{ + bool interested; + if (plugin_subscriptions_contains(p, n->method)) { + plugin_send(p, json_stream_dup(p, n->stream, p->log)); + interested = true; + } else + interested = false; + + if (taken(n)) + tal_free(n); + + return interested; +} + void plugins_notify(struct plugins *plugins, const struct jsonrpc_notification *n TAKES) { struct plugin *p; + if (taken(n)) + tal_steal(tmpctx, n); + /* If we're shutting down, ld->plugins will be NULL */ if (plugins) { list_for_each(&plugins->plugins, p, list) { - if (plugin_subscriptions_contains(p, n->method)) - plugin_send(p, json_stream_dup(p, n->stream, - p->log)); + plugin_single_notify(p, n); } } - if (taken(n)) - tal_free(n); } static void destroy_request(struct jsonrpc_request *req, @@ -2067,3 +2070,49 @@ bool was_plugin_destroyed(struct plugin_destroyed *pd) tal_free(pd); return true; } + +static void plugin_shutdown_timeout(struct lightningd *ld) +{ + io_break(ld->plugins); +} + +void shutdown_plugins(struct lightningd *ld) +{ + struct plugin *p, *next; + + /* This makes sure we don't complain about important plugins + * vanishing! */ + ld->plugins->shutdown = true; + + /* Tell them all to shutdown; if they care. */ + list_for_each_safe(&ld->plugins->plugins, p, next, list) { + /* Kill immediately, deletes self from list. */ + if (!notify_plugin_shutdown(ld, p)) + tal_free(p); + } + + /* If anyone was interested in shutdown, give them time. */ + if (!list_empty(&ld->plugins->plugins)) { + struct oneshot *t; + + /* 30 seconds should do it. */ + t = new_reltimer(ld->timers, ld, + time_from_sec(30), + plugin_shutdown_timeout, ld); + + io_loop_with_timers(ld); + tal_free(t); + + /* Report and free remaining plugins. */ + while (!list_empty(&ld->plugins->plugins)) { + p = list_pop(&ld->plugins->plugins, struct plugin, list); + log_debug(ld->log, + "%s: failed to shutdown, killing.", + p->shortname); + tal_free(p); + } + } + + /* NULL stops notifications trying to access plugins. */ + ld->plugins = tal_free(ld->plugins); +} diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 57c6a3f5d..99ab5422d 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -189,26 +189,6 @@ void plugins_add_default_dir(struct plugins *plugins); */ void plugins_init(struct plugins *plugins); -/** - * Free all resources that are held by plugins in the correct order. - * - * This function ensures that the resources dangling off of the plugins struct - * are freed in the correct order. This is necessary since the struct manages - * two orthogonal sets of resources: - * - * - Plugins - * - Hook calls and notifications - * - * The tal hierarchy is organized in a plugin centric way, i.e., the plugins - * may exit in an arbitrary order and they'll unregister pointers in the other - * resources. However this will fail if `tal_free` decides to free one of the - * non-plugin resources (technically a sibling in the allocation tree) before - * the plugins we will get a use-after-free. This function fixes this by - * freeing in the correct order without adding additional child-relationships - * in the allocation structure and without adding destructors. - */ -void plugins_free(struct plugins *plugins); - /** * Register a plugin for initialization and execution. * @@ -271,6 +251,11 @@ bool plugins_send_getmanifest(struct plugins *plugins); void plugin_kill(struct plugin *plugin, enum log_level loglevel, const char *fmt, ...); +/** + * Tell all the plugins we're shutting down, and free them. + */ +void shutdown_plugins(struct lightningd *ld); + /** * Returns the plugin which registers the command with name {cmd_name} */ @@ -351,6 +336,17 @@ char *add_plugin_dir(struct plugins *plugins, const char *dir, */ void clear_plugins(struct plugins *plugins); +/** + * Send notification to this single plugin, if interested. + * + * Returns true if it was subscribed to the notification. + */ +bool plugin_single_notify(struct plugin *p, + const struct jsonrpc_notification *n TAKES); + +/** + * Send notification to all interested plugins. + */ void plugins_notify(struct plugins *plugins, const struct jsonrpc_notification *n TAKES); diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index 4dde6dbf4..3eb28f7fa 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -184,9 +184,6 @@ void per_peer_state_set_fds_arr(struct per_peer_state *pps UNNEEDED, const int * /* Generated stub for plugins_config */ void plugins_config(struct plugins *plugins UNNEEDED) { fprintf(stderr, "plugins_config called!\n"); abort(); } -/* Generated stub for plugins_free */ -void plugins_free(struct plugins *plugins UNNEEDED) -{ fprintf(stderr, "plugins_free called!\n"); abort(); } /* Generated stub for plugins_init */ void plugins_init(struct plugins *plugins UNNEEDED) { fprintf(stderr, "plugins_init called!\n"); abort(); } @@ -205,6 +202,9 @@ void setup_color_and_alias(struct lightningd *ld UNNEEDED) void setup_topology(struct chain_topology *topology UNNEEDED, u32 min_blockheight UNNEEDED, u32 max_blockheight UNNEEDED) { fprintf(stderr, "setup_topology called!\n"); abort(); } +/* Generated stub for shutdown_plugins */ +void shutdown_plugins(struct lightningd *ld UNNEEDED) +{ fprintf(stderr, "shutdown_plugins called!\n"); abort(); } /* Generated stub for stop_topology */ void stop_topology(struct chain_topology *topo UNNEEDED) { fprintf(stderr, "stop_topology called!\n"); abort(); }