plugin: don't call notification after free.

This is an old bug, where a plugin can get called while we're shutting
down (and have freed plugins), but it's triggered more reliably by the
new warning notification hook.

For good measure, we also make freeing a plugin self-delete.

Valgrind error file: valgrind-errors.16763
==16886== Invalid read of size 8
==16886==    at 0x422919: plugins_notify (plugin.c:1096)
==16886==    by 0x413919: notify_warning (notification.c:61)
==16886==    by 0x412BDE: logv (log.c:251)
==16886==    by 0x412A98: log_ (log.c:311)
==16886==    by 0x4044BE: bcli_finished (bitcoind.c:178)
==16886==    by 0x459480: destroy_conn (poll.c:244)
==16886==    by 0x459499: destroy_conn_close_fd (poll.c:250)
==16886==    by 0x4619E1: notify (tal.c:235)
==16886==    by 0x461A7E: del_tree (tal.c:397)
==16886==    by 0x461AB5: del_tree (tal.c:407)
==16886==    by 0x461AB5: del_tree (tal.c:407)
==16886==    by 0x461AB5: del_tree (tal.c:407)
==16886==  Address 0x634a578 is 40 bytes inside a block of size 352 free'd
==16886==    at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16886==    by 0x461AFD: del_tree (tal.c:416)
==16886==    by 0x461FB7: tal_free (tal.c:481)
==16886==    by 0x411E0A: main (lightningd.c:841)
==16886==  Block was alloc'd at
==16886==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16886==    by 0x4617CE: allocate (tal.c:245)
==16886==    by 0x461E4C: tal_alloc_ (tal.c:423)
==16886==    by 0x42255E: plugins_new (plugin.c:106)
==16886==    by 0x41133D: new_lightningd (lightningd.c:218)
==16886==    by 0x411AD4: main (lightningd.c:649)

Signed-off-by: Rusty Russell <[email protected]>

lightningd/lightningd.c

@@ -838,7 +838,8 @@ int main(int argc, char *argv[])
 
 	shutdown_subdaemons(ld);
 
-	tal_free(ld->plugins);
+	/* Remove plugins. */
+	ld->plugins = tal_free(ld->plugins);
 
 	/* 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.,

lightningd/plugin.c

@@ -112,6 +112,11 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book,
 	return p;
 }
 
+static void destroy_plugin(struct plugin *p)
+{
+	list_del(&p->list);
+}
+
 void plugin_register(struct plugins *plugins, const char* path TAKES)
 {
 	struct plugin *p;
@@ -126,6 +131,7 @@ void plugin_register(struct plugins *plugins, const char* path TAKES)
 			 path_basename(tmpctx, p->cmd));
 	p->methods = tal_arr(p, const char *, 0);
 	list_head_init(&p->plugin_opts);
+	tal_add_destructor(p, destroy_plugin);
 }
 
 static bool paths_match(const char *cmd, const char *name)
@@ -1093,9 +1099,14 @@ void plugins_notify(struct plugins *plugins,
 		    const struct jsonrpc_notification *n TAKES)
 {
 	struct plugin *p;
-	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));
+
+	/* 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));
+		}
 	}
 	if (taken(n))
 		tal_free(n);