MINOR: tinfo: add a new thread flag to indicate a call from a sig handler

Signal handlers must absolutely not change anything, but some long and
complex call chains may look innocuous at first glance, yet result in
some subtle write accesses (e.g. pools) that can conflict with a running
thread being interrupted.

Let's add a new thread flag TH_FL_IN_SIG_HANDLER that is only set when
entering a signal handler and cleared when leaving them. Note, we're
speaking about real signal handlers (synchronous ones), not deferred
ones. This will allow some sensitive call places to act differently
when detecting such a condition, and possibly even to place a few new
BUG_ON().

diff --git a/include/haproxy/tinfo-t.h b/include/haproxy/tinfo-t.h
index 8aff35452b90bd50bcd2fb46c151d9c750339da9..a8a3e0d8bf9df4e682cf812e1f4b83cc643077eb 100644 (file)
--- a/include/haproxy/tinfo-t.h
+++ b/include/haproxy/tinfo-t.h
@@ -65,6 +65,7 @@ enum {
 #define TH_FL_STARTED           0x00000010  /* set once the thread starts */
 #define TH_FL_IN_LOOP           0x00000020  /* set only inside the polling loop */
 #define TH_FL_DUMPING_OTHERS    0x00000040  /* thread currently dumping other threads */
+#define TH_FL_IN_SIG_HANDLER    0x00000080  /* thread currently in signal handler */
 
 /* we have 4 buffer-wait queues, in highest to lowest emergency order */
 #define DYNBUF_NBQ              4

diff --git a/src/debug.c b/src/debug.c
index 35f60c7a433b614d8d662a2f4989ffd58f380ce3..2e5c06a9e324a881164d89ec4d55ce4931b24e68 100644 (file)
--- a/src/debug.c
+++ b/src/debug.c
@@ -2378,6 +2378,9 @@ void debug_handler(int sig, siginfo_t *si, void *arg)
        if (!buf || (ulong)buf & 0x1UL)
                return;
 
+       /* inform callees to be careful, we're in a signal handler! */
+       _HA_ATOMIC_OR(&th_ctx->flags, TH_FL_IN_SIG_HANDLER);
+
        /* Special value 0x2 is used during panics and requires that the thread
         * allocates its own dump buffer among its own trash buffers. The goal
         * is that all threads keep a copy of their own dump.
@@ -2398,6 +2401,8 @@ void debug_handler(int sig, siginfo_t *si, void *arg)
         */
        while (no_return)
                wait(NULL);
+
+       _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER);
 }
 
 static int init_debug_per_thread()

diff --git a/src/signal.c b/src/signal.c
index 9dfb5dc4b51b57d227f07d6755559f171dea6764..e5ca614e048d5821ac33d9ca96493b81765e5d3a 100644 (file)
--- a/src/signal.c
+++ b/src/signal.c
@@ -38,10 +38,14 @@ DECLARE_STATIC_POOL(pool_head_sig_handlers, "sig_handlers", sizeof(struct sig_ha
  */
 void signal_handler(int sig)
 {
+       /* inform callees to be careful, we're in a signal handler! */
+       _HA_ATOMIC_OR(&th_ctx->flags, TH_FL_IN_SIG_HANDLER);
+
        if (sig < 0 || sig >= MAX_SIGNAL) {
                /* unhandled signal */
                signal(sig, SIG_IGN);
                qfprintf(stderr, "Received unhandled signal %d. Signal has been disabled.\n", sig);
+               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER);
                return;
        }
 
@@ -59,6 +63,7 @@ void signal_handler(int sig)
 
        /* If the thread is TH_FL_SLEEPING we need to wake it */
        wake_thread(tid);
+       _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER);
 }
 
 /* Call handlers of all pending signals and clear counts and queue length. The

diff --git a/src/wdt.c b/src/wdt.c
index dd8f9a1a49b8ca1af8dc8fd5771ba9edb499bed5..b5edece447311fa91b657294b614092b9b327a4b 100644 (file)
--- a/src/wdt.c
+++ b/src/wdt.c
@@ -68,6 +68,9 @@ void wdt_handler(int sig, siginfo_t *si, void *arg)
        ulong thr_bit;
        int thr, tgrp;
 
+       /* inform callees to be careful, we're in a signal handler! */
+       _HA_ATOMIC_OR(&th_ctx->flags, TH_FL_IN_SIG_HANDLER);
+
        switch (si->si_code) {
        case SI_TIMER:
                /* A thread's timer fired, the thread ID is in si_int. We have
@@ -163,6 +166,7 @@ void wdt_handler(int sig, siginfo_t *si, void *arg)
 #endif
        default:
                /* unhandled other conditions */
+               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER);
                return;
        }
 
@@ -177,10 +181,14 @@ void wdt_handler(int sig, siginfo_t *si, void *arg)
        else
 #endif
                ha_panic();
+
+       _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER);
        return;
 
  update_and_leave:
        wdt_ping(thr);
+
+       _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_IN_SIG_HANDLER);
 }
 
 /* parse the "warn-blocked-traffic-after" parameter */