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

BUG/MINOR: debug: fix a small race in the thread dumping code

Browse Source 
If a thread dump is requested from a signal handler, it may interrupt
a thread already waiting for a dump to complete, and may see the
threads_to_dump variable go to zero while others are waiting, steal
the lock and prevent other threads from ever completing. This tends
to happen when dumping many threads upon a watchdog timeout, to threads
waiting for their turn.

Instead now we proceed in two steps :
  1) the last dumped thread sets all bits again
  2) all threads only wait for their own bit to appear, then clear it
     and quit

This way there's no risk that a bit performs a double flip in the same
loop and threads cannot get stuck here anymore.

This should be backported to 2.0 as it clarifies stack traces.
This commit is contained in:
Willy Tarreau 2019-07-31 19:15:45 +02:00
parent a8c73748f8
commit c07736209d
1 changed files with 9 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/debug.c
View File

@ -439,8 +439,8 @@ void debug_handler(int sig, siginfo_t *si, void *arg)
* 1- wait for our turn, i.e. when all lower bits are gone. * 1- wait for our turn, i.e. when all lower bits are gone.
* 2- perform the action if our bit is set * 2- perform the action if our bit is set
* 3- remove our bit to let the next one go, unless we're * 3- remove our bit to let the next one go, unless we're
* the last one and have to put them all but ours * the last one and have to put them all as a signal
* 4- wait for zero and clear our bit if it's set * 4- wait out bit to re-appear, then clear it and quit.
*/ */
/* wait for all previous threads to finish first */ /* wait for all previous threads to finish first */
@ -453,7 +453,7 @@ void debug_handler(int sig, siginfo_t *si, void *arg)
ha_thread_dump(thread_dump_buffer, tid, thread_dump_tid); ha_thread_dump(thread_dump_buffer, tid, thread_dump_tid);
if ((threads_to_dump & all_threads_mask) == tid_bit) { if ((threads_to_dump & all_threads_mask) == tid_bit) {
/* last one */ /* last one */
HA_ATOMIC_STORE(&threads_to_dump, all_threads_mask & ~tid_bit); HA_ATOMIC_STORE(&threads_to_dump, all_threads_mask);
thread_dump_buffer = NULL; thread_dump_buffer = NULL;
} }
else else
@ -461,14 +461,13 @@ void debug_handler(int sig, siginfo_t *si, void *arg)
} }
/* now wait for all others to finish dumping. The last one will set all /* now wait for all others to finish dumping. The last one will set all
* bits again to broadcast the leaving condition. * bits again to broadcast the leaving condition so we'll see ourselves
* present again. This way the threads_to_dump variable never passes to
* zero until all visitors have stopped waiting.
*/ */
while (threads_to_dump & all_threads_mask) { while (!(threads_to_dump & tid_bit))
if (threads_to_dump & tid_bit)
HA_ATOMIC_AND(&threads_to_dump, ~tid_bit);
else
ha_thread_relax(); ha_thread_relax();
} HA_ATOMIC_AND(&threads_to_dump, ~tid_bit);
/* mark the current thread as stuck to detect it upon next invocation /* mark the current thread as stuck to detect it upon next invocation
* if it didn't move. * if it didn't move.