Repository navigation
IPC segfault with 32345 + 29409 + custom client #176
Description
Originally posted by @darosior in bitcoin/bitcoin#32345 (comment)
I tested this PR by rebasing #29409 on top of it and running my (updated) core_bdk_wallet against it.
I found that bitcoin-node will (still) segfault if i try to stop it while the client is running. The steps i followed were to 1) start bitcoin-node 2) start my PoC which will connect to it and stall with the open connection for 6 seconds 3) before the 6 seconds have elapsed, stop the bitcoin-node process (in this specific case by sending it a SIGINT). Here are the logs of the bitcoin-node process:
Click for `bitcoin-node` logs before segfault
[...]
2025-05-20T18:44:44Z init message: Starting network threads…
2025-05-20T18:44:44Z DNS seeding disabled
2025-05-20T18:44:44Z init message: Done loading
2025-05-20T18:44:44Z initload thread start
2025-05-20T18:44:44Z msghand thread start
2025-05-20T18:44:44Z Loading 0 mempool transactions from file...
2025-05-20T18:44:44Z Imported mempool transactions from file: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial broadcast
2025-05-20T18:44:44Z opencon thread start
2025-05-20T18:44:44Z addcon thread start
2025-05-20T18:44:44Z net thread start
2025-05-20T18:44:44Z initload thread exit
2025-05-20T18:44:44Z New manual v2 peer connected: version: 70016, blocks=113, peer=0
2025-05-20T18:44:44Z Synchronizing blockheaders, height: 113 (~0.47%)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #1 Init.construct$Params ()
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #1 Init.construct$Results (threadMap = <external capability>)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #2 Init.makeChain$Params (context = (thread = <external capability>))
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request #2 {bitcoin-node-948798/b-capnp-loop-948823 (from )}
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #2 Init.makeChain$Results (result = <external capability>)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #3 Chain.initMessage$Params (context = (thread = <external capability>), message = "Oxydation of the Bitcoin Core wallet in progress..")
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request #3 {bitcoin-node-948798/b-capnp-loop-948823 (from )}
2025-05-20T18:44:45Z init message: Oxydation of the Bitcoin Core wallet in progress..
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #3 Chain.initMessage$Results ()
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #4 Chain.showProgress$Params (context = (thread = <external capability>), title = "BDK Core startup", progress = 1, resumePossible = false)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request #4 {bitcoin-node-948798/b-capnp-loop-948823 (from )}
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #4 Chain.showProgress$Results ()
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #5 Chain.getHeight$Params (context = (thread = <external capability>))
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request #5 {bitcoin-node-948798/b-capnp-loop-948823 (from )}
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #5 Chain.getHeight$Results (result = 113, hasResult = true)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #6 Chain.getBlockHash$Params (context = (thread = <external capability>), height = 113)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request #6 {bitcoin-node-948798/b-capnp-loop-948823 (from )}
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #6 Chain.getBlockHash$Results (result = "H\\3678\\307#\\020|\\020\\017\\340\\2469\\330\\356\\335\\277\\341dOB{\\220\\327\\354[\\361\\036\\233\\301\\035DD")
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #7 Chain.showProgress$Params (context = (thread = <external capability>), title = "BDK Core startup", progress = 100, resumePossible = true)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request #7 {bitcoin-node-948798/b-capnp-loop-948823 (from )}
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #7 Chain.showProgress$Results ()
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request #8 Chain.handleNotifications$Params (context = (thread = <external capability>), notifications = <external capability>)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request #8 {bitcoin-node-948798/b-capnp-loop-948823 (from )}
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response #8 Chain.handleNotifications$Results (result = <external capability>)
2025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server destroy N2mp11ProxyServerIN3ipc5capnp8messages7HandlerEEE
^C2025-05-20T18:44:47Z Shutdown: In progress...
2025-05-20T18:44:47Z addcon thread exit
2025-05-20T18:44:47Z opencon thread exit
2025-05-20T18:44:47Z net thread exit
2025-05-20T18:44:47Z msghand thread exit
2025-05-20T18:44:47Z scheduler thread exit
2025-05-20T18:44:47Z Writing 0 mempool transactions to file...
2025-05-20T18:44:47Z Writing 0 unbroadcast transactions to file.
2025-05-20T18:44:47Z Dumped mempool: 0.000s to copy, 0.003s to dump, 27 bytes dumped to file
2025-05-20T18:44:47Z Flushed fee estimates to fee_estimates.dat.
2025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-shutoff-948798} IPC client first request from current thread, constructing waiter
2025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-shutoff-948798} IPC client send ChainNotifications.chainStateFlushed$Params (context = (thread = <external capability>, callbackThread = <external capability>), role = 0, locator = "\\200\\021\\001\\000\\022H\\3678\\307#\\020|\\020\\017\\340\\2469\\330\\356\\335\\277\\341dOB{\\220\\327\\354[\\361\\036\\233\\301\\035DD\\tGm\\237]YbZDV9\\254\\252f\\205\\001\\323K\\375!\\340\\t\\367\\375\\346\\264W\\207\\000\\2101\\035:\\314\\233\\246\\275\\207\\v\\\\\\\\\\343\\202\\026\\274\\037\\213\\314\\303V\\2116<*+\\352\\255\\026\\204M\\332\\340\\nT\\306\\360Pa\\304\\002\\024\\356\\247\\220\\263\\257l\\230\\350\\362\\027Yl\\300\\316lk\\232\\f\\333\\235,\\234\\336\\204+JE\\263>\\237\\177\\305z\\017\\252\\311@\\341e\\026\\300dk\\305\\212V\\334\\377\\004cb\\217p\\n\\307w\\0046\\2261|\\024\\\\N\\336\\223G\\263~n\\271\\256\\264\\346\\227\\301\\032\\301\\030\\bw\\241Z.\\302\\261\\027 \\005\\374B\\357R\\231\\237Ct\\324\\232I\\322\\370\\303T\\253\\205\\354\\233\\337\\031C\\3215O\\3746\\016\\360b &\\300\\201\\244U\\221\\306 Pql+c\\212\\2524\\314a\\t\\216\\275=\\230b\\207ouv[\\365(\\3...
2025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-shutoff-948798} IPC client recv ChainNotifications.chainStateFlushed$Results ()
2025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server destroy N2mp11ProxyServerIN3ipc5capnp8messages5ChainEEE
2025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server destroy N2mp11ProxyServerIN3ipc5capnp8messages4InitEEE
Segmentation fault
Activity
fail to reproduce segfault reported bitcoin-core/libmultiprocess#176
ryanofsky commentedon May 21, 2025
So far could not reproduce this combining bitcoin/bitcoin#32345 + bitcoin/bitcoin#29409 + bitcoin/bitcoin#30437 and shutting down node with control-c while client is still connected.
Code tested was: ryanofsky/bitcoin@42bf3ec (branch) and testing steps are in the commit description
One difference between the client I tested with and the custom client reported in this bug is that apparently from the logs the custom client is calling
Chain.handleNotifcationsand registering for notifications. So maybe the node is running into problems if the notificationsChainNotificationsobject is being destroyed too late, or something like that.It would help to have a stack trace from the segfault. It should be easy to get a stack trace by running the node with:
and typing
bt(backtrace) after the segfault happensdarosior commentedon May 21, 2025
Here is the backtrace
Logs when sending `SIGINT` from withing gdb + backtrace
ryanofsky commentedon May 21, 2025
Thanks! I also just reproduced this and got a similar backtrace with: ryanofsky/bitcoin@7d6a108 (branch)
ryanofsky commentedon May 21, 2025
So commit ryanofsky/bitcoin@c93c985 (branch) fixes the problem for me.
It should be possible to cherry-pick it or just apply the diff https://github.com/ryanofsky/bitcoin/commit/c93c9852d6408a3022e346f457c730c08c175b02.diff to test.
I'm not very satisfied with this fix because it is not really compatible with the way bitcoin-gui works in bitcoin/bitcoin#10102 (details in commit description), but assuming it solves current problems, it is a clean fix.
darosior commentedon May 21, 2025
Can confirm it's fixed. Are we sure it only happens in this specific instance and there should be no other way the order of destruction can lead to a crash? Can i continue testing with this commit applied or should i wait till you've updated your PR?
ryanofsky commentedon May 21, 2025
This fix should be a pretty general fix because it just waits for all objects associated with any IPC connection to be destroyed before freeing resources they could be referencing. So I think if you continued to test it you would not find similar problems. To be clear, IPC connections using the
Chaininterface are pretty trusted, so there are probably other things IPC clients could do using it to crash the node. But the fix should address any shutdown issues.I'm not sure yet if I will update bitcoin/bitcoin#32345 with this specific fix or a more general one that could work better with later changes in bitcoin/bitcoin#10102.
The problem in bitcoin/bitcoin#10102 is that
bitcoin-nodecan be spawned by a parentbitcoin-guiprocess there. To work correctly, it should be able to keep its connection to thebitcoin-guiprocess alive while destroying other connections. I think to allow this,Connectionobjects in libmultiprocess could have a newbool m_controllingfield indicating whether they are "controlling" connections that should not be disconnected. Then in addition to theEventLoop::m_num_cientsreference count incremented by normal connections, there could be anEventLoop::m_num_cients_controllingreference count incremented by controlling connections. Then, whenbitcoin-nodeis shutting down, it could disconnect all non-controlling connections, wait form_num_cients == m_num_cients_controllingto be true, free all resources the non-controlling connections could have been referencing, and the finally disconnect the controlling connection. Implementing this should not be too complicated and it should be behave same way as the current fix, while being compatible with bitcoin/bitcoin#10102ryanofsky commentedon May 22, 2025
Thinking about it more, a different and more general approach would probably be simpler.
Instead of creating a distinction between controlling and non-controlling connections, and adding way to wait for objects associated with non-controlling connections to be freed after they are disconnected, it would make more sense to just add a general way to wait for objects associated with any connection to be freed after disconnection.
This should be pretty straightforward to implement. Instead of having
EventLoop::m_num_clientsandEventLoop::m_num_clients_controllingreference counts there could beEventLoop::m_num_clientsandConnection::m_num_clientsreference counts so objects associated with each connection are tracked separately. This will require another change to makeConnectionobjects outlive all the client and server objects associated with them which is not currently the case. But that should not be too hard to implement.fail to reproduce segfault reported bitcoin-core/libmultiprocess#176
ryanofsky commentedon Jun 5, 2025
Thanks for your patience. I went on a real journey with this implementing the connection refcounting idea described above before figuring out that a much simpler fix is possible, because cap'n proto provides better guarantees about when objects will be destroyed than I was assuming.
I updated bitcoin/bitcoin#32345 with the simpler fix, and fix itself is commit ryanofsky/bitcoin@94db353 (branch).
That branch also contains combined code from bitcoin/bitcoin#32345, bitcoin/bitcoin#29409, and bitcoin/bitcoin#30437 which I was using to test this issue.
The unused code I had written to the implement the connection refcounting idea is ryanofsky@a35138a and I will probably try to port over some of the documentation and organizational improvements it makes in a separate PR. I might keep the refcounting implementation too since in some ways it makes code simpler, and having separate connection counts instead of a global count could allow adding more asserts and make problems easier to debug.
Update: I wound up doing work on the connection refcounting idea to fix another problem in #182, before abandoning it again after finding a simpler solution to that problem as well. Current implementation is c3a85ae (tag) and I will probably clean it up and turn it into a followup PR
Update: Latest iteration of the connection refcounting idea is #336
Reproduce IPC looping connect & shutdown bug
Merge #160: refactor: EventLoop locking cleanups + client disconnect …
Merge #32345: ipc: Handle unclean shutdowns better
ryanofsky commentedon Aug 23, 2025
The fix above ryanofsky/bitcoin@94db353 was merged in bitcoin/bitcoin#32345
Merge bitcoin/bitcoin#32345: ipc: Handle unclean shutdowns better
Merge bitcoin/bitcoin#32345: ipc: Handle unclean shutdowns better
Merge bitcoin/bitcoin#32345: ipc: Handle unclean shutdowns better