From: Willy Tarreau <w@1wt.eu>
Date: Wed, 6 Jul 2022 08:13:05 +0000 (+0200)
Subject: BUG/MEDIUM: thread: mask stopping_threads with threads_enabled when checking it
X-Git-Tag: v2.7-dev2~93
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f34a3fa33d519ffe57f5e9834d4b0b08ef438b3d;p=thirdparty%2Fhaproxy.git

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.
---

diff --git a/src/haproxy.c b/src/haproxy.c
index e8c209e95c..f86e3b5739 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) {