From cc76416447ef0ed2be57dba973e18e4c25232edd Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Mon, 4 Feb 2019 21:25:42 +1030
Subject: [PATCH] jsonrpc: use tal destructor to remove json commands when
 required.

This fixes a bug with a plugin duplicating an existing name
where we'd crash, too.

This doesn't work for builtins, which aren't tal objects, so
create a separate path for them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 lightningd/jsonrpc.c | 33 ++++++++++++++++++++++-----------
 lightningd/jsonrpc.h | 10 ++--------
 lightningd/plugin.c  |  7 -------
 3 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c
index 3d9d7bb49..8081393cb 100644
--- a/lightningd/jsonrpc.c
+++ b/lightningd/jsonrpc.c
@@ -769,7 +769,20 @@ static struct io_plan *incoming_jcon_connected(struct io_conn *conn,
 	return jcon_connected(notleak(conn), ld);
 }
 
-bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command)
+static void destroy_json_command(struct json_command *command, struct jsonrpc *rpc)
+{
+	for (size_t i = 0; i < tal_count(rpc->commands); i++) {
+		if (rpc->commands[i] == command) {
+			tal_arr_remove(&rpc->commands, i);
+			return;
+		}
+	}
+	abort();
+}
+
+/* For built-in ones, they're not tal objects, so no destructor */
+static bool jsonrpc_command_add_perm(struct jsonrpc *rpc,
+				     struct json_command *command)
 {
 	size_t count = tal_count(rpc->commands);
 
@@ -782,16 +795,12 @@ bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command)
 	return true;
 }
 
-void jsonrpc_command_remove(struct jsonrpc *rpc, const char *method)
+bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command)
 {
-	for (size_t i=0; i<tal_count(rpc->commands); i++) {
-		struct json_command *cmd = rpc->commands[i];
-		if (streq(cmd->name, method)) {
-			tal_arr_remove(&rpc->commands, i);
-			tal_free(cmd);
-			break;
-		}
-	}
+	if (!jsonrpc_command_add_perm(rpc, command))
+		return false;
+	tal_add_destructor2(command, destroy_json_command, rpc);
+	return true;
 }
 
 struct jsonrpc *jsonrpc_new(const tal_t *ctx, struct lightningd *ld)
@@ -802,7 +811,9 @@ struct jsonrpc *jsonrpc_new(const tal_t *ctx, struct lightningd *ld)
 	jsonrpc->commands = tal_arr(jsonrpc, struct json_command *, 0);
 	jsonrpc->log = new_log(jsonrpc, ld->log_book, "jsonrpc");
 	for (size_t i=0; i<num_cmdlist; i++) {
-		jsonrpc_command_add(jsonrpc, commands[i]);
+		if (!jsonrpc_command_add_perm(jsonrpc, commands[i]))
+			fatal("Cannot add duplicate command %s",
+			      commands[i]->name);
 	}
 	jsonrpc->rpc_listener = NULL;
 	return jsonrpc;
diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h
index b248d54bb..f296fe75c 100644
--- a/lightningd/jsonrpc.h
+++ b/lightningd/jsonrpc.h
@@ -168,16 +168,10 @@ void jsonrpc_listen(struct jsonrpc *rpc, struct lightningd *ld);
  *
  * Returns true if the command was added correctly, false if adding
  * this would clobber a command name.
- */
-bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command);
-
-/**
- * Remove a command/method from the JSON-RPC.
  *
- * Used to dynamically remove a `struct json_command` from the
- * JSON-RPC dispatch table by its name.
+ * Free @command to remove it.
  */
-void jsonrpc_command_remove(struct jsonrpc *rpc, const char *method);
+bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command);
 
 /**
  * Begin a JSON-RPC notification with the specified topic.
diff --git a/lightningd/plugin.c b/lightningd/plugin.c
index 7f4d7c88a..14c5e5208 100644
--- a/lightningd/plugin.c
+++ b/lightningd/plugin.c
@@ -547,12 +547,6 @@ static bool plugin_opts_add(struct plugin *plugin,
 	return true;
 }
 
-static void plugin_rpcmethod_destroy(struct json_command *cmd,
-				     struct jsonrpc *rpc)
-{
-	jsonrpc_command_remove(rpc, cmd->name);
-}
-
 static void json_stream_forward_change_id(struct json_stream *stream,
 					  const char *buffer,
 					  const jsmntok_t *toks,
@@ -685,7 +679,6 @@ static bool plugin_rpcmethod_add(struct plugin *plugin,
 
 	cmd->deprecated = false;
 	cmd->dispatch = plugin_rpcmethod_dispatch;
-	tal_add_destructor2(cmd, plugin_rpcmethod_destroy, plugin->plugins->rpc);
 	if (!jsonrpc_command_add(plugin->plugins->rpc, cmd)) {
 		log_broken(plugin->log,
 			   "Could not register method \"%s\", a method with "