Skip to content

fix(ipc): close supervised process when start() is cancelled - #6051

Merged
longcw merged 2 commits into
mainfrom
longc/fix-cancelled-start-leak
Jun 12, 2026
Merged

fix(ipc): close supervised process when start() is cancelled#6051
longcw merged 2 commits into
mainfrom
longc/fix-cancelled-start-leak

Conversation

@longcw

@longcw longcw commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

test_slow_initialization flakes on CI with a leaked ProcJobExecutor._supervise_task at teardown. The root cause is a real shutdown race in SupervisedProc, not a test issue.

When the owning task (e.g. ProcPool._proc_spawn_task during pool close) is cancelled while awaiting start(), the shielded _start() keeps running and creates the supervise task afterwards. The cleanup path then calls aclose(), which no-ops because the proc isn't marked started yet — leaking the supervise task and the child process. The orphaned child blocks forever waiting for InitializeRequest, and its non-daemon join thread can hang worker shutdown indefinitely.

Changes

  • start() keeps a handle on the shielded _start() task; aclose() waits for it before checking started, so an abandoned start can't race past the check.
  • aclose() kills a never-initialized process instead of attempting a graceful shutdown: the child reads InitializeRequest before servicing any other message, so a ShutdownRequest could never be acked.
  • kill() resolves a pending _initialize_fut so the supervise task (which waits on it before supervising) can observe the process exit.
  • Added test_aclose_after_cancelled_start, which deterministically reproduces the leak (fails the leaked-tasks check and hangs pytest exit without the fix).

nit: Also sets the agent session tests to speed = 1 and raises the default drain_delay — they run under virtual time, so the speed factor no longer buys wall-clock time.

longcw added 2 commits June 11, 2026 12:08
When the owning task is cancelled while awaiting start(), the shielded _start() keeps running and creates the supervise task afterwards, but aclose() no-ops because the proc isn't marked started yet. This leaks the supervise task and the child process, and can hang worker shutdown (surfaced as a flaky teardown failure of test_slow_initialization on slow CI runners).

aclose() now waits for the in-flight _start() before checking started, and kills the process when it was never initialized: the child reads InitializeRequest before servicing any other message, so a graceful ShutdownRequest could never be acked. kill() resolves the pending initialize future so the supervise task can observe the process exit.
speed_factor no longer buys wall-clock time now that the session tests run under virtual time, so set it to 1 and raise the default drain_delay to 5s for more headroom at no cost.
@chenghao-mou
chenghao-mou requested a review from a team June 11, 2026 04:09

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@longcw
longcw merged commit a677ddf into main Jun 12, 2026
26 checks passed
@longcw
longcw deleted the longc/fix-cancelled-start-leak branch June 12, 2026 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants