lightningd: clean up properly if we fail to exec plugin.

Reported-by: @niftynei Signed-off-by: Rusty Russell <[email protected]> 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`.
ElementsProject · Aug 17, 2023 · 538492b · 538492b
1 parent 8428839
commit 538492b
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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)