From c6ec9443b9929c1f43a3688ad8d66bbf9e3298f3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 12 Jun 2019 10:08:55 +0930 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + lightningd/jsonrpc.c | 50 +++++++++++++++++++++++++++++------------ lightningd/lightningd.c | 15 +++++++++++++ lightningd/lightningd.h | 5 +++++ 4 files changed, 57 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cee9a30d0..d4b6157d3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 4fb82d2ec..6af8032ce 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -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); diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 214346943..d144dec99 100644 --- a/lightningd/lightningd.c +++ b/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; } diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index bb28e8659..0855431b1 100644 --- a/lightningd/lightningd.h +++ b/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;