sv2-tp: reconnect to bitcoin core IPC after disconnect

src/ipc/libmultiprocess/doc/versions.md

@@ -7,9 +7,14 @@ Library versions are tracked with simple
 Versioning policy is described in the [version.h](../include/mp/version.h)
 include.
 
-## v10
+## v11
 - Current unstable version.
 
+## [v10.0](https://github.com/bitcoin-core/libmultiprocess/commits/v10.0)
+- Increases spawn test timeout to avoid spurious failures.
+- Uses `throwRecoverableException` instead of raw `throw` to improve runtime error messages in macOS builds.
+- Used in Bitcoin Core master branch, pulled in by [#34977](https://github.com/bitcoin/bitcoin/pull/34977). Also pulled into Bitcoin Core 31.x stable branch by [#35028](https://github.com/bitcoin/bitcoin/pull/35028).
+
 ## [v9.0](https://github.com/bitcoin-core/libmultiprocess/commits/v9.0)
 - Fixes race conditions where worker thread could be used after destruction, where getParams() could be called after request cancel, and where m_on_cancel could be called after request finishes.
 - Adds `CustomHasField` hook to map Cap'n Proto null values to C++ null values.

src/ipc/libmultiprocess/include/mp/proxy-io.h

@@ -538,7 +538,12 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
         // the remote object, waiting for it to be deleted server side. If the
         // capnp interface does not define a destroy method, this will just call
         // an empty stub defined in the ProxyClientBase class and do nothing.
-        Sub::destroy(*this);
+        // Exceptions are caught and logged rather than propagated because
+        // ~ProxyClientBase is noexcept and the peer may be gone by the time
+        // this runs.
+        if (kj::runCatchingExceptions([&]{ Sub::destroy(*this); }) != nullptr) {
+            MP_LOG(*m_context.loop, Log::Warning) << "Remote destroy call failed during cleanup. Continuing.";
+        }
 
         // FIXME: Could just invoke removed addCleanup fn here instead of duplicating code
         m_context.loop->sync([&]() {

src/ipc/libmultiprocess/include/mp/version.h

@@ -24,7 +24,7 @@
 //! pointing at the prior merge commit. The /doc/versions.md file should also be
 //! updated, noting any significant or incompatible changes made since the
 //! previous version.
-#define MP_MAJOR_VERSION 10
+#define MP_MAJOR_VERSION 11
 
 //! Minor version number. Should be incremented in stable branches after
 //! backporting changes. The /doc/versions.md file should also be updated to

src/ipc/libmultiprocess/src/mp/proxy.cpp

@@ -22,6 +22,7 @@
 #include <kj/async-prelude.h>
 #include <kj/common.h>
 #include <kj/debug.h>
+#include <kj/exception.h>
 #include <kj/function.h>
 #include <kj/memory.h>
 #include <kj/string.h>
@@ -245,7 +246,12 @@ void EventLoop::loop()
         if (read_bytes != 1) throw std::logic_error("EventLoop wait_stream closed unexpectedly");
         Lock lock(m_mutex);
         if (m_post_fn) {
-            Unlock(lock, *m_post_fn);
+            // m_post_fn throwing is never expected. If it does happen, the caller
+            // of EventLoop::post() will return without any indication of failure,
+            // which will likely cause other bugs. Log the error and continue.
+            KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() MP_REQUIRES(m_mutex) { Unlock(lock, *m_post_fn); })) {
+                MP_LOG(*this, Log::Error) << "EventLoop: m_post_fn threw: " << kj::str(*exception).cStr();
+            }
             m_post_fn = nullptr;
             m_cv.notify_all();
         } else if (done()) {

src/ipc/libmultiprocess/test/mp/test/test.cpp

@@ -427,6 +427,32 @@ KJ_TEST("Calling async IPC method, with server disconnect after cleanup")
     }
 }
 
+KJ_TEST("Destroying ProxyClient<> with destroy method after peer disconnect")
+{
+    // Regression test for bitcoin-core/libmultiprocess#219 where
+    // ~ProxyClientBase would call std::terminate if the remote destroy RPC
+    // failed during teardown.
+    //
+    // Save a callback on the server so it holds a ProxyClient<FooCallback>
+    // pointing back to this side, then disconnect. When the server is torn
+    // down, the ProxyClient<FooCallback> destructor issues a destroy RPC over
+    // the now dead connection; without the bugfix the exception escapes the
+    // noexcept destructor and aborts the process.
+
+    TestSetup setup{/*client_owns_connection=*/false};
+    ProxyClient<messages::FooInterface>* foo = setup.client.get();
+    foo->initThreadMap();
+
+    class Callback : public FooCallback
+    {
+    public:
+        int call(int arg) override { return arg; }
+    };
+
+    foo->saveCallback(std::make_shared<Callback>());
+    setup.client_disconnect();
+}
+
 KJ_TEST("Make simultaneous IPC calls on single remote thread")
 {
     TestSetup setup;

src/sv2-tp.cpp

@@ -125,6 +125,11 @@ static void AddArgs(ArgsManager& args)
 }
 
 static bool g_interrupt{false};
+namespace {
+constexpr auto IPC_RECONNECT_INITIAL_DELAY{std::chrono::seconds{1}};
+constexpr auto IPC_RECONNECT_MAX_DELAY{std::chrono::seconds{32}};
+constexpr auto IPC_RECONNECT_POLL_INTERVAL{100ms};
+}
 
 #ifndef WIN32
 static void registerSignalHandler(int signal, void(*handler)(int))
@@ -231,34 +236,52 @@ MAIN_FUNCTION
         options.template_interval = std::chrono::seconds(args.GetIntArg("-templateinterval", 0));
     }
 
-    // Connect to bitcoin-node via IPC
-    //
-    // If the node is not available, keep retrying in a loop every 10 seconds.
     std::unique_ptr<interfaces::Init> mine_init{interfaces::MakeBasicInit("sv2-tp", argc > 0 ? argv[0] : "")};
     assert(mine_init);
-    std::unique_ptr<interfaces::Init> node_init;
     std::string address{args.GetArg("-ipcconnect", "unix")};
 
     LogPrintf("Attempting IPC connection to bitcoin-node at %s\n", address);
     LogPrintf("Ensure Bitcoin Core is running with '-ipcbind=unix' and the correct network (%s)\n",
               ChainTypeToString(chain_type));
 
-    while (true) {
-        try {
-            node_init = mine_init->ipc()->connectAddress(address);
-            LogPrintf("Connected to bitcoin-node via IPC at: %s\n", address);
-            break;  // Success: break out of the loop
-        } catch (const std::exception& exception) {
-            LogPrintf("IPC connection failed: %s\n", exception.what());
-            LogPrintf("Retrying in 10 seconds... (Ensure Bitcoin Core is running with '-ipcbind=unix')\n");
-            std::this_thread::sleep_for(std::chrono::seconds(10));
+    const auto connect_to_node = [&]() -> std::pair<std::unique_ptr<interfaces::Init>, std::unique_ptr<interfaces::Mining>> {
+        auto retry_delay = IPC_RECONNECT_INITIAL_DELAY;
+        while (!g_interrupt) {
+            try {
+                std::unique_ptr<interfaces::Init> node_init = mine_init->ipc()->connectAddress(address);
+                if (!node_init) {
+                    throw std::runtime_error("IPC connection returned no remote init interface");
+                }
+                std::unique_ptr<interfaces::Mining> mining = node_init->makeMining();
+                if (!mining) {
+                    throw std::runtime_error("IPC connection returned no mining interface");
+                }
+                LogPrintf("Connected to bitcoin-node via IPC at: %s\n", address);
+                return {std::move(node_init), std::move(mining)};
+            } catch (const std::exception& exception) {
+                LogPrintf("IPC connection failed: %s\n", exception.what());
+                LogPrintf("Retrying in %d seconds... (Ensure Bitcoin Core is running with '-ipcbind=unix')\n",
+                          retry_delay.count());
+                auto waited{0ms};
+                while (!g_interrupt && waited < retry_delay) {
+                    std::this_thread::sleep_for(IPC_RECONNECT_POLL_INTERVAL);
+                    waited += IPC_RECONNECT_POLL_INTERVAL;
+                }
+                retry_delay = std::min(retry_delay * 2, IPC_RECONNECT_MAX_DELAY);
+            }
         }
-    }
+        return {};
+    };
+
+    auto connection = connect_to_node();
+    auto node_init = std::move(connection.first);
+    auto mining = std::move(connection.second);
+    if (g_interrupt) return EXIT_SUCCESS;
     assert(node_init);
-    std::unique_ptr<interfaces::Mining> mining{node_init->makeMining()};
     assert(mining);
 
-    auto tp = std::make_unique<Sv2TemplateProvider>(*mining);
+    auto tp = std::make_unique<Sv2TemplateProvider>();
+    tp->ReplaceBackend(std::move(node_init), std::move(mining));
 
     if (!tp->Start(options)) {
         tfm::format(std::cerr, "Unable to start Stratum v2 Template Provider");
@@ -273,8 +296,20 @@ MAIN_FUNCTION
     registerSignalHandler(SIGINT, HandleSIGTERM);
 #endif
 
-    while(!g_interrupt) {
-        UninterruptibleSleep(100ms);
+    while (!g_interrupt) {
+        if (!tp->BackendDisconnected()) {
+            UninterruptibleSleep(100ms);
+            continue;
+        }
+
+        LogPrintf("Restarting sv2-tp after Bitcoin Core IPC disconnect\n");
+        connection = connect_to_node();
+        node_init = std::move(connection.first);
+        mining = std::move(connection.second);
+        if (g_interrupt) break;
+        assert(node_init);
+        assert(mining);
+        tp->ReplaceBackend(std::move(node_init), std::move(mining));
     }
 
     tp->Interrupt();