From 1d92d89edbb1812dc353084c62772ebb1024d632 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 7 Jul 2023 17:32:54 -0400 Subject: [PATCH] util: Get rid of uncaught exceptions thrown by SignalInterrupt class Replace exceptions thrown by signal and wait methods with [[nodiscard]] return values. This is mostly a refactoring, but there is a slight change of behavior if AbortShutdown function fails. The original behavior which was unintentionally changed in #27861 is restored, so it now triggers an assert failure again instead of throwing an exception. (The AbortShutdown function is only ever called in the the GUI version of Bitcoin Core when corruption is detected on loading and the user tries to reindex.) Problems with using exceptions were pointed out by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1255496707. --- src/shutdown.cpp | 13 ++++++------- src/util/signalinterrupt.cpp | 15 +++++++++------ src/util/signalinterrupt.h | 6 +++--- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/shutdown.cpp b/src/shutdown.cpp index fc18ccd2076..9e0b2693e5b 100644 --- a/src/shutdown.cpp +++ b/src/shutdown.cpp @@ -15,9 +15,7 @@ void StartShutdown() { - try { - Assert(kernel::g_context)->interrupt(); - } catch (const std::system_error&) { + if (!Assert(kernel::g_context)->interrupt()) { LogPrintf("Sending shutdown token failed\n"); assert(0); } @@ -25,7 +23,10 @@ void StartShutdown() void AbortShutdown() { - Assert(kernel::g_context)->interrupt.reset(); + if (!Assert(kernel::g_context)->interrupt.reset()) { + LogPrintf("Reading shutdown token failed\n"); + assert(0); + } } bool ShutdownRequested() @@ -35,9 +36,7 @@ bool ShutdownRequested() void WaitForShutdown() { - try { - Assert(kernel::g_context)->interrupt.wait(); - } catch (const std::system_error&) { + if (!Assert(kernel::g_context)->interrupt.wait()) { LogPrintf("Reading shutdown token failed\n"); assert(0); } diff --git a/src/util/signalinterrupt.cpp b/src/util/signalinterrupt.cpp index c551ba8044c..f36f3dbf9ee 100644 --- a/src/util/signalinterrupt.cpp +++ b/src/util/signalinterrupt.cpp @@ -30,15 +30,16 @@ SignalInterrupt::operator bool() const return m_flag; } -void SignalInterrupt::reset() +bool SignalInterrupt::reset() { // Cancel existing interrupt by waiting for it, this will reset condition flags and remove // the token from the pipe. - if (*this) wait(); + if (*this && !wait()) return false; m_flag = false; + return true; } -void SignalInterrupt::operator()() +bool SignalInterrupt::operator()() { #ifdef WIN32 std::unique_lock lk(m_mutex); @@ -52,13 +53,14 @@ void SignalInterrupt::operator()() // 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"); + return false; } } #endif + return true; } -void SignalInterrupt::wait() +bool SignalInterrupt::wait() { #ifdef WIN32 std::unique_lock lk(m_mutex); @@ -66,9 +68,10 @@ void SignalInterrupt::wait() #else int res = m_pipe_r.TokenRead(); if (res != 'x') { - throw std::ios_base::failure("Did not read expected interrupt token"); + return false; } #endif + return true; } } // namespace util diff --git a/src/util/signalinterrupt.h b/src/util/signalinterrupt.h index ca02feda91c..605d124206b 100644 --- a/src/util/signalinterrupt.h +++ b/src/util/signalinterrupt.h @@ -30,9 +30,9 @@ class SignalInterrupt public: SignalInterrupt(); explicit operator bool() const; - void operator()(); - void reset(); - void wait(); + [[nodiscard]] bool operator()(); + [[nodiscard]] bool reset(); + [[nodiscard]] bool wait(); private: std::atomic m_flag;