json-rpc: Shutdown the JSON-RPC in the context of a DB transaction

This needs to be done separately from the rest of the daemon since we can
otherwise not make sure that it happens before the DB is freed and we might
still need the DN, and be running in a DB transaction, for some destructors to
run.

Author: cdecker

lightningd/jsonrpc.c

@@ -447,7 +447,7 @@ static void parse_request(struct json_connection *jcon, const jsmntok_t tok[])
 
 	/* This is a convenient tal parent for duration of command
 	 * (which may outlive the conn!). */
-	c = tal(jcon->ld, struct command);
+	c = tal(jcon->ld->rpc_listener, struct command);
 	c->jcon = jcon;
 	c->ld = jcon->ld;
 	c->pending = false;
@@ -650,8 +650,7 @@ void setup_jsonrpc(struct lightningd *ld, const char *rpc_filename)
 		err(1, "Listening on '%s'", rpc_filename);
 
 	log_debug(ld->log, "Listening on '%s'", rpc_filename);
-	/* Technically this is a leak, but there's only one */
-	notleak(io_new_listener(ld, fd, incoming_jcon_connected, ld));
+	ld->rpc_listener = io_new_listener(ld->rpc_filename, fd, incoming_jcon_connected, ld);
 }
 
 /**
@@ -790,4 +789,3 @@ bool json_tok_wtx(struct wallet_tx * tx, const char * buffer,
         }
         return true;
 }
-

lightningd/lightningd.c

@@ -455,6 +455,14 @@ int main(int argc, char *argv[])
 	}
 
 	shutdown_subdaemons(ld);
+
+	/* 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.,
+	 * unreserving UTXOs (see #1737) */
+	db_begin_transaction(ld->wallet->db);
+	tal_free(ld->rpc_listener);
+	db_commit_transaction(ld->wallet->db);
+
 	close(pid_fd);
 	remove(ld->pidfile);
 

lightningd/lightningd.h

@@ -78,8 +78,15 @@ struct lightningd {
 
 	/* Our config dir, and rpc file */
 	char *config_dir;
+
+	/* Location of the RPC socket. */
 	char *rpc_filename;
 
+	/* The listener for the RPC socket. Can be shut down separately from the
+	 * rest of the daemon to allow a clean shutdown, which frees all pending
+	 * cmds in a DB transaction. */
+	struct io_listener *rpc_listener;
+
 	/* Configuration file name */
 	char *config_filename;
 	/* Configuration settings. */

tests/test_rpc.py

@@ -1,7 +1,6 @@
 from fixtures import *  # noqa: F401,F403
 
 import os
-import pytest
 import signal
 import unittest
 
@@ -11,13 +10,13 @@
 DEVELOPER = os.getenv("DEVELOPER", config['DEVELOPER']) == "1"
 
 
-@pytest.mark.xfail(strict=True)
 @unittest.skipIf(not DEVELOPER, "needs --dev-disconnect")
 def test_stop_pending_fundchannel(node_factory, executor):
     """Stop the daemon while waiting for an accept_channel
 
-    This used to crash the node, since we were calling unreserve_utxo while freeing
-    the daemon, but that needs a DB transaction to be open.
+    This used to crash the node, since we were calling unreserve_utxo while
+    freeing the daemon, but that needs a DB transaction to be open.
+
     """
     l1 = node_factory.get_node()
     l2 = node_factory.get_node()