From f4e2d9a0ae908d95726e5b5d2e0c7436faba721c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 17 Aug 2023 19:50:15 +0930 Subject: [PATCH] lightningd: clean up properly if we fail to exec plugin. Reported-by: @niftynei Signed-off-by: Rusty Russell Changelog-Fixed: Plugins: we clean up properly if a plugin fails to start, and we don't kill all processes if it's from `plugin startdir`. --- lightningd/plugin.c | 2 +- lightningd/plugin_control.c | 7 +++++-- tests/test_plugin.py | 2 -- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index e3fc17014..3555fc1c4 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -1869,7 +1869,7 @@ bool plugins_send_getmanifest(struct plugins *plugins, const char *cmd_id) } if (plugins->startup) fatal("error starting plugin '%s': %s", p->cmd, err); - plugin_kill(p, LOG_UNUSUAL, "%s", err); + tal_free(p); } return sent; diff --git a/lightningd/plugin_control.c b/lightningd/plugin_control.c index 1d334a4cf..9879f9903 100644 --- a/lightningd/plugin_control.c +++ b/lightningd/plugin_control.c @@ -74,13 +74,16 @@ plugin_dynamic_start(struct plugin_command *pcmd, const char *plugin_path, "%s: %s", plugin_path, errno ? strerror(errno) : "already registered"); - /* This will come back via plugin_cmd_killed or plugin_cmd_succeeded */ err = plugin_send_getmanifest(p, pcmd->cmd->id); - if (err) + if (err) { + /* Free plugin with cmd (it owns buffer and params!) */ + tal_steal(pcmd->cmd, p); return command_fail(pcmd->cmd, PLUGIN_ERROR, "%s: %s", plugin_path, err); + } + /* This will come back via plugin_cmd_killed or plugin_cmd_succeeded */ return command_still_pending(pcmd->cmd); } diff --git a/tests/test_plugin.py b/tests/test_plugin.py index ab221b9d2..63ea0425b 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -4275,7 +4275,6 @@ def test_renepay_not_important(node_factory): l1.rpc.plugin_start(os.path.join(os.getcwd(), 'plugins/cln-renepay')) -@pytest.mark.xfail(strict=True) @unittest.skipIf(VALGRIND, "Valgrind doesn't handle bad #! lines the same") def test_plugin_nostart(node_factory): "Should not appear in list if it didn't even start" @@ -4287,7 +4286,6 @@ def test_plugin_nostart(node_factory): assert [p['name'] for p in l1.rpc.plugin_list()['plugins'] if 'badinterp' in p['name']] == [] -@pytest.mark.xfail(strict=True) def test_plugin_startdir_lol(node_factory): """Though we fail to start many of them, we don't crash!""" l1 = node_factory.get_node(allow_broken_log=True)