ipc: Handle unclean shutdowns better #32345

pull ryanofsky wants to merge 8 commits into bitcoin:master from ryanofsky:pr/ipc-stop changing 77 files +1104 −471
  1. ryanofsky commented at 8:58 pm on April 24, 2025: contributor

    This PR fixes various problems when IPC connections are broken or hang which were reported in https://github.com/bitcoin-core/libmultiprocess/issues/123, https://github.com/bitcoin-core/libmultiprocess/issues/176, and https://github.com/bitcoin-core/libmultiprocess/pull/182. The different fixes are described in commit messages.


    The first two commits of this PR update the libmultiprocess subtree including the following PRs:

    The subtree changes can be verified by running test/lint/git-subtree-check.sh src/ipc/libmultiprocess as described in developer notes and lint instructions.

    The remaining commits are:

    The new commits depend on the subtree update, and because the subtree update includes an incompatible API change, the “Use EventLoopRef” commit needs to be part of the same PR to avoid breaking the build. The other commits also make sense to merge at the same time because the bitcoin & libmultiprocess changes were written and tested together.


    This PR is part of the process separation project.

  2. DrahtBot commented at 8:58 pm on April 24, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32345.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK darosior, pinheadmz

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #32427 ((RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store by TheCharlatan)
    • #31382 (kernel: Flush in ChainstateManager destructor by TheCharlatan)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. DrahtBot added the label CI failed on Apr 25, 2025
  4. DrahtBot commented at 0:33 am on April 25, 2025: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/41118314270

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  5. fanquake commented at 10:20 am on April 25, 2025: member

    https://github.com/bitcoin/bitcoin/actions/runs/14651550530/job/41119318297?pr=32345#step:5:966:

    0Assertion failed: fRPCInWarmup, file ./rpc/server.cpp, line 329
    1Error: Process completed with exit code 3.
    

    https://cirrus-ci.com/task/5477140512112640?logs=ci#L5037:

    0[17:33:37.941] ./test/node_init_tests.cpp(41): error: in "node_init_tests/init_test": check AppInitMain(m_node) has failed
    1[17:33:37.941] Error: Cannot resolve -bind address: ''
    2[17:33:37.954] ./test/node_init_tests.cpp(32): Leaving test case "init_test"; testing time: 32703us
    

    https://cirrus-ci.com/task/5336403023757312?logs=ci#L2886:

    0[18:45:09.651] /ci_container_base/src/ipc/interfaces.cpp:35:5: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
    1[18:45:09.651]    35 |     write(STDOUT_FILENO, g_ignore_ctrl_c.data(), g_ignore_ctrl_c.size());
    2[18:45:09.651]       |     ^~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    3[18:45:09.651] 1 error generated.
    
  6. ryanofsky force-pushed on Apr 25, 2025
  7. ryanofsky commented at 5:38 pm on April 25, 2025: contributor
    Updated e8372fa3a606d35146c72fc61663e17c67a4621a -> a56468ab0bf2154159919a2c3feba9974486d10e (pr/ipc-stop.1 -> pr/ipc-stop.2, compare) to fix draftbot typos and CI failures, though I could not reproduce the previous releases AppInitMain failure in local CI so it could still be broken
  8. darosior commented at 7:53 pm on April 25, 2025: member
    Concept ACK. Will test using my Rust client for #29409 rebased on top of this.
  9. ryanofsky force-pushed on Apr 28, 2025
  10. ryanofsky commented at 4:22 pm on April 28, 2025: contributor
    Updated a56468ab0bf2154159919a2c3feba9974486d10e -> 0b72656ecf363d9fc4820a9e3e22afb2c98d4e75 (pr/ipc-stop.2 -> pr/ipc-stop.3, compare) to try to fix init test CI failures https://github.com/bitcoin/bitcoin/actions/runs/14670251419/job/41176416988?pr=32345 and https://cirrus-ci.com/task/6094268256747520 Updated 0b72656ecf363d9fc4820a9e3e22afb2c98d4e75 -> 5b38e62ccbfcdb7f20a6a74fcc4ef2358e3da56c (pr/ipc-stop.3 -> pr/ipc-stop.4, compare) to fix test_bitcoin-qt failures caused by previous fix https://github.com/bitcoin/bitcoin/pull/32345/checks?check_run_id=41288589561
  11. ryanofsky force-pushed on Apr 28, 2025
  12. DrahtBot removed the label CI failed on Apr 28, 2025
  13. maflcko commented at 6:18 am on April 29, 2025: member
    There are also some typos, according to #32345 (comment), if you want to fix them if you have to re-touch for other reasons anyway.
  14. DrahtBot added the label CI failed on May 5, 2025
  15. DrahtBot added the label Needs rebase on May 9, 2025
  16. darosior commented at 6:49 pm on May 20, 2025: member

    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:

     0[...]
     12025-05-20T18:44:44Z init message: Starting network threads
     22025-05-20T18:44:44Z DNS seeding disabled
     32025-05-20T18:44:44Z init message: Done loading
     42025-05-20T18:44:44Z initload thread start
     52025-05-20T18:44:44Z msghand thread start
     62025-05-20T18:44:44Z Loading 0 mempool transactions from file...
     72025-05-20T18:44:44Z Imported mempool transactions from file: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial broadcast
     82025-05-20T18:44:44Z opencon thread start
     92025-05-20T18:44:44Z addcon thread start
    102025-05-20T18:44:44Z net thread start
    112025-05-20T18:44:44Z initload thread exit
    122025-05-20T18:44:44Z New manual v2 peer connected: version: 70016, blocks=113, peer=0
    132025-05-20T18:44:44Z Synchronizing blockheaders, height: 113 (~0.47%)
    142025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#1](/bitcoin-bitcoin/1/) Init.construct$Params ()
    152025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#1](/bitcoin-bitcoin/1/) Init.construct$Results (threadMap = <external capability>)
    162025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#2](/bitcoin-bitcoin/2/) Init.makeChain$Params (context = (thread = <external capability>))
    172025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request  [#2](/bitcoin-bitcoin/2/) {bitcoin-node-948798/b-capnp-loop-948823 (from )}
    182025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#2](/bitcoin-bitcoin/2/) Init.makeChain$Results (result = <external capability>)
    192025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#3](/bitcoin-bitcoin/3/) Chain.initMessage$Params (context = (thread = <external capability>), message = "Oxydation of the Bitcoin Core wallet in progress..")
    202025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request  [#3](/bitcoin-bitcoin/3/) {bitcoin-node-948798/b-capnp-loop-948823 (from )}
    212025-05-20T18:44:45Z init message: Oxydation of the Bitcoin Core wallet in progress..
    222025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#3](/bitcoin-bitcoin/3/) Chain.initMessage$Results ()
    232025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#4](/bitcoin-bitcoin/4/) Chain.showProgress$Params (context = (thread = <external capability>), title = "BDK Core startup", progress = 1, resumePossible = false)
    242025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request  [#4](/bitcoin-bitcoin/4/) {bitcoin-node-948798/b-capnp-loop-948823 (from )}
    252025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#4](/bitcoin-bitcoin/4/) Chain.showProgress$Results ()
    262025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#5](/bitcoin-bitcoin/5/) Chain.getHeight$Params (context = (thread = <external capability>))
    272025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request  [#5](/bitcoin-bitcoin/5/) {bitcoin-node-948798/b-capnp-loop-948823 (from )}
    282025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#5](/bitcoin-bitcoin/5/) Chain.getHeight$Results (result = 113, hasResult = true)
    292025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#6](/bitcoin-bitcoin/6/) Chain.getBlockHash$Params (context = (thread = <external capability>), height = 113)
    302025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request  [#6](/bitcoin-bitcoin/6/) {bitcoin-node-948798/b-capnp-loop-948823 (from )}
    312025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#6](/bitcoin-bitcoin/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")
    322025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#7](/bitcoin-bitcoin/7/) Chain.showProgress$Params (context = (thread = <external capability>), title = "BDK Core startup", progress = 100, resumePossible = true)
    332025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request  [#7](/bitcoin-bitcoin/7/) {bitcoin-node-948798/b-capnp-loop-948823 (from )}
    342025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#7](/bitcoin-bitcoin/7/) Chain.showProgress$Results ()
    352025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server recv request  [#8](/bitcoin-bitcoin/8/) Chain.handleNotifications$Params (context = (thread = <external capability>), notifications = <external capability>)
    362025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server post request  [#8](/bitcoin-bitcoin/8/) {bitcoin-node-948798/b-capnp-loop-948823 (from )}
    372025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server send response [#8](/bitcoin-bitcoin/8/) Chain.handleNotifications$Results (result = <external capability>)
    382025-05-20T18:44:45Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server destroy N2mp11ProxyServerIN3ipc5capnp8messages7HandlerEEE
    39^C2025-05-20T18:44:47Z Shutdown: In progress...
    402025-05-20T18:44:47Z addcon thread exit
    412025-05-20T18:44:47Z opencon thread exit
    422025-05-20T18:44:47Z net thread exit
    432025-05-20T18:44:47Z msghand thread exit
    442025-05-20T18:44:47Z scheduler thread exit
    452025-05-20T18:44:47Z Writing 0 mempool transactions to file...
    462025-05-20T18:44:47Z Writing 0 unbroadcast transactions to file.
    472025-05-20T18:44:47Z Dumped mempool: 0.000s to copy, 0.003s to dump, 27 bytes dumped to file
    482025-05-20T18:44:47Z Flushed fee estimates to fee_estimates.dat.
    492025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-shutoff-948798} IPC client first request from current thread, constructing waiter
    502025-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...
    512025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-shutoff-948798} IPC client recv ChainNotifications.chainStateFlushed$Results ()
    522025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server destroy N2mp11ProxyServerIN3ipc5capnp8messages5ChainEEE
    532025-05-20T18:44:47Z [ipc] {bitcoin-node-948798/b-capnp-loop-948800} IPC server destroy N2mp11ProxyServerIN3ipc5capnp8messages4InitEEE
    54Segmentation fault
    
  17. ryanofsky commented at 2:33 am on May 21, 2025: contributor

    re: #32345 (comment)

    I found that bitcoin-node will (still) segfault if i try to stop it while the client is running.

    Thanks for testing this! I created a new issue in https://github.com/bitcoin-core/libmultiprocess/issues/176 to help debug. So far I couldn’t reproduce the problem running a very basic client there, but I can try to extend it to do more of the operations your client is doing and see if I can make the problem happen. It would also help to have a stack trace from the segfault, and I provided instructions for getting a stack trace there.

  18. ryanofsky force-pushed on Jun 5, 2025
  19. ryanofsky force-pushed on Jun 5, 2025
  20. ryanofsky commented at 5:50 pm on June 5, 2025: contributor
    Rebased 5b38e62ccbfcdb7f20a6a74fcc4ef2358e3da56c -> c1cce04f763e88347cf025fab76b894b4b015dab (pr/ipc-stop.4 -> pr/ipc-stop.5, compare) due to conflicts with #32641 and #32438 Updated c1cce04f763e88347cf025fab76b894b4b015dab -> a180915d3604a2bff14040ccbfd67ee586a8e212 (pr/ipc-stop.5 -> pr/ipc-stop.6, compare) to fix https://github.com/bitcoin-core/libmultiprocess/issues/176 Updated a180915d3604a2bff14040ccbfd67ee586a8e212 -> d2b1e821d48f4d33d5015f3ea3064a26788a3e3c (pr/ipc-stop.6 -> pr/ipc-stop.7, compare) to fix more test problems with globals in node_init_tests https://cirrus-ci.com/task/4606963511721984
  21. DrahtBot removed the label Needs rebase on Jun 5, 2025
  22. ryanofsky force-pushed on Jun 6, 2025
  23. DrahtBot removed the label CI failed on Jun 6, 2025
  24. ryanofsky force-pushed on Jun 13, 2025
  25. ryanofsky commented at 8:08 pm on June 13, 2025: contributor
    Rebased d2b1e821d48f4d33d5015f3ea3064a26788a3e3c -> 49646814a40cc2b4199e3e4fee60ac49da468f5d (pr/ipc-stop.7 -> pr/ipc-stop.8, compare) on top of updated base PR https://github.com/bitcoin-core/libmultiprocess/pull/160 pr/eventlock.7 to resolve more issues reported in https://github.com/bitcoin-core/libmultiprocess/pull/182 Rebased 49646814a40cc2b4199e3e4fee60ac49da468f5d -> 616a09631ffb7f7c7e8746957289bdfd4e734a4e (pr/ipc-stop.8 -> pr/ipc-stop.9, compare) on top of updated base PR https://github.com/bitcoin-core/libmultiprocess/pull/160 pr/eventlock.8 to fix spurious thread safety warning https://cirrus-ci.com/task/4956347122319360 Rebased 616a09631ffb7f7c7e8746957289bdfd4e734a4e -> 53b12cac6badc2746e0fbb8f94eae5fe84fc842b (pr/ipc-stop.9 -> pr/ipc-stop.10, compare) on top of updated base R https://github.com/bitcoin-core/libmultiprocess/pull/160 pr/eventlock.9 to fix spurious thread safety warning https://cirrus-ci.com/task/5066891695226880
  26. DrahtBot added the label CI failed on Jun 13, 2025
  27. DrahtBot commented at 11:51 pm on June 13, 2025: contributor

    🚧 At least one of the CI tasks failed. Task multiprocess, i686, DEBUG: https://github.com/bitcoin/bitcoin/runs/44074743936 LLM reason (✨ experimental): The CI failure is caused by a compile error due to an unheld mutex lock in src/ipc/libmultiprocess/src/mp/proxy.cpp.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  28. ryanofsky force-pushed on Jun 16, 2025
  29. ryanofsky force-pushed on Jun 16, 2025
  30. DrahtBot removed the label CI failed on Jun 16, 2025
  31. pinheadmz commented at 3:24 pm on June 17, 2025: member
    concept ACK, solved an edge case I had with #32297
  32. ryanofsky referenced this in commit 258a617c1e on Jun 19, 2025
  33. ryanofsky force-pushed on Jun 24, 2025
  34. ryanofsky commented at 5:13 pm on June 24, 2025: contributor

    Rebased 53b12cac6badc2746e0fbb8f94eae5fe84fc842b -> 3ed3f7f979fb8e4cf3023deb64acf9b0a545057a (pr/ipc-stop.10 -> pr/ipc-stop.11, compare) with no changes after https://github.com/bitcoin-core/libmultiprocess/pull/160 merge

    Will change status from draft -> ready for review

  35. ryanofsky marked this as ready for review on Jun 24, 2025
  36. DrahtBot added the label CI failed on Jun 24, 2025
  37. DrahtBot commented at 7:03 pm on June 24, 2025: contributor

    🚧 At least one of the CI tasks failed. Task multiprocess, i686, DEBUG: https://github.com/bitcoin/bitcoin/runs/44704578313 LLM reason (✨ experimental): The CI failure is due to a test timeout during execution.

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  38. DrahtBot removed the label CI failed on Jun 24, 2025
  39. DrahtBot added the label CI failed on Jun 25, 2025
  40. DrahtBot removed the label CI failed on Jun 25, 2025
  41. maflcko commented at 8:29 am on June 25, 2025: member
    the CI failure is real, can manifest itself via segfault, timeout, abort, assertion failure, …
  42. Sjors commented at 8:32 am on June 25, 2025: member
    I’ve been seeing segfaults etc too in https://github.com/Sjors/bitcoin/pull/90. I’m running it again now with the most recent versions of this and https://github.com/bitcoin-core/libmultiprocess/pull/184 (and #31802 for increased CI coverage). Update: yup, more segfaults.
  43. in src/ipc/interfaces.cpp:35 in 3ed3f7f979 outdated
    26@@ -26,6 +27,27 @@
    27 
    28 namespace ipc {
    29 namespace {
    30+#ifndef WIN32
    31+std::string g_ignore_ctrl_c;
    32+
    33+void HandleCtrlC(int)
    34+{
    35+    (void)write(STDOUT_FILENO, g_ignore_ctrl_c.data(), g_ignore_ctrl_c.size());
    


    Sjors commented at 7:31 am on June 27, 2025:
    CI on https://github.com/Sjors/bitcoin/pull/90#issuecomment-3011857912 complains of an unused result. Which is odd because the (void) cast should prevent such a warning. This seems to be a gcc thing: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425

    ryanofsky commented at 3:18 am on July 1, 2025:

    re: #32345 (review)

    CI on Sjors#90 (comment) complains of an unused result. Which is odd because the (void) cast should prevent such a warning. This seems to be a gcc thing: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425

    Thanks! I tried to work around this by adding a ! though reading gcc issue maybe that will not be sufficient.


    Sjors commented at 7:32 am on July 1, 2025:
    Ok, we’ll find out, if CI passes in the latest https://github.com/Sjors/bitcoin/pull/90

    Sjors commented at 9:07 am on July 1, 2025:
    Narrator: it does
  44. in src/ipc/interfaces.cpp:21 in 3ed3f7f979 outdated
    17@@ -18,6 +18,7 @@
    18 #include <cstring>
    19 #include <functional>
    20 #include <memory>
    21+#include <signal.h>
    


    Sjors commented at 7:47 am on June 27, 2025:
    <csignal>

    ryanofsky commented at 3:13 am on July 1, 2025:

    re: #32345 (review)

    <csignal>

    Thanks, fixed

  45. ryanofsky force-pushed on Jul 1, 2025
  46. ryanofsky commented at 3:21 am on July 1, 2025: contributor

    Rebased 3ed3f7f979fb8e4cf3023deb64acf9b0a545057a -> 5024f552924b214fa8a092c1daf9acf92a4c40f0 (pr/ipc-stop.11 -> pr/ipc-stop.12, compare) with update to fix tidy errors and try to incorporate https://github.com/bitcoin-core/libmultiprocess/pull/186 Rebased 5024f552924b214fa8a092c1daf9acf92a4c40f0 -> b06a204dc0f64e3b30f3537e07a5fe1e0c8791c6 (pr/ipc-stop.12 -> pr/ipc-stop.13, compare) on latest https://github.com/bitcoin-core/libmultiprocess/pull/186 and adding test fix 9c7defac7f3965e782d3c6cf250b044e19d5146b forgot to include in previous push Updated b06a204dc0f64e3b30f3537e07a5fe1e0c8791c6 -> d62ec01e3d15865762fad671611555cce5315016 (pr/ipc-stop.13 -> pr/ipc-stop.14, compare) adding comment to explain (void)! Squashed d62ec01e3d15865762fad671611555cce5315016 -> a67f3b07d4a1ea9289d4f7d5d3536fe2033b67b4 (pr/ipc-stop.14 -> pr/ipc-stop.15, compare) after realizing 9c7defac7f3965e782d3c6cf250b044e19d5146b was fixing a bug introduced in an earlier commit

    Marked as a draft since this now depends on https://github.com/bitcoin-core/libmultiprocess/pull/186

  47. ryanofsky marked this as a draft on Jul 1, 2025
  48. in src/ipc/interfaces.cpp:36 in 5024f55292 outdated
    26@@ -26,6 +27,27 @@
    27 
    28 namespace ipc {
    29 namespace {
    30+#ifndef WIN32
    31+std::string g_ignore_ctrl_c;
    32+
    33+void HandleCtrlC(int)
    34+{
    35+    (void)!write(STDOUT_FILENO, g_ignore_ctrl_c.data(), g_ignore_ctrl_c.size());
    


    Sjors commented at 7:24 am on July 1, 2025:
    Maybe comment that the ! is for gcc

    ryanofsky commented at 4:38 pm on July 1, 2025:

    re: #32345 (review)

    Maybe comment that the ! is for gcc

    Makes sense, added comment

  49. ryanofsky force-pushed on Jul 1, 2025
  50. ryanofsky force-pushed on Jul 1, 2025
  51. ryanofsky force-pushed on Jul 1, 2025
  52. Squashed 'src/ipc/libmultiprocess/' changes from 27c7e8e5a581..a11e6905c238
    a11e6905c238 Merge bitcoin-core/libmultiprocess#186: Fix mptest failures in bitcoin CI
    6f340a583f2b doc: fix DrahtBot LLM Linter error
    c6f7fdf17350 type-context: revert client disconnect workaround
    e09143d2ea2f proxy-types: fix UndefinedBehaviorSanitizer: null-pointer-use
    84b292fcc4db mptest: fix MemorySanitizer: use-of-uninitialized-value
    fe4a188803c6 proxy-io: fix race conditions in disconnect callback code
    d8011c83608e proxy-io: fix race conditions in ProxyClientBase cleanup handler
    97e82ce19c47 doc: Add note about Waiter::m_mutex and interaction with the EventLoop::m_mutex
    81d58f5580e8 refactor: Rename ProxyClient cleanup_it variable
    07230f259f55 refactor: rename ProxyClient<Thread>::m_cleanup_it
    c0efaa5e8cb1 Merge chaincodelabs/libmultiprocess#187: ci: have bash scripts explicitly opt out of locale dependence.
    0d986ff144cd mptest: fix race condition in TestSetup constructor
    d2f6aa2e84ef ci: add thread sanitizer job
    3a6db38e561f ci: rename configs to .bash
    401e0ce1d9c3 ci: add copyright to bash scripts
    e956467ae464 ci: export LC_ALL
    8954cc0377d8 Merge chaincodelabs/libmultiprocess#184: Add CI jobs and fix clang-tidy and iwyu errors
    757e13a75546 ci: add gnu32 cross-compiled 32-bit build
    15bf349000eb doc: fix typo found by DrahtBot
    1a598d5905f7 clang-tidy: drop 'bitcoin-*' check
    cbb1e43fdc6e ci: test libc++ instead of libstdc++ in one job
    76313450c2c4 type-context: disable clang-tidy UndefinedBinaryOperatorResult error
    4896e7fe51ba proxy-types: fix clang-tidy EnumCastOutOfRange error
    060a73926956 proxy-types: fix clang-tidy StackAddressEscape error
    977d721020f6 ci: add github actions jobs testing gcc, clang-20, clang-tidy, and iwyu
    0d5f1faae5da iwyu: fix add/remove include errors
    753d2b10cc27 util: fix clang-tidy modernize-use-equals-default error
    ae4f1dc2bb1a type-number: fix clang-tidy modernize-use-nullptr error
    07a741bf6946 proxy-types: fix clang-tidy bugprone-use-after-move error
    3673114bc9d9 proxy-types: fix clang-tidy bugprone-use-after-move error
    422923f38485 proxy-types: fix clang-tidy bugprone-use-after-move error
    c6784c6adefa mpgen: disable clang-tidy misc-no-recursion error
    c5498aa11ba6 tidy: copy clang-tidy file from bitcoin core
    258a617c1eec Merge chaincodelabs/libmultiprocess#160: refactor: EventLoop locking cleanups + client disconnect exception
    84cf56a0b5f4 test: Test disconnects during IPC calls
    949573da8411 Prevent IPC server crash if disconnected during IPC call
    019839758085 Merge chaincodelabs/libmultiprocess#179: scripted-diff: Remove copyright year (ranges)
    ea38392960e1 Prevent EventLoop async cleanup thread early exit during shutdown
    616d9a75d20a doc: Document ProxyClientBase destroy_connection option
    56fff76f940b Improve IPC client disconnected exceptions
    9b8ed3dc5f87 refactor: Add clang thread safety annotations to EventLoop
    52256e730f51 refactor: Remove DestructorCatcher and AsyncCallable
    f24894794adf refactor: Drop addClient/removeClient methods
    2b830e558e61 refactor: Use EventLoopRef instead of addClient/removeClient
    315ff537fb65 refactor: Add ProxyContext EventLoop* member
    9aaeec3678d3 proxy-io.h: Add EventLoopRef RAII class handle addClient/removeClient refcounting
    f58c8d8ba2f0 proxy-io.h: Add more detailed EventLoop comment
    5108445e5d16 test: Add test coverage for client & server disconnections
    59030c68cb5f Merge chaincodelabs/libmultiprocess#181: type-function.h: Fix CustomBuildField overload
    688140b1dffc test: Add coverage for type-function.h
    8b96229da58e type-function.h: Fix CustomBuildField overload
    fa2ff9a66842 scripted-diff: Remove copyright year (ranges)
    
    git-subtree-dir: src/ipc/libmultiprocess
    git-subtree-split: a11e6905c238dc35a8bbef995190296bc6329d49
    ca509dd2b9
  53. Merge commit 'ca509dd2b91385294c837a5d264b10a29ebb0bc8' into pr/ipc-stop-base b6bd3ddf56
  54. ipc: Use EventLoopRef instead of addClient/removeClient
    Use EventLoopRef to avoid reference counting bugs and be more exception safe
    and deal with removal of addClient/removeClient methods in
    https://github.com/bitcoin-core/libmultiprocess/pull/160
    
    A test update is also required due to
    https://github.com/bitcoin-core/libmultiprocess/pull/160 to deal with changed
    reference count semantics. In IpcPipeTest(), it is now necessary to destroy
    the client Proxy object instead of just the client Connection object to
    decrease the event loop reference count and allow the loop to exit so the test
    does not hang on shutdown.
    f8fd3959d5
  55. test: Add unit test coverage for Init and Shutdown code
    Currently this code is not called in unit tests. Calling should make it
    possible to write tests for things like IPC exceptions being thrown during
    shutdown.
    637502894d
  56. ipc: Avoid waiting for clients to disconnect when shutting down
    This fixes behavior reported by Antoine Poinsot <darosior@protonmail.com>
    https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-2546088852 where if
    an IPC client is connected, the node will wait forever for it to disconnect
    before exiting.
    ff1c9278e7
  57. doc: Improve IPC interface comments
    Fix some comments that were referring to previous versions of these methods and
    did not make sense.
    d37b740047
  58. ipc: Add Ctrl-C handler for spawned subprocesses
    This fixes an error reported by Antoine Poinsot <darosior@protonmail.com> in
    https://github.com/bitcoin-core/libmultiprocess/issues/123 that does not happen
    in master, but does happen with https://github.com/bitcoin/bitcoin/pull/10102
    applied, where if Ctrl-C is pressed when `bitcoin-node` is started, it is
    handled by both `bitcoin-node` and `bitcoin-wallet` processes, causing the
    wallet to shutdown abruptly instead of waiting for the node and shutting down
    cleanly.
    
    This change fixes the problem by having the wallet process print to stdout when
    it receives a Ctrl-C signal but not otherwise react, letting the node shut
    everything down cleanly.
    008f4ae01e
  59. ipc: Handle bitcoin-wallet disconnections
    This fixes an error reported by Antoine Poinsot <darosior@protonmail.com> in
    https://github.com/bitcoin-core/libmultiprocess/issues/123 that does not happen
    in master, but does happen with https://github.com/bitcoin/bitcoin/pull/10102
    applied, where if the child bitcoin-wallet process is killed (either by an
    external signal or by Ctrl-C as reported in the issue) the bitcoin-node process
    will not shutdown cleanly after that because chain client stop()
    calls will fail.
    
    This change fixes the problem by handling ipc::Exception errors thrown during
    the stop() calls, and it relies on the fixes to disconnect detection
    implemented in https://github.com/bitcoin-core/libmultiprocess/pull/160 to work
    effectively.
    0a9a194078
  60. ryanofsky referenced this in commit a11e6905c2 on Jul 1, 2025
  61. ryanofsky force-pushed on Jul 1, 2025
  62. ryanofsky commented at 7:13 pm on July 1, 2025: contributor
    Rebased a67f3b07d4a1ea9289d4f7d5d3536fe2033b67b4 -> 0a9a1940782fad8dc3494c4aacee0a06c030eb5c (pr/ipc-stop.15 -> pr/ipc-stop.16, compare) after https://github.com/bitcoin-core/libmultiprocess/pull/186 merge. Marking ready for review again

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-06 21:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me