From 8c084d57ff7b9ef03857bf89a56525e18a910a0a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 12 Feb 2018 20:43:04 +1030 Subject: [PATCH] lightningd: channels own the peer. Channels are within the peer structure, but the peer is freed only when the last channel is freed. We also implement channel_set_owner() and make peer_set_owner() a temporary wrapper. Signed-off-by: Rusty Russell --- lightningd/channel.c | 28 ++++++++++++++++++++++++++++ lightningd/channel.h | 4 ++++ lightningd/lightningd.c | 14 ++++++++++++-- lightningd/peer_control.c | 35 +++++++++-------------------------- 4 files changed, 53 insertions(+), 28 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 99007e796..1b380bf2b 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -9,9 +9,37 @@ #include #include +void channel_set_owner(struct channel *channel, struct subd *owner) +{ + struct subd *old_owner = channel->owner; + channel->owner = owner; + + if (old_owner) + subd_release_peer(old_owner, channel2peer(channel)); +} + static void destroy_channel(struct channel *channel) { + /* Free any old owner still hanging around. */ + channel_set_owner(channel, NULL); + list_del_from(&channel->peer->channels, &channel->list); + + /* Last one out frees the peer */ + if (list_empty(&channel->peer->channels)) + tal_free(channel->peer); +} + +/* This lets us give a more detailed error than just a destructor. */ +void free_channel(struct channel *channel, const char *why) +{ + if (channel->opening_cmd) { + command_fail(channel->opening_cmd, "%s", why); + channel->opening_cmd = NULL; + } + wallet_channel_delete(channel->peer->ld->wallet, channel->dbid, + channel->peer->dbid); + tal_free(channel); } /* FIXME: We have no business knowing this! */ diff --git a/lightningd/channel.h b/lightningd/channel.h index d774c2844..0445d30a7 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -86,9 +86,13 @@ struct channel { }; struct channel *new_channel(struct peer *peer, u64 dbid, u32 first_blocknum); +/* This lets us give a more detailed error than just a destructor. */ +void free_channel(struct channel *channel, const char *why); const char *channel_state_name(const struct channel *channel); +void channel_set_owner(struct channel *channel, struct subd *owner); + void derive_channel_seed(struct lightningd *ld, struct privkey *seed, const struct pubkey *peer_id, const u64 dbid); diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 69644a731..be37a95c2 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -199,8 +199,18 @@ static void shutdown_subdaemons(struct lightningd *ld) free_htlcs(ld, NULL); - while ((p = list_top(&ld->peers, struct peer, list)) != NULL) - tal_free(p); + while ((p = list_top(&ld->peers, struct peer, list)) != NULL) { + struct channel *c, *next; + + /* Careful: deleting last channel frees p! */ + do { + c = list_top(&p->channels, struct channel, list); + next = list_next(&p->channels, c, list); + + tal_free(c); + c = next; + } while (c); + } db_commit_transaction(ld->wallet->db); } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 7736bd03e..0dbe92930 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -91,12 +91,7 @@ static void peer_accept_channel(struct lightningd *ld, static void peer_set_owner(struct peer *peer, struct subd *owner) { - struct channel *channel = peer2channel(peer); - struct subd *old_owner = channel->owner; - channel->owner = owner; - - if (old_owner) - subd_release_peer(old_owner, peer); + channel_set_owner(peer2channel(peer), owner); } static void destroy_peer(struct peer *peer) @@ -127,8 +122,6 @@ static void destroy_peer(struct peer *peer) htlc_state_name(hin->hstate)); } - /* Free any old owner still hanging around. */ - peer_set_owner(peer, NULL); list_del_from(&peer->ld->peers, &peer->list); } @@ -136,7 +129,8 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid, const struct pubkey *id, const struct wireaddr *addr) { - struct peer *peer = tal(ld, struct peer); + /* We are owned by our channels, and freed manually by destroy_channel */ + struct peer *peer = tal(NULL, struct peer); const char *idname; peer->ld = ld; @@ -222,17 +216,6 @@ static void drop_to_chain(struct peer *peer) remove_sig(peer2channel(peer)->last_tx); } -/* This lets us give a more detailed error than just a destructor. */ -static void free_peer(struct peer *peer, const char *why) -{ - if (peer2channel(peer)->opening_cmd) { - command_fail(peer2channel(peer)->opening_cmd, "%s", why); - peer2channel(peer)->opening_cmd = NULL; - } - wallet_channel_delete(peer->ld->wallet, peer2channel(peer)->dbid, peer->dbid); - tal_free(peer); -} - void peer_fail_permanent(struct peer *peer, const char *fmt, ...) { va_list ap; @@ -272,7 +255,7 @@ void peer_fail_permanent(struct peer *peer, const char *fmt, ...) drop_to_chain(peer); tal_free(why); } else - free_peer(peer, why); + free_channel(channel, why); } void peer_internal_error(struct peer *peer, const char *fmt, ...) @@ -313,7 +296,7 @@ void peer_fail_transient(struct peer *peer, const char *fmt, ...) if (!peer_persists(peer)) { log_info(peer->log, "Only reached state %s: forgetting", channel_state_name(peer2channel(peer))); - free_peer(peer, why); + free_channel(peer2channel(peer), why); return; } tal_free(why); @@ -511,7 +494,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg, /* Reconnect: discard old one. */ case OPENINGD: - free_peer(peer, "peer reconnected"); + free_channel(channel, "peer reconnected"); peer = NULL; goto return_to_gossipd; @@ -1250,7 +1233,7 @@ static void handle_irrevocably_resolved(struct peer *peer, const u8 *msg) log_info(peer->log, "onchaind complete, forgetting peer"); /* This will also free onchaind. */ - free_peer(peer, "onchaind complete, forgetting peer"); + free_channel(peer2channel(peer), "onchaind complete, forgetting peer"); } /** @@ -2428,7 +2411,7 @@ static unsigned int opening_negotiation_failed(struct subd *openingd, log_unusual(peer->log, "Opening negotiation failed: %s", why); /* This will free openingd, since that's peer->owner */ - free_peer(peer, why); + free_channel(peer2channel(peer), why); return 0; } @@ -2979,7 +2962,7 @@ static void process_dev_forget_channel(struct bitcoind *bitcoind UNUSED, json_add_txid(response, "funding_txid", peer2channel(forget->peer)->funding_txid); json_object_end(response); - free_peer(forget->peer, "dev-forget-channel called"); + free_channel(peer2channel(forget->peer), "dev-forget-channel called"); command_success(forget->cmd, response); }