diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index cd0b3caf3..a89516554 100644 --- a/lightningd/jsonrpc.c +++ b/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; } - diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 8ea916ec9..3128fb6a5 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -462,6 +462,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); diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 1fe8fb3f4..b164eb757 100644 --- a/lightningd/lightningd.h +++ b/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. */ diff --git a/tests/test_rpc.py b/tests/test_rpc.py index 06020d848..ab57dc0a7 100644 --- a/tests/test_rpc.py +++ b/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 @@ with open('config.vars') as configfile: 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()