From 90b669857e5bea26b2bd192386829a75b18a11f5 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 28 Dec 2021 09:51:09 +1030 Subject: [PATCH] lightningd: handle channel cleanups more explicitly. 1. Freeing an unconfirmed channel already releases the subd, so don't do that explicitly. 2. Use channel->owner to transfer ownership where possible, using channel_set_owner() which handles all the cases. This simplifies the code and makes it more readable, IMHO. Signed-off-by: Rusty Russell --- lightningd/channel.c | 2 +- lightningd/opening_common.c | 9 +++------ lightningd/opening_control.c | 10 ++-------- lightningd/subd.c | 2 +- lightningd/subd.h | 5 +++-- wallet/test/run-wallet.c | 2 +- 6 files changed, 11 insertions(+), 19 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index e0c1f8779..f8e7972dd 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -826,7 +826,7 @@ void channel_internal_error(struct channel *channel, const char *fmt, ...) channel_cleanup_commands(channel, why); if (channel_unsaved(channel)) { - subd_release_channel(channel->owner, channel); + channel_set_owner(channel, NULL); delete_channel(channel); tal_free(why); return; diff --git a/lightningd/opening_common.c b/lightningd/opening_common.c index 635aaeef5..0b02b41d5 100644 --- a/lightningd/opening_common.c +++ b/lightningd/opening_common.c @@ -17,8 +17,9 @@ static void destroy_uncommitted_channel(struct uncommitted_channel *uc) { - if (uc->open_daemon) { - struct subd *open_daemon= uc->open_daemon; + struct subd *open_daemon = uc->open_daemon; + + if (open_daemon) { uc->open_daemon = NULL; subd_release_channel(open_daemon, uc); } @@ -115,10 +116,6 @@ void kill_uncommitted_channel(struct uncommitted_channel *uc, { log_info(uc->log, "Killing opening daemon: %s", why); - /* Close opend daemon. */ - subd_release_channel(uc->open_daemon, uc); - uc->open_daemon = NULL; - uncommitted_channel_disconnect(uc, LOG_INFORM, why); tal_free(uc); } diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index dd378219f..9e78a6963 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -319,9 +319,7 @@ static void opening_funder_start_replied(struct subd *openingd, const u8 *resp, return; failed: - subd_release_channel(openingd, fc->uc); - fc->uc->open_daemon = NULL; - /* Frees fc too, and tmpctx */ + /* Frees fc too */ tal_free(fc->uc); } @@ -414,9 +412,7 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, peer_start_channeld(channel, pps, NULL, false, NULL); cleanup: - subd_release_channel(openingd, fc->uc); - fc->uc->open_daemon = NULL; - /* Frees fc too, and tmpctx */ + /* Frees fc too */ tal_free(fc->uc); } @@ -528,8 +524,6 @@ static void opening_fundee_finished(struct subd *openingd, /* On to normal operation! */ peer_start_channeld(channel, pps, fwd_msg, false, NULL); - subd_release_channel(openingd, uc); - uc->open_daemon = NULL; tal_free(uc); return; diff --git a/lightningd/subd.c b/lightningd/subd.c index bebd5adc1..a42849b48 100644 --- a/lightningd/subd.c +++ b/lightningd/subd.c @@ -888,7 +888,7 @@ struct subd *subd_shutdown(struct subd *sd, unsigned int seconds) return tal_free(sd); } -void subd_release_channel(struct subd *owner, void *channel) +void subd_release_channel(struct subd *owner, const void *channel) { /* If owner is a per-peer-daemon, and not already freeing itself... */ if (owner->channel) { diff --git a/lightningd/subd.h b/lightningd/subd.h index 94fabf9e3..73b9534bd 100644 --- a/lightningd/subd.h +++ b/lightningd/subd.h @@ -197,9 +197,10 @@ void subd_req_(const tal_t *ctx, * @channel: channel to release. * * If the subdaemon is not already shutting down, and it is a per-channel - * subdaemon, this shuts it down. + * subdaemon, this shuts it down. Don't call this directly, use + * channel_set_owner() or uncommitted_channel_release_subd(). */ -void subd_release_channel(struct subd *owner, void *channel); +void subd_release_channel(struct subd *owner, const void *channel); /** * subd_shutdown - try to politely shut down a subdaemon. diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 01348b60c..52617a9e3 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -654,7 +654,7 @@ u8 *serialize_onionpacket( const struct onionpacket *packet UNNEEDED) { fprintf(stderr, "serialize_onionpacket called!\n"); abort(); } /* Generated stub for subd_release_channel */ -void subd_release_channel(struct subd *owner UNNEEDED, void *channel UNNEEDED) +void subd_release_channel(struct subd *owner UNNEEDED, const void *channel UNNEEDED) { fprintf(stderr, "subd_release_channel called!\n"); abort(); } /* Generated stub for subd_req_ */ void subd_req_(const tal_t *ctx UNNEEDED,