From f34a3fa33d519ffe57f5e9834d4b0b08ef438b3d Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 6 Jul 2022 10:13:05 +0200 Subject: [PATCH] BUG/MEDIUM: thread: mask stopping_threads with threads_enabled when checking it When soft-stopping, there's a comparison between stopping_threads and threads_enabled to make sure all threads are stopped, but this is not correct and is racy since the threads_enabled bit is removed when a thread is stopped but not its stopping_threads bit. The consequence is that depending on timing, when stopping, if the first stopping thread is fast enough to remove its bit from threads_enabled, the other threads will see that stopping_threads doesn't match threads_enabled anymore and will wait forever. As such the mask must be applied to stopping_threads during the test. This issue was introduced in recent commit ef422ced9 ("MEDIUM: thread: make stopping_threads per-group and add stopping_tgroups"), no backport is needed. --- src/haproxy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/haproxy.c b/src/haproxy.c index e8c209e95..f86e3b573 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -2840,7 +2840,8 @@ void run_poll_loop() (_HA_ATOMIC_LOAD(&stopping_tgroup_mask) & all_tgroups_mask) == all_tgroups_mask) { /* check that all threads are aware of the stopping status */ for (i = 0; i < global.nbtgroups; i++) - if (_HA_ATOMIC_LOAD(&ha_tgroup_ctx[i].stopping_threads) != ha_tgroup_info[i].threads_enabled) + if ((_HA_ATOMIC_LOAD(&ha_tgroup_ctx[i].stopping_threads) & ha_tgroup_info[i].threads_enabled) != + ha_tgroup_info[i].threads_enabled) break; #ifdef USE_THREAD if (i == global.nbtgroups) {