Skip to content

Commit

Permalink
lightningd: free timers on shutdown.
Browse files Browse the repository at this point in the history
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 <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell committed Jun 30, 2019
1 parent fc6de94 commit ddc44a1
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 13 deletions.
2 changes: 1 addition & 1 deletion lightningd/bitcoind.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion lightningd/connect_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion lightningd/io_loop_with_timers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
4 changes: 2 additions & 2 deletions lightningd/jsonrpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down
14 changes: 11 additions & 3 deletions lightningd/lightningd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lightningd/lightningd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lightningd/memdump.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
2 changes: 1 addition & 1 deletion lightningd/pay.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion lightningd/peer_htlcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit ddc44a1

Please sign in to comment.