Repository navigation
Conversation
…g down bitcoind->pluginsmap keeps a pointer to the backend plugin for each of its methods. When the plugin exits during shutdown, bitcoin_destructor() lets it go, but the map still points at the freed plugin, so anything that asks the backend after that uses freed memory. 0b8239f ("lightningd: fix access to bcli plugin once it's freed.") stopped gossipd's get_txout from doing that. A channel that fails during shutdown does it too, through drop_to_chain(): lightningd: FATAL SIGNAL 11 (version v26.06.9) plugin_send <- plugin_request_send <- bitcoin_plugin_send <- bitcoind_sendrawtx_ <- broadcast_tx_ <- sign_and_send_last <- drop_to_chain <- channel_fail_perm <- channel_fail_permanent <- channel_errmsg <- channel_control_errmsg <- handle_peer_error <- sd_msg_read <- ... <- io_loop <- shutdown_plugins <- main There, connectd reconnected to the peer while shutdown_plugins() waited for the plugins to exit, lightningd started channeld for the channel, and the peer answered the reestablish with an error. So lightningd failed the channel and broadcast its commitment tx through the freed bcli. Check in bitcoin_plugin_send() instead, so no caller can get there, and send estimatefees through it too. Not broadcasting the close is safe: the channel is AWAITING_UNILATERAL in the db, and resend_closing_transactions() broadcasts it on restart. Fixes: ElementsProject#9719 Changelog-Fixed: lightningd: no longer crashes when a channel is force-closed while plugins shut down (e.g. a peer reconnects and sends an error); the close is broadcast on restart.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:
tools/lightning-downgradeFixes #9719.
At shutdown, the backend plugin (bcli) can exit and be freed while
bitcoind->pluginsmapstill points at it. #8687 stopped gossipd'sget_txout()from using it at that point. A channel that fails during shutdown uses it too:drop_to_chain()broadcasts the commitment tx throughbitcoin_plugin_send(), and lightningd crashes inplugin_send()(backtrace in #9719).With this PR,
bitcoin_plugin_send()sends nothing onceLD_STATE_SHUTDOWNis set, andestimatefees, which calledplugin_request_send()directly, goes through it too. Callers get no callback. That's fine at shutdown:stop_topology()already drops the answers to topology's requests. A close we don't broadcast is broadcast on restart byresend_closing_transactions(), because the channel is alreadyAWAITING_UNILATERALin the db. The check #8687 added inget_txout()stays, and now only saves starting the request.One behaviour change: a plugin that uses the backend from its shutdown handler, over an RPC connection it already has open, gets no answer and is killed after 30 seconds. Today that works only if bcli hasn't exited yet; otherwise it reads freed memory.
The new test makes a peer that forgot the channel reconnect while lightningd waits for a plugin that ignores
shutdown. With this PR, lightningd fails the channel, doesn't broadcast, exits cleanly, and broadcasts the close after the restart. On master it aborts inplugin_send().It doesn't stop connectd from reconnecting during shutdown, which is how a channeld gets started after
subd_shutdown_nonglobals(). See #9719.