From 5148fcaeed1fbb13a367bbdea4e5e71b674de3ac Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 24 Jul 2023 12:53:26 +0930 Subject: [PATCH] lightningd: fix false memleak report (test flake)! We get intermittant reports of subd->conn being leaked, but I could never find it. That's because it's actually subd which is not referenced any more: subd->conn gets reported because it's subd's tal_parent (and, except for the reference in subd, not referenced either). The real issue is that the channel->owner is reassigned to the new subdaemon, and the old one is still exiting. During that time, we can see a "leak". ``` - Node /tmp/ltests-hkr089bp/test_sql_1/lightning-3/ has memory leaks: [ { "backtrace": [ "ccan/ccan/tal/tal.c:477 (tal_alloc_)", "ccan/ccan/io/io.c:91 (io_new_conn_)", "lightningd/subd.c:774 (new_subd)", "lightningd/subd.c:828 (new_channel_subd_)", "lightningd/dual_open_control.c:3662 (peer_restart_dualopend)", "lightningd/peer_control.c:1161 (connect_activate_subd)", "lightningd/peer_control.c:1273 (peer_connected_hook_final)", "lightningd/plugin_hook.c:213 (plugin_hook_callback)", "lightningd/plugin.c:591 (plugin_response_handle)", "lightningd/plugin.c:702 (plugin_read_json_one)", "lightningd/plugin.c:747 (plugin_read_json)", "ccan/ccan/io/io.c:59 (next_plan)", "ccan/ccan/io/io.c:407 (do_plan)", "ccan/ccan/io/io.c:417 (io_ready)", "ccan/ccan/io/poll.c:453 (io_loop)", "lightningd/io_loop_with_timers.c:22 (io_loop_with_timers)", "lightningd/lightningd.c:1249 (main)" ], "label": "ccan/ccan/io/io.c:91:struct io_conn", "parents": [ "lightningd/lightningd.c:107:struct lightningd" ], "value": "0x556c63c859f8" } ``` Signed-off-by: Rusty Russell --- lightningd/subd.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightningd/subd.c b/lightningd/subd.c index 9071f4c90..9e785e90b 100644 --- a/lightningd/subd.c +++ b/lightningd/subd.c @@ -921,6 +921,11 @@ void subd_release_channel(struct subd *owner, const void *channel) assert(owner->channel == channel); owner->channel = NULL; tal_free(owner); + } else { + /* Caller has reassigned channel->owner, so there's no pointer + * to this subd owner while it's freeing itself. If we + * ask memleak right now, it will complain! */ + notleak(owner); } }