Repository navigation
[v3-4-test] Fix LocalExecutor deadlock when a workload put blocks (#75086) - #76223
Merged
Merged
Conversation
…5086) * Keep draining LocalExecutor results while a workload put is blocked Draining the result queue once before each put did not remove the deadlock reported in #70526. A worker can write a result between the drain and the put; if the activity pipe is full at that point the put blocks, and the worker keeps writing results until the result pipe is full too, so each side waits on the other. Linux pipes free space only a page at a time, which is why the regression test from #70545 hangs on loaded CI runners but not on macOS. Make the put on a short-lived helper thread and keep reading results on the main thread until it completes, so the result pipe can never fill up behind a blocked put. The workload is registered as running before it is sent, so a "running" result read while the put is in flight is not dropped. Generated-by: Claude Opus 5 * Undo a failed LocalExecutor put and spawn workers while one is blocked Registering a workload before its put meant a put that raised left the key running and counted although nothing was sent; restore the queued workload and its bookkeeping instead. Counting each workload before its put keeps a worker from decrementing the outstanding count below zero when a batch is interrupted part way. A blocked put can only complete once a live worker reads the activity pipe, but workers were only spawned or replaced after the batch, so with the spawn start method (or once every worker had died) a batch larger than the pipe waited forever. Spawn them from the drain loop whenever fewer than parallelism are alive: spawn starts one per call, so a single busy worker would otherwise hold the put until its task finished. Generated-by: Claude Opus 5 * Cover undoing a failed put while an earlier dispatch of the key is running The test only reached the no-earlier-dispatch path, so the remaining > 0 branch of _undo_dispatch had no coverage. (cherry picked from commit 1d1d17b) Co-authored-by: Jarek Potiuk <jarek@potiuk.com> Generated-by: Claude Opus 5
1 task done
potiuk
marked this pull request as ready for review
November 1, 2026 12:33
potiuk
requested review from
dheerajturaga,
hussein-awala,
o-nikolas and
pierrejeambrun
as code owners
November 1, 2026 12:33
vatsrahul1001
pushed a commit
that referenced
this pull request
Nov 4, 2026
…5086) (#76223) * Keep draining LocalExecutor results while a workload put is blocked Draining the result queue once before each put did not remove the deadlock reported in #70526. A worker can write a result between the drain and the put; if the activity pipe is full at that point the put blocks, and the worker keeps writing results until the result pipe is full too, so each side waits on the other. Linux pipes free space only a page at a time, which is why the regression test from #70545 hangs on loaded CI runners but not on macOS. Make the put on a short-lived helper thread and keep reading results on the main thread until it completes, so the result pipe can never fill up behind a blocked put. The workload is registered as running before it is sent, so a "running" result read while the put is in flight is not dropped. Generated-by: Claude Opus 5 * Undo a failed LocalExecutor put and spawn workers while one is blocked Registering a workload before its put meant a put that raised left the key running and counted although nothing was sent; restore the queued workload and its bookkeeping instead. Counting each workload before its put keeps a worker from decrementing the outstanding count below zero when a batch is interrupted part way. A blocked put can only complete once a live worker reads the activity pipe, but workers were only spawned or replaced after the batch, so with the spawn start method (or once every worker had died) a batch larger than the pipe waited forever. Spawn them from the drain loop whenever fewer than parallelism are alive: spawn starts one per call, so a single busy worker would otherwise hold the put until its task finished. Generated-by: Claude Opus 5 * Cover undoing a failed put while an earlier dispatch of the key is running The test only reached the no-earlier-dispatch path, so the remaining > 0 branch of _undo_dispatch had no coverage. (cherry picked from commit 1d1d17b) Generated-by: Claude Opus 5 Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draining the result queue once before each put did not remove the
deadlock reported in #70526. A worker can write a result between the
drain and the put; if the activity pipe is full at that point the put
blocks, and the worker keeps writing results until the result pipe is
full too, so each side waits on the other. Linux pipes free space only a
page at a time, which is why the regression test from #70545 hangs on
loaded CI runners but not on macOS.
Make the put on a short-lived helper thread and keep reading results on
the main thread until it completes, so the result pipe can never fill up
behind a blocked put. The workload is registered as running before it is
sent, so a "running" result read while the put is in flight is not
dropped.
Generated-by: Claude Opus 5
Registering a workload before its put meant a put that raised left the
key running and counted although nothing was sent; restore the queued
workload and its bookkeeping instead. Counting each workload before its
put keeps a worker from decrementing the outstanding count below zero
when a batch is interrupted part way.
A blocked put can only complete once a live worker reads the activity
pipe, but workers were only spawned or replaced after the batch, so with
the spawn start method (or once every worker had died) a batch larger
than the pipe waited forever. Spawn them from the drain loop whenever
fewer than parallelism are alive: spawn starts one per call, so a single
busy worker would otherwise hold the put until its task finished.
Generated-by: Claude Opus 5
The test only reached the no-earlier-dispatch path, so the remaining > 0 branch of _undo_dispatch had no coverage.
(cherry picked from commit 1d1d17b)
Co-authored-by: Jarek Potiuk jarek@potiuk.com
Generated-by: Claude Opus 5