Fix ParallelExecutor destructor deadlock - #173
copybara-service[bot] merged 1 commit into
Conversation
The destructor set next_task to 1 and num_tasks to 1, so the worker wait predicate (next_task < num_tasks) stayed false and workers never woke to observe the terminate flag. This deadlocked destruction of every ParallelExecutor. Store 0 instead so workers wake, observe terminate, and exit.
|
Hi, why it is draft? Do you plan to refine that? |
|
Hi eustas, thanks for checking in - sorry, it sat as a draft for longer than it should have. I opened it as a draft while I reproduced the deadlock locally and double-checked the change against the worker loop; that's done now, and the PR is marked ready for review. A quick recap of the change in case it helps: in I verified it with a standalone 4-thread reproduction: destruction hung before the change and completes normally after. All the CI checks for the PR are green. Could you please take another look when you have a chance? |
The
ParallelExecutordestructor never returns: it storesnext_task = 1while
num_tasks = 1, so the workers' wait predicatenext_task.load() < num_tasks(i.e.1 < 1) stays false and the workerthreads are never woken to observe the
terminateflag. Every constructionof
ParallelExecutor(e.g.cbrunsli/dbrunslibuilt withBRUNSLI_EXPERIMENTAL_GROUPS) therefore hangs forever at destruction.Fix: store
0instead, so0 < num_tasksis true, workers wake up,increment
busy_count, observeterminate, and exit, letting thedestructor's
finish_latch.waitcomplete.Verified with a standalone reproduction of the executor (4 threads, one
executeround): before the change it timed out at destruction; after thechange it exits cleanly.