BUG/MEDIUM: threads: handle signal queue only in thread 0

Signals were handled in all threads which caused some signals to be lost
from time to time. To avoid complicated lock system (threads+signals),
we prefer handling the signals in one thread avoiding concurrent access.

The side effect of this bug was that some process were not leaving from
time to time during a reload.

This patch must be backported in 1.8.

diff --git a/src/haproxy.c b/src/haproxy.c
index 766758f628f34377b68cbb6f478995a94fbfd888..96b8abcf9b26b22fd75192d331359095d491e3c3 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2402,8 +2402,10 @@ static void run_poll_loop()
                /* Process a few tasks */
                process_runnable_tasks();
 
-               /* check if we caught some signals and process them */
-               signal_process_queue();
+               /* check if we caught some signals and process them in the
+                first thread */
+               if (tid == 0)
+                       signal_process_queue();
 
                /* Check if we can expire some tasks */
                next = wake_expired_tasks();
@@ -2418,7 +2420,7 @@ static void run_poll_loop()
                        activity[tid].wake_cache++;
                else if (active_tasks_mask & tid_bit)
                        activity[tid].wake_tasks++;
-               else if (signal_queue_len)
+               else if (signal_queue_len && tid == 0)
                        activity[tid].wake_signal++;
                else
                        exp = next;
@@ -3008,6 +3010,7 @@ int main(int argc, char **argv)
                unsigned int *tids    = calloc(global.nbthread, sizeof(unsigned int));
                pthread_t    *threads = calloc(global.nbthread, sizeof(pthread_t));
                int          i;
+               sigset_t     blocked_sig, old_sig;
 
                THREAD_SYNC_INIT((1UL << global.nbthread) - 1);
 
@@ -3015,6 +3018,15 @@ int main(int argc, char **argv)
                for (i = 0; i < global.nbthread; i++)
                        tids[i] = i;
 
+               /* ensure the signals will be blocked in every thread */
+               sigfillset(&blocked_sig);
+               sigdelset(&blocked_sig, SIGPROF);
+               sigdelset(&blocked_sig, SIGBUS);
+               sigdelset(&blocked_sig, SIGFPE);
+               sigdelset(&blocked_sig, SIGILL);
+               sigdelset(&blocked_sig, SIGSEGV);
+               pthread_sigmask(SIG_SETMASK, &blocked_sig, &old_sig);
+
                /* Create nbthread-1 thread. The first thread is the current process */
                threads[0] = pthread_self();
                for (i = 1; i < global.nbthread; i++)
@@ -3048,6 +3060,9 @@ int main(int argc, char **argv)
                }
 #endif /* !USE_CPU_AFFINITY */
 
+               /* when multithreading we need to let only the thread 0 handle the signals */
+               pthread_sigmask(SIG_SETMASK, &old_sig, NULL);
+
                /* Finally, start the poll loop for the first thread */
                run_thread_poll_loop(&tids[0]);
 

diff --git a/src/signal.c b/src/signal.c
index a0975910b1b300dba1938f89640b7ba4e1a40a1c..f1f682188353752af70500de9a06bbade6bc3cb1 100644 (file)
--- a/src/signal.c
+++ b/src/signal.c
@@ -31,7 +31,6 @@ struct pool_head *pool_head_sig_handlers = NULL;
 sigset_t blocked_sig;
 int signal_pending = 0; /* non-zero if t least one signal remains unprocessed */
 
-__decl_hathreads(HA_SPINLOCK_T signals_lock);
 
 /* Common signal handler, used by all signals. Received signals are queued.
  * Signal number zero has a specific status, as it cannot be delivered by the
@@ -71,9 +70,6 @@ void __signal_process_queue()
        struct signal_descriptor *desc;
        sigset_t old_sig;
 
-       if (HA_SPIN_TRYLOCK(SIGNALS_LOCK, &signals_lock))
-               return;
-
        /* block signal delivery during processing */
        sigprocmask(SIG_SETMASK, &blocked_sig, &old_sig);
 
@@ -100,7 +96,6 @@ void __signal_process_queue()
 
        /* restore signal delivery */
        sigprocmask(SIG_SETMASK, &old_sig, NULL);
-       HA_SPIN_UNLOCK(SIGNALS_LOCK, &signals_lock);
 }
 
 /* perform minimal intializations, report 0 in case of error, 1 if OK. */
@@ -112,8 +107,6 @@ int signal_init()
        memset(signal_queue, 0, sizeof(signal_queue));
        memset(signal_state, 0, sizeof(signal_state));
 
-       HA_SPIN_INIT(&signals_lock);
-
        /* Ensure signals are not blocked. Some shells or service managers may
         * accidently block all of our signals unfortunately, causing lots of
         * zombie processes to remain in the background during reloads.
@@ -148,7 +141,6 @@ void deinit_signals()
                        pool_free(pool_head_sig_handlers, sh);
                }
        }
-       HA_SPIN_DESTROY(&signals_lock);
 }
 
 /* Register a function and an integer argument on a signal. A pointer to the