jsonrpc: don't return "stop" until we actually have freed resources.

This is a painpoint with testing, that there's a noticable delay between
"Shutting down" from lightning-cli and being able to restart lightningd.

This fixes that by creating a canned response for this case, which is
simply written out immediately before exit.  At this point, the pidfile
has been deleted, the sockets have been closed, and the database
has been closed.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

CHANGELOG.md

@@ -61,6 +61,7 @@ changes.
 - JSON RPC: `decodeinvoice` and `pay` now handle unknown invoice fields properly.
 - JSON API: `waitsendpay` (PAY_STOPPED_RETRYING) error handler now returns valid JSON
 - protocol: don't send multiple identical feerate changes if we want the feerate higher than we can afford.
+- JSON API: `stop` now only returns once lightningd has released all resources.
 
 ### Security
 

lightningd/jsonrpc.c

@@ -23,6 +23,7 @@
 #include <ccan/err/err.h>
 #include <ccan/io/io.h>
 #include <ccan/json_escape/json_escape.h>
+#include <ccan/json_out/json_out.h>
 #include <ccan/str/hex/hex.h>
 #include <ccan/strmap/strmap.h>
 #include <ccan/tal/str/str.h>
@@ -87,9 +88,6 @@ struct json_connection {
 	/* How much has just been filled. */
 	size_t len_read;
 
-	/* We've been told to stop. */
-	bool stop;
-
 	/* Our commands */
 	struct list_head commands;
 
@@ -182,12 +180,37 @@ static struct command_result *json_stop(struct command *cmd,
 					const jsmntok_t *obj UNNEEDED,
 					const jsmntok_t *params)
 {
+	struct json_out *jout;
+	const char *p;
+	size_t len;
+
 	if (!param(cmd, buffer, params, NULL))
 		return command_param_failed();
 
 	/* This can't have closed yet! */
-	cmd->jcon->stop = true;
-	return command_success(cmd, json_stream_success(cmd));
+	cmd->ld->stop_conn = cmd->jcon->conn;
+	log_unusual(cmd->ld->log, "JSON-RPC shutdown");
+
+	/* This is the one place where result is a literal string. */
+	jout = json_out_new(tmpctx);
+	json_out_start(jout, NULL, '{');
+	json_out_addstr(jout, "jsonrpc", "2.0");
+	/* id may be a string or number, so copy direct. */
+	memcpy(json_out_member_direct(jout, "id", strlen(cmd->id)),
+	       cmd->id, strlen(cmd->id));
+	json_out_addstr(jout, "result", "Shutdown complete");
+	json_out_end(jout, '}');
+	json_out_finished(jout);
+
+	/* Add two \n */
+	memcpy(json_out_direct(jout, 2), "\n\n", strlen("\n\n"));
+	p = json_out_contents(jout, &len);
+	cmd->ld->stop_response = tal_strndup(cmd->ld, p, len);
+
+	/* Wake write loop in case it's not already. */
+	io_wake(cmd->jcon);
+
+	return command_still_pending(cmd);
 }
 
 static const struct json_command stop_command = {
@@ -661,7 +684,14 @@ static struct io_plan *start_json_stream(struct io_conn *conn,
 	/* Tell reader it can run next command. */
 	io_wake(conn);
 
-	/* Wait for attach_json_stream */
+	/* Once the stop_conn conn is drained, we can shut down. */
+	if (jcon->ld->stop_conn == conn) {
+		/* Return us to toplevel lightningd.c */
+		io_break(jcon->ld);
+		/* We never come back. */
+		return io_out_wait(conn, conn, io_never, conn);
+	}
+
 	return io_out_wait(conn, jcon, start_json_stream, jcon);
 }
 
@@ -673,13 +703,6 @@ static struct io_plan *stream_out_complete(struct io_conn *conn,
 	jcon_remove_json_stream(jcon, js);
 	tal_free(js);
 
-	if (jcon->stop) {
-		log_unusual(jcon->log, "JSON-RPC shutdown");
-		/* Return us to toplevel lightningd.c */
-		io_break(jcon->ld);
-		return io_close(conn);
-	}
-
 	/* Wait for more output. */
 	return start_json_stream(conn, jcon);
 }
@@ -761,7 +784,6 @@ static struct io_plan *jcon_connected(struct io_conn *conn,
 	jcon->ld = ld;
 	jcon->used = 0;
 	jcon->buffer = tal_arr(jcon, char, 64);
-	jcon->stop = false;
 	jcon->js_arr = tal_arr(jcon, struct json_stream *, 0);
 	jcon->len_read = 0;
 	list_head_init(&jcon->commands);

lightningd/lightningd.c

@@ -217,6 +217,9 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	 */
 	ld->plugins = plugins_new(ld, ld->log_book, ld);
 
+	/*~ This is set when a JSON RPC command comes in to shut us down. */
+	ld->stop_conn = NULL;
+
 	return ld;
 }
 
@@ -615,6 +618,8 @@ int main(int argc, char *argv[])
 	struct lightningd *ld;
 	u32 min_blockheight, max_blockheight;
 	int connectd_gossipd_fd, pid_fd;
+	int stop_fd;
+	const char *stop_response;
 
 	/*~ What happens in strange locales should stay there. */
 	setup_locale();
@@ -826,6 +831,11 @@ int main(int argc, char *argv[])
 	 */
 	assert(io_loop_ret == ld);
 
+	/* Keep this fd around, to write final response at the end. */
+	stop_fd = io_conn_fd(ld->stop_conn);
+	io_close_taken_fd(ld->stop_conn);
+	stop_response = tal_steal(NULL, ld->stop_response);
+
 	shutdown_subdaemons(ld);
 
 	tal_free(ld->plugins);
@@ -848,6 +858,11 @@ int main(int argc, char *argv[])
 
 	daemon_shutdown();
 
+	/* Finally, send response to shutdown command */
+	write_all(stop_fd, stop_response, strlen(stop_response));
+	close(stop_fd);
+	tal_free(stop_response);
+
 	/*~ Farewell.  Next stop: hsmd/hsmd.c. */
 	return 0;
 }

lightningd/lightningd.h

@@ -185,6 +185,11 @@ struct lightningd {
 	/* If we want to debug a subdaemon/plugin. */
 	const char *dev_debug_subprocess;
 
+	/* RPC which asked us to shutdown, if non-NULL */
+	struct io_conn *stop_conn;
+	/* RPC response to send once we've shut down. */
+	const char *stop_response;
+
 #if DEVELOPER
 	/* If we have a --dev-disconnect file */
 	int dev_disconnect_fd;