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 766758f..96b8abc 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2402,8 +2402,10 @@
/* 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 @@
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 @@
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 @@
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 @@
}
#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 a097591..f1f6821 100644
--- a/src/signal.c
+++ b/src/signal.c
@@ -31,7 +31,6 @@
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 @@
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 @@
/* 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 @@
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 @@
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