util: Add SignalInterrupt class and use in shutdown.cpp

This change helps generalize shutdown code so an interrupt can be
provided to libbitcoinkernel callers. This may also be useful to
eventually de-globalize all of the shutdown code.

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: TheCharlatan <seb.kung@gmail.com>

10 files changed, 175 insertions, 65 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 85c3eaf08d..fe0b621340 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -310,6 +310,7 @@ BITCOIN_CORE_H = \
   util/readwritefile.h \
   util/result.h \
   util/serfloat.h \
+  util/signalinterrupt.h \
   util/sock.h \
   util/spanparsing.h \
   util/string.h \
@@ -733,6 +734,7 @@ libbitcoin_util_a_SOURCES = \
   util/moneystr.cpp \
   util/rbf.cpp \
   util/readwritefile.cpp \
+  util/signalinterrupt.cpp \
   util/thread.cpp \
   util/threadinterrupt.cpp \
   util/threadnames.cpp \
@@ -972,6 +974,7 @@ libbitcoinkernel_la_SOURCES = \
   util/moneystr.cpp \
   util/rbf.cpp \
   util/serfloat.cpp \
+  util/signalinterrupt.cpp \
   util/strencodings.cpp \
   util/string.cpp \
   util/syserror.cpp \
diff --git a/src/init.cpp b/src/init.cpp
index c38352ee38..e9421606ef 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -812,9 +812,7 @@ bool AppInitBasicSetup(const ArgsManager& args, std::atomic<int>& exit_status)
     // Enable heap terminate-on-corruption
     HeapSetInformation(nullptr, HeapEnableTerminationOnCorruption, nullptr, 0);
 #endif
-    if (!InitShutdownState(exit_status)) {
-        return InitError(Untranslated("Initializing wait-for-shutdown state failed."));
-    }
+    InitShutdownState(exit_status);
 
     if (!SetupNetworking()) {
         return InitError(Untranslated("Initializing networking failed."));
diff --git a/src/kernel/context.cpp b/src/kernel/context.cpp
index 1205da869e..3f4a423531 100644
--- a/src/kernel/context.cpp
+++ b/src/kernel/context.cpp
@@ -14,9 +14,12 @@
 
 
 namespace kernel {
+Context* g_context;
 
 Context::Context()
 {
+    assert(!g_context);
+    g_context = this;
     std::string sha256_algo = SHA256AutoDetect();
     LogPrintf("Using the '%s' SHA256 implementation\n", sha256_algo);
     RandomInit();
@@ -26,6 +29,8 @@ Context::Context()
 Context::~Context()
 {
     ECC_Stop();
+    assert(g_context);
+    g_context = nullptr;
 }
 
 } // namespace kernel
diff --git a/src/kernel/context.h b/src/kernel/context.h
index f11b7b54f0..e74e0a6f08 100644
--- a/src/kernel/context.h
+++ b/src/kernel/context.h
@@ -5,6 +5,8 @@
 #ifndef BITCOIN_KERNEL_CONTEXT_H
 #define BITCOIN_KERNEL_CONTEXT_H
 
+#include <util/signalinterrupt.h>
+
 #include <memory>
 
 namespace kernel {
@@ -16,12 +18,24 @@ namespace kernel {
 //! State stored directly in this struct should be simple. More complex state
 //! should be stored to std::unique_ptr members pointing to opaque types.
 struct Context {
+    //! Interrupt object that can be used to stop long-running kernel operations.
+    util::SignalInterrupt interrupt;
+
     //! Declare default constructor and destructor that are not inline, so code
     //! instantiating the kernel::Context struct doesn't need to #include class
     //! definitions for all the unique_ptr members.
     Context();
     ~Context();
 };
+
+//! Global pointer to kernel::Context for legacy code. New code should avoid
+//! using this, and require state it needs to be passed to it directly.
+//!
+//! Having this pointer is useful because it allows state be moved out of global
+//! variables into the kernel::Context struct before all global references to
+//! that state are removed. This allows the global references to be removed
+//! incrementally, instead of all at once.
+extern Context* g_context;
 } // namespace kernel
 
 #endif // BITCOIN_KERNEL_CONTEXT_H
diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp
index e918e84184..fb8029cb65 100644
--- a/src/qt/test/apptests.cpp
+++ b/src/qt/test/apptests.cpp
@@ -86,7 +86,6 @@ void AppTests::appTests()
 
     // Reset global state to avoid interfering with later tests.
     LogInstance().DisconnectTestLogger();
-    AbortShutdown();
 }
 
 //! Entry point for BitcoinGUI tests.
diff --git a/src/shutdown.cpp b/src/shutdown.cpp
index d70017d734..185cad5baf 100644
--- a/src/shutdown.cpp
+++ b/src/shutdown.cpp
@@ -9,17 +9,15 @@
 #include <config/bitcoin-config.h>
 #endif
 
+#include <kernel/context.h>
 #include <logging.h>
 #include <node/interface_ui.h>
 #include <util/check.h>
-#include <util/tokenpipe.h>
+#include <util/signalinterrupt.h>
 #include <warnings.h>
 
-#include <assert.h>
 #include <atomic>
-#ifdef WIN32
-#include <condition_variable>
-#endif
+#include <cassert>
 
 static std::atomic<int>* g_exit_status{nullptr};
 
@@ -36,76 +34,37 @@ bool AbortNode(const std::string& strMessage, bilingual_str user_message)
     return false;
 }
 
-static std::atomic<bool> fRequestShutdown(false);
-#ifdef WIN32
-/** On windows it is possible to simply use a condition variable. */
-std::mutex g_shutdown_mutex;
-std::condition_variable g_shutdown_cv;
-#else
-/** On UNIX-like operating systems use the self-pipe trick.
- */
-static TokenPipeEnd g_shutdown_r;
-static TokenPipeEnd g_shutdown_w;
-#endif
-
-bool InitShutdownState(std::atomic<int>& exit_status)
+void InitShutdownState(std::atomic<int>& exit_status)
 {
     g_exit_status = &exit_status;
-#ifndef WIN32
-    std::optional<TokenPipe> pipe = TokenPipe::Make();
-    if (!pipe) return false;
-    g_shutdown_r = pipe->TakeReadEnd();
-    g_shutdown_w = pipe->TakeWriteEnd();
-#endif
-    return true;
 }
 
 void StartShutdown()
 {
-#ifdef WIN32
-    std::unique_lock<std::mutex> lk(g_shutdown_mutex);
-    fRequestShutdown = true;
-    g_shutdown_cv.notify_one();
-#else
-    // This must be reentrant and safe for calling in a signal handler, so using a condition variable is not safe.
-    // Make sure that the token is only written once even if multiple threads call this concurrently or in
-    // case of a reentrant signal.
-    if (!fRequestShutdown.exchange(true)) {
-        // Write an arbitrary byte to the write end of the shutdown pipe.
-        int res = g_shutdown_w.TokenWrite('x');
-        if (res != 0) {
-            LogPrintf("Sending shutdown token failed\n");
-            assert(0);
-        }
+    try {
+        Assert(kernel::g_context)->interrupt();
+    } catch (const std::system_error&) {
+        LogPrintf("Sending shutdown token failed\n");
+        assert(0);
     }
-#endif
 }
 
 void AbortShutdown()
 {
-    if (fRequestShutdown) {
-        // Cancel existing shutdown by waiting for it, this will reset condition flags and remove
-        // the shutdown token from the pipe.
-        WaitForShutdown();
-    }
-    fRequestShutdown = false;
+    Assert(kernel::g_context)->interrupt.reset();
 }
 
 bool ShutdownRequested()
 {
-    return fRequestShutdown;
+    return bool{Assert(kernel::g_context)->interrupt};
 }
 
 void WaitForShutdown()
 {
-#ifdef WIN32
-    std::unique_lock<std::mutex> lk(g_shutdown_mutex);
-    g_shutdown_cv.wait(lk, [] { return fRequestShutdown.load(); });
-#else
-    int res = g_shutdown_r.TokenRead();
-    if (res != 'x') {
+    try {
+        Assert(kernel::g_context)->interrupt.wait();
+    } catch (const std::system_error&) {
         LogPrintf("Reading shutdown token failed\n");
         assert(0);
     }
-#endif
 }
diff --git a/src/shutdown.h b/src/shutdown.h
index c119bee96f..0e51575c5a 100644
--- a/src/shutdown.h
+++ b/src/shutdown.h
@@ -16,7 +16,7 @@ bool AbortNode(const std::string& strMessage, bilingual_str user_message = bilin
 /** Initialize shutdown state. This must be called before using either StartShutdown(),
  * AbortShutdown() or WaitForShutdown(). Calling ShutdownRequested() is always safe.
  */
-bool InitShutdownState(std::atomic<int>& exit_status);
+void InitShutdownState(std::atomic<int>& exit_status);
 
 /** Request shutdown of the application. */
 void StartShutdown();
diff --git a/src/util/signalinterrupt.cpp b/src/util/signalinterrupt.cpp
new file mode 100644
index 0000000000..c551ba8044
--- /dev/null
+++ b/src/util/signalinterrupt.cpp
@@ -0,0 +1,74 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <util/signalinterrupt.h>
+
+#ifdef WIN32
+#include <mutex>
+#else
+#include <util/tokenpipe.h>
+#endif
+
+#include <ios>
+#include <optional>
+
+namespace util {
+
+SignalInterrupt::SignalInterrupt() : m_flag{false}
+{
+#ifndef WIN32
+    std::optional<TokenPipe> pipe = TokenPipe::Make();
+    if (!pipe) throw std::ios_base::failure("Could not create TokenPipe");
+    m_pipe_r = pipe->TakeReadEnd();
+    m_pipe_w = pipe->TakeWriteEnd();
+#endif
+}
+
+SignalInterrupt::operator bool() const
+{
+    return m_flag;
+}
+
+void SignalInterrupt::reset()
+{
+    // Cancel existing interrupt by waiting for it, this will reset condition flags and remove
+    // the token from the pipe.
+    if (*this) wait();
+    m_flag = false;
+}
+
+void SignalInterrupt::operator()()
+{
+#ifdef WIN32
+    std::unique_lock<std::mutex> lk(m_mutex);
+    m_flag = true;
+    m_cv.notify_one();
+#else
+    // This must be reentrant and safe for calling in a signal handler, so using a condition variable is not safe.
+    // Make sure that the token is only written once even if multiple threads call this concurrently or in
+    // case of a reentrant signal.
+    if (!m_flag.exchange(true)) {
+        // Write an arbitrary byte to the write end of the pipe.
+        int res = m_pipe_w.TokenWrite('x');
+        if (res != 0) {
+            throw std::ios_base::failure("Could not write interrupt token");
+        }
+    }
+#endif
+}
+
+void SignalInterrupt::wait()
+{
+#ifdef WIN32
+    std::unique_lock<std::mutex> lk(m_mutex);
+    m_cv.wait(lk, [this] { return m_flag.load(); });
+#else
+    int res = m_pipe_r.TokenRead();
+    if (res != 'x') {
+        throw std::ios_base::failure("Did not read expected interrupt token");
+    }
+#endif
+}
+
+} // namespace util
diff --git a/src/util/signalinterrupt.h b/src/util/signalinterrupt.h
new file mode 100644
index 0000000000..ca02feda91
--- /dev/null
+++ b/src/util/signalinterrupt.h
@@ -0,0 +1,52 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_SIGNALINTERRUPT_H
+#define BITCOIN_UTIL_SIGNALINTERRUPT_H
+
+#ifdef WIN32
+#include <condition_variable>
+#include <mutex>
+#else
+#include <util/tokenpipe.h>
+#endif
+
+#include <atomic>
+#include <cstdlib>
+
+namespace util {
+/**
+ * Helper class that manages an interrupt flag, and allows a thread or
+ * signal to interrupt another thread.
+ *
+ * This class is safe to be used in a signal handler. If sending an interrupt
+ * from a signal handler is not necessary, the more lightweight \ref
+ * CThreadInterrupt class can be used instead.
+ */
+
+class SignalInterrupt
+{
+public:
+    SignalInterrupt();
+    explicit operator bool() const;
+    void operator()();
+    void reset();
+    void wait();
+
+private:
+    std::atomic<bool> m_flag;
+
+#ifndef WIN32
+    // On UNIX-like operating systems use the self-pipe trick.
+    TokenPipeEnd m_pipe_r;
+    TokenPipeEnd m_pipe_w;
+#else
+    // On windows use a condition variable, since we don't have any signals there
+    std::mutex m_mutex;
+    std::condition_variable m_cv;
+#endif
+};
+} // namespace util
+
+#endif // BITCOIN_UTIL_SIGNALINTERRUPT_H
diff --git a/src/util/threadinterrupt.h b/src/util/threadinterrupt.h
index ccc053f576..0b79b38276 100644
--- a/src/util/threadinterrupt.h
+++ b/src/util/threadinterrupt.h
@@ -12,11 +12,17 @@
 #include <chrono>
 #include <condition_variable>
 
-/*
-    A helper class for interruptible sleeps. Calling operator() will interrupt
-    any current sleep, and after that point operator bool() will return true
-    until reset.
-*/
+/**
+ * A helper class for interruptible sleeps. Calling operator() will interrupt
+ * any current sleep, and after that point operator bool() will return true
+ * until reset.
+ *
+ * This class should not be used in a signal handler. It uses thread
+ * synchronization primitives that are not safe to use with signals. If sending
+ * an interrupt from a signal handler is necessary, the \ref SignalInterrupt
+ * class can be used instead.
+ */
+
 class CThreadInterrupt
 {
 public: