From b592d6fd8ff007db3811ca8b79addf88b3a1bac2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 5 May 2020 02:49:05 +0930 Subject: [PATCH] lightningd: fix race where we do rescan before all plugins finish init. The symptom (under heavy load and valgrind) in test_plugin_command: lightningd: common/json_stream.c:237: json_stream_output_: Assertion `!js->reader' failed. This is because we try to call `getmanifest` again on `pay` which has not yet responded to init. The minimal fix for this is to keep proper state, so we can tell the difference between "not yet called getmanifest" and "not yet finished init". Signed-off-by: Rusty Russell --- lightningd/bitcoind.c | 5 +++-- lightningd/plugin.c | 5 +++-- lightningd/plugin.h | 8 +++++++- lightningd/plugin_control.c | 6 +++--- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 89a9e22d9..2a4b28a18 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -47,7 +47,7 @@ static void plugin_config_cb(const char *buffer, const jsmntok_t *idtok, struct plugin *plugin) { - plugin->plugin_state = CONFIGURED; + plugin->plugin_state = INIT_COMPLETE; io_break(plugin); } @@ -77,8 +77,9 @@ static void wait_plugin(struct bitcoind *bitcoind, const char *method, * before responding to `init`). * Note that lightningd/plugin will not send `init` to an already * configured plugin. */ - if (p->plugin_state != CONFIGURED) + if (p->plugin_state == NEEDS_INIT) config_plugin(p); + strmap_add(&bitcoind->pluginsmap, method, p); } diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 01e5e4d67..ef1b81741 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -969,6 +969,7 @@ bool plugin_parse_getmanifest_response(const char *buffer, !plugin_hooks_add(plugin, buffer, resulttok)) return false; + plugin->plugin_state = NEEDS_INIT; return true; } @@ -1167,7 +1168,7 @@ static void plugin_config_cb(const char *buffer, const jsmntok_t *idtok, struct plugin *plugin) { - plugin->plugin_state = CONFIGURED; + plugin->plugin_state = INIT_COMPLETE; } void @@ -1240,7 +1241,7 @@ void plugins_config(struct plugins *plugins) { struct plugin *p; list_for_each(&plugins->plugins, p, list) { - if (p->plugin_state == UNCONFIGURED) + if (p->plugin_state == NEEDS_INIT) plugin_config(p); } diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 1a47232dd..0d26325e9 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -22,8 +22,14 @@ enum plugin_state { + /* We have to ask getmanifest */ UNCONFIGURED, - CONFIGURED + /* Got `getmanifest` reply, now we need to send `init`. */ + NEEDS_INIT, + /* We have to get `init` response */ + AWAITING_INIT_RESPONSE, + /* We have `init` response. */ + INIT_COMPLETE }; /** diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 0ed7f951e..c0fdad82d 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -25,7 +25,7 @@ static struct command_result *plugin_dynamic_list_plugins(struct command *cmd) json_object_start(response, NULL); json_add_string(response, "name", p->cmd); json_add_bool(response, "active", - p->plugin_state == CONFIGURED); + p->plugin_state == INIT_COMPLETE); json_object_end(response); } json_array_end(response); @@ -69,14 +69,14 @@ static void plugin_dynamic_config_callback(const char *buffer, { struct plugin *p; - dp->plugin->plugin_state = CONFIGURED; + dp->plugin->plugin_state = INIT_COMPLETE; /* Reset the timer only now so that we are either configured, or * killed. */ tal_free(dp->plugin->timeout_timer); tal_del_destructor2(dp->plugin, plugin_dynamic_crash, dp); list_for_each(&dp->plugin->plugins->plugins, p, list) { - if (p->plugin_state != CONFIGURED) + if (p->plugin_state != INIT_COMPLETE) return; }