From bb81f26fcb6c99ac3a81cd6f4d511e6d8f4e0029 Mon Sep 17 00:00:00 2001
From: Mikulas Patocka <mikulas@twibright.com>
Date: Sun, 6 Apr 2025 19:10:19 +0200
Subject: [PATCH] Make Ctrl-C handling thread-safe (#7603)

The Ctrl-C handling is not thread safe, there's a global variable g_obj
that is being accessed without any locking. The signal handlers are
per-process, not per-thread, so that different threads step over each
other's handlers. It is unpredictable in which thread the signal handler
runs, so the handler may race with the scoped_ctrl_c destructor.

Fix this by introducing the functions signal_lock and signal_unlock.
signal_lock blocks the SIGINT signal and then takes a mutex (so that the
signal handler can't be called while the mutex is held). signal_unlock
drops the mutex and restores the signal mask.

We protect all the global variables with signal_lock and signal_unlock.

Note that on Windows, the SIGINT handler is being run in a separate
thread (and there is no way how to block it), so we can use a simple
mutex to synchronize the signal handler with the other threads.

Signed-off-by: Mikulas Patocka <mikulas@twibright.com>
---
 src/util/scoped_ctrl_c.cpp | 121 ++++++++++++++++++++++++++++++-------
 src/util/scoped_ctrl_c.h   |   2 -
 2 files changed, 100 insertions(+), 23 deletions(-)

diff --git a/src/util/scoped_ctrl_c.cpp b/src/util/scoped_ctrl_c.cpp
index a3f5ee772..5d1700823 100644
--- a/src/util/scoped_ctrl_c.cpp
+++ b/src/util/scoped_ctrl_c.cpp
@@ -12,46 +12,125 @@ Abstract:
 Author:
 
     Leonardo de Moura (leonardo) 2011-04-27.
+    Mikulas Patocka 2025-04-05. (rewritten to be thread safe)
 
 Revision History:
 
 --*/
-#include<signal.h>
+#include <signal.h>
+#include <cstring>
+#include <mutex>
 #include "util/scoped_ctrl_c.h"
 
-static scoped_ctrl_c * g_obj = nullptr;
+#ifdef _WINDOWS
+#define USE_SIGNAL
+#endif
 
-static void on_ctrl_c(int) {
-    if (g_obj->m_first) {
-        g_obj->m_cancel_eh(CTRL_C_EH_CALLER);
-        if (g_obj->m_once) {
-            g_obj->m_first = false;
-            signal(SIGINT, on_ctrl_c); // re-install the handler
-        }
+static std::mutex context_lock;
+static std::vector<scoped_ctrl_c *> active_contexts;
+#ifdef USE_SIGNAL
+static void (*old_handler)(int);
+#else
+static sigset_t context_old_set;
+static struct sigaction old_sigaction;
+#endif
+static bool signal_handled = false;
+
+static void signal_lock(void) {
+#ifdef USE_SIGNAL
+    context_lock.lock();
+#else
+    sigset_t set, old_set;
+    sigemptyset(&set);
+    sigaddset(&set, SIGINT);
+    if (sigprocmask(SIG_BLOCK, &set, &old_set))
+        abort();
+    context_lock.lock();
+    context_old_set = old_set;
+#endif
+}
+
+static void signal_unlock(void) {
+#ifdef USE_SIGNAL
+    context_lock.unlock();
+#else
+    sigset_t old_set = context_old_set;
+    context_lock.unlock();
+    if (sigprocmask(SIG_SETMASK, &old_set, NULL))
+        abort();
+#endif
+}
+
+static void test_and_unhandle(void) {
+    if (!signal_handled)
+        return;
+    for (auto a : active_contexts) {
+        if (a->m_first)
+            return;
     }
-    else {
-        signal(SIGINT, g_obj->m_old_handler); 
-        raise(SIGINT);
+#ifdef USE_SIGNAL
+    signal(SIGINT, old_handler);
+#else
+    if (sigaction(SIGINT, &old_sigaction, NULL))
+        abort();
+#endif
+    signal_handled = false;
+}
+
+static void on_sigint(int) {
+    signal_lock();
+#ifdef USE_SIGNAL
+    if (signal_handled)
+        signal(SIGINT, on_sigint);
+#endif
+    for (auto a : active_contexts) {
+        if (a->m_first)
+            a->m_cancel_eh(CTRL_C_EH_CALLER);
+        if (a->m_once)
+            a->m_first = false;
     }
+    test_and_unhandle();
+    signal_unlock();
 }
 
 scoped_ctrl_c::scoped_ctrl_c(event_handler & eh, bool once, bool enabled):
-    m_cancel_eh(eh), 
+    m_cancel_eh(eh),
     m_first(true),
     m_once(once),
-    m_enabled(enabled),
-    m_old_scoped_ctrl_c(g_obj) {
+    m_enabled(enabled) {
     if (m_enabled) {
-        g_obj = this;
-        m_old_handler = signal(SIGINT, on_ctrl_c); 
+        signal_lock();
+        active_contexts.push_back(this);
+        if (!signal_handled) {
+#ifdef USE_SIGNAL
+            old_handler = signal(SIGINT, on_sigint);
+#else
+            struct sigaction sa;
+            memset(&sa, 0, sizeof(struct sigaction));
+            sa.sa_handler = on_sigint;
+            sigemptyset(&sa.sa_mask);
+            sa.sa_flags = SA_RESTART;
+            if (sigaction(SIGINT, &sa, &old_sigaction))
+                abort();
+#endif
+            signal_handled = true;
+        }
+        signal_unlock();
     }
 }
 
-scoped_ctrl_c::~scoped_ctrl_c() { 
+scoped_ctrl_c::~scoped_ctrl_c() {
     if (m_enabled) {
-        g_obj = m_old_scoped_ctrl_c;
-        if (m_old_handler != SIG_ERR) {
-            signal(SIGINT, m_old_handler);
+        signal_lock();
+        for (auto it = active_contexts.begin(); it != active_contexts.end(); it++) {
+            if (*it == this) {
+                active_contexts.erase(it);
+                goto found;
+            }
         }
+        abort();
+found:
+        test_and_unhandle();
+        signal_unlock();
     }
 }
diff --git a/src/util/scoped_ctrl_c.h b/src/util/scoped_ctrl_c.h
index 09bc55c05..2b67f59a5 100644
--- a/src/util/scoped_ctrl_c.h
+++ b/src/util/scoped_ctrl_c.h
@@ -26,8 +26,6 @@ struct scoped_ctrl_c {
     bool m_first;
     bool m_once;
     bool m_enabled;
-    void  (STD_CALL *m_old_handler)(int);
-    scoped_ctrl_c * m_old_scoped_ctrl_c;
 public:
     // If once == true, then the cancel_eh is invoked only at the first Ctrl-C.
     // The next time, the old signal handler will take over.