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 e8c209e95caeae6ef411198a42d9ea0b4c7f53e9..f86e3b5739a38a0b8fac81de2ebaf4c4a4455cee 100644 (file)
--- 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) {