mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-22 06:11:32 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: haproxy/threads: close a possible race in soft-stop detection

Browse Source 
Commit 4b3f27b ("BUG/MINOR: haproxy/threads: try to make all threads
leave together") improved the soft-stop synchronization but it left a
small race open because it looks at tasks_run_queue, which can drop
to zero then back to one while another thread picks the task from the
run queue to insert it into the tasklet_list. The risk is very low but
not null. In addition the condition didn't consider the possible presence
of signals in the queue.

This patch moves the stopping detection just after the "wake" calculation
which already takes care of the various queues' sizes and signals. It
avoids needlessly duplicating these tests.

The bug was discovered during a code review but will probably never be
observed. This fix may be backported to 2.1 and 2.0 along with the commit
above.
This commit is contained in:
Willy Tarreau 2020-03-23 09:27:28 +01:00
parent ce6fc25b17
commit 4f46a354e6
1 changed files with 10 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/haproxy.c
View File

@ -2806,14 +2806,6 @@ void run_poll_loop()
if (tid == 0) if (tid == 0)
signal_process_queue(); signal_process_queue();
if (stopping && tasks_run_queue == 0)
_HA_ATOMIC_OR(&stopping_thread_mask, tid_bit);
/* stop when there's nothing left to do */
if ((jobs - unstoppable_jobs) == 0 && tasks_run_queue == 0 &&
(stopping_thread_mask & all_threads_mask) == all_threads_mask)
break;
/* also stop if we failed to cleanly stop all tasks */ /* also stop if we failed to cleanly stop all tasks */
if (killed > 1) if (killed > 1)
break; break;
@ -2834,6 +2826,16 @@ void run_poll_loop()
wake = 0; wake = 0;
} }
if (!wake) {
if (stopping)
_HA_ATOMIC_OR(&stopping_thread_mask, tid_bit);
/* stop when there's nothing left to do */
if ((jobs - unstoppable_jobs) == 0 &&
(stopping_thread_mask & all_threads_mask) == all_threads_mask)
break;
}
/* If we have to sleep, measure how long */ /* If we have to sleep, measure how long */
next = wake ? TICK_ETERNITY : next_timer_expiry(); next = wake ? TICK_ETERNITY : next_timer_expiry();