From d5bd1682f5cde03e9ad5c16cf526ac937cbdee27 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Jun 2019 13:19:23 +0930 Subject: [PATCH] lightningd: free timers on shutdown. Direct leak of 1024 byte(s) in 2 object(s) allocated from: #0 0x7f4c84ce4448 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10c448) #1 0x55d11b782c96 in timer_default_alloc ccan/ccan/timer/timer.c:16 #2 0x55d11b7832b7 in add_level ccan/ccan/timer/timer.c:166 #3 0x55d11b783864 in timer_fast_forward ccan/ccan/timer/timer.c:334 #4 0x55d11b78396a in timers_expire ccan/ccan/timer/timer.c:359 #5 0x55d11b774993 in io_loop ccan/ccan/io/poll.c:395 #6 0x55d11b72322f in plugins_init lightningd/plugin.c:1013 #7 0x55d11b7060ea in main lightningd/lightningd.c:664 #8 0x7f4c84696b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a) To fix this, we actually make 'ld->timers' a pointer, so we can clean it up last of all. We can't free it before ld, because that causes timers to be destroyed. Signed-off-by: Rusty Russell --- lightningd/bitcoind.c | 2 +- lightningd/connect_control.c | 2 +- lightningd/io_loop_with_timers.c | 2 +- lightningd/jsonrpc.c | 4 ++-- lightningd/lightningd.c | 14 +++++++++++--- lightningd/lightningd.h | 2 +- lightningd/memdump.c | 2 +- lightningd/pay.c | 2 +- lightningd/peer_control.c | 2 +- lightningd/peer_htlcs.c | 2 +- 10 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index f6875291302e..b59e34392748 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -161,7 +161,7 @@ static void bcli_failure(struct bitcoind *bitcoind, bitcoind->error_count++; /* Retry in 1 second (not a leak!) */ - new_reltimer(&bitcoind->ld->timers, notleak(bcli), time_from_sec(1), + new_reltimer(bitcoind->ld->timers, notleak(bcli), time_from_sec(1), retry_bcli, bcli); } diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index c2834da666c5..28951fcafff7 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -222,7 +222,7 @@ void delay_then_reconnect(struct channel *channel, u32 seconds_delay, /* We fuzz the timer by up to 1 second, to avoid getting into * simultanous-reconnect deadlocks with peer. */ - notleak(new_reltimer(&ld->timers, d, + notleak(new_reltimer(ld->timers, d, timerel_add(time_from_sec(seconds_delay), time_from_usec(pseudorand(1000000))), maybe_reconnect, d)); diff --git a/lightningd/io_loop_with_timers.c b/lightningd/io_loop_with_timers.c index be781d355d01..e1e9d7002fea 100644 --- a/lightningd/io_loop_with_timers.c +++ b/lightningd/io_loop_with_timers.c @@ -21,7 +21,7 @@ void *io_loop_with_timers(struct lightningd *ld) * It will only exit if there's an expired timer, *or* someone * calls io_break, or if there are no more file descriptors * (which never happens in our code). */ - retval = io_loop(&ld->timers, &expired); + retval = io_loop(ld->timers, &expired); /*~ Notice that timers are called here in the event loop like * anything else, so there are no weird concurrency issues. */ diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 6515bc87a6a6..5d81e3f7ff7c 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -265,7 +265,7 @@ static void slowcmd_finish(struct slowcmd *sc) static void slowcmd_start(struct slowcmd *sc) { sc->js = json_stream_success(sc->cmd); - new_reltimer(&sc->cmd->ld->timers, sc, time_from_msec(*sc->msec), + new_reltimer(sc->cmd->ld->timers, sc, time_from_msec(*sc->msec), slowcmd_finish, sc); } @@ -282,7 +282,7 @@ static struct command_result *json_slowcmd(struct command *cmd, NULL)) return command_param_failed(); - new_reltimer(&cmd->ld->timers, sc, time_from_msec(0), slowcmd_start, sc); + new_reltimer(cmd->ld->timers, sc, time_from_msec(0), slowcmd_start, sc); return command_still_pending(cmd); } diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 583726091ad4..61e4917c7824 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -189,7 +189,8 @@ static struct lightningd *new_lightningd(const tal_t *ctx) * ingenious bucket system which more precisely sorts timers as they * approach expiry. It's a fascinating implementation you should read * if you have a spare few hours. */ - timers_init(&ld->timers, time_mono()); + ld->timers = tal(ld, struct timers); + timers_init(ld->timers, time_mono()); /*~ This is detailed in chaintopology.c */ ld->topology = new_topology(ld, ld->log); @@ -619,6 +620,7 @@ int main(int argc, char *argv[]) u32 min_blockheight, max_blockheight; int connectd_gossipd_fd, pid_fd; int stop_fd; + struct timers *timers; const char *stop_response; /*~ What happens in strange locales should stay there. */ @@ -677,7 +679,7 @@ int main(int argc, char *argv[]) /*~ Our "wallet" code really wraps the db, which is more than a simple * bitcoin wallet (though it's that too). It also stores channel * states, invoices, payments, blocks and bitcoin transactions. */ - ld->wallet = wallet_new(ld, ld->log, &ld->timers); + ld->wallet = wallet_new(ld, ld->log, ld->timers); /*~ We keep a filter of scriptpubkeys we're interested in. */ ld->owned_txfilter = txfilter_new(ld); @@ -750,7 +752,7 @@ int main(int argc, char *argv[]) /*~ Initialize block topology. This does its own io_loop to * talk to bitcoind, so does its own db transactions. */ - setup_topology(ld->topology, &ld->timers, + setup_topology(ld->topology, ld->timers, min_blockheight, max_blockheight); /*~ Pull peers, channels and HTLCs from db. Needs to happen after the @@ -854,7 +856,13 @@ int main(int argc, char *argv[]) /* FIXME: pay can have children off tmpctx which unlink from * ld->payments, so clean that up. */ clean_tmpctx(); + + /* Free this last: other things may clean up timers. */ + timers = tal_steal(NULL, ld->timers); tal_free(ld); + + timers_cleanup(timers); + tal_free(timers); opt_free_table(); daemon_shutdown(); diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index 0855431b13db..9002e993a6f7 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -112,7 +112,7 @@ struct lightningd { u8 *rgb; /* tal_len() == 3. */ /* Any pending timers. */ - struct timers timers; + struct timers *timers; /* Port we're listening on */ u16 portnum; diff --git a/lightningd/memdump.c b/lightningd/memdump.c index c3dac0e545d6..fa3818258dc4 100644 --- a/lightningd/memdump.c +++ b/lightningd/memdump.c @@ -209,7 +209,7 @@ static void report_leak_info(struct command *cmd, struct subd *leaker) leak_info->leaker = leaker; /* Leak detection in a reply handler thinks we're leaking conn. */ - notleak(new_reltimer(&leak_info->cmd->ld->timers, leak_info->cmd, + notleak(new_reltimer(leak_info->cmd->ld->timers, leak_info->cmd, time_from_sec(0), report_leak_info2, leak_info)); } diff --git a/lightningd/pay.c b/lightningd/pay.c index 058343bb347f..2e162187467c 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -921,7 +921,7 @@ static struct command_result *json_waitsendpay(struct command *cmd, return res; if (timeout) - new_reltimer(&cmd->ld->timers, cmd, time_from_sec(*timeout), + new_reltimer(cmd->ld->timers, cmd, time_from_sec(*timeout), &waitsendpay_timeout, cmd); return command_still_pending(cmd); } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index c3a519e64f50..012ed8634612 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -336,7 +336,7 @@ register_close_command(struct lightningd *ld, tal_add_destructor2(channel, &destroy_close_command_on_channel_destroy, cc); - new_reltimer(&ld->timers, cc, time_from_sec(timeout), + new_reltimer(ld->timers, cc, time_from_sec(timeout), &close_command_timeout, cc); } diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index cc0362871cb9..ed685392e43c 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -456,7 +456,7 @@ enum onion_type send_htlc_out(struct channel *out, /* Give channel 30 seconds to commit (first) htlc. */ if (!out->htlc_timeout) - out->htlc_timeout = new_reltimer(&out->peer->ld->timers, + out->htlc_timeout = new_reltimer(out->peer->ld->timers, out, time_from_sec(30), htlc_offer_timeout, out);