Le 20/06/2018 à 18:29, Willy Tarreau a écrit :
On Wed, Jun 20, 2018 at 04:42:58PM +0200, Christopher Faulet wrote:
When HAProxy is shutting down, it exits the polling loop when there is no jobs
anymore (jobs == 0). When there is no thread, it works pretty well, but when
HAProxy is started with several threads, a thread can decide to exit because
jobs variable reached 0 while another one is processing a task (e.g. a
health-check). At this stage, the running thread could decide to request a
synchronization. But because at least one of them has already gone, the others
will wait infinitly in the sync point and the process will never die.

Just a comment on this last sentence, I think this is the root cause of the
problem : a thread quits and its thread_mask bit doesn't disappear. In my
opinion if we're looping, it's precisely because there's no way by looking
at the all_threads_mask if some threads are missing. Thus I think that a more
reliable long term solution would require that each exiting thread does at
least "all_threads_mask &= ~tid_bit".

Now I have no idea whether or not the current sync point code is compatible
with this nor if this will be sufficient, but I'm pretty sure that over time
we'll have to go this way to fix this inconsistency.


Hi Willy,

You're right, removing the thread from all_threads_mask when it exits is a good idea. Here a patch to do so.

Thanks,
--
Christopher Faulet
>From cf238c63d4db1567756d388c7473122451adef17 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Thu, 21 Jun 2018 09:57:39 +0200
Subject: [PATCH] MINOR: threads: Be sure to remove threads from
 all_threads_mask on exit

When HAProxy is started with several threads, Each running thread holds a bit in
the bitfiled all_threads_mask. This bitfield is used here and there to check
which threads are registered to take part in a specific processing. So when a
thread exits, it seems normal to remove it from all_threads_mask.
---
 src/haproxy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/haproxy.c b/src/haproxy.c
index 5906f7989..11d1d47ce 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2473,6 +2473,8 @@ static void *run_thread_poll_loop(void *data)
 	list_for_each_entry(ptdf, &per_thread_deinit_list, list)
 		ptdf->fct();
 
+	HA_ATOMIC_AND(&all_threads_mask, ~tid_bit);
+
 #ifdef USE_THREAD
 	if (tid > 0)
 		pthread_exit(NULL);
-- 
2.17.1

Reply via email to