Skip to content

Kill process group on flow run cancellation#21112

Open
wavebyrd wants to merge 9436 commits intoPrefectHQ:mainfrom
wavebyrd:fix/kill-process-group-on-cancel
Open

Kill process group on flow run cancellation#21112
wavebyrd wants to merge 9436 commits intoPrefectHQ:mainfrom
wavebyrd:fix/kill-process-group-on-cancel

Conversation

@wavebyrd
Copy link

Summary

Fixes #20979.

When a flow run is cancelled, the worker sends SIGTERM only to the flow run process itself. Any child processes (like those from ShellOperation) are left orphaned.

This PR:

  • Starts flow run subprocesses in their own process group (start_new_session=True on POSIX)
  • Switches cancellation signals from os.kill to os.killpg, targeting the whole process group

This covers all three cancellation/kill paths: Runner._kill_process, ProcessManager.kill, and Runner.reschedule_current_flow_runs. The new EngineCommandStarter path also gets start_new_session=True.

Windows behavior is unchanged (still uses CTRL_BREAK_EVENT).

Test plan

  • Updated existing test__process_manager.py tests to verify killpg/getpgid usage
  • Manual: deploy flow with ShellOperation(commands=["sleep 9999"]) to a process work pool, cancel via UI, confirm sleep process is terminated

devin-ai-integration bot and others added 30 commits February 17, 2026 12:25
…efectHQ#20718)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…#20716)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…#20720)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…fectHQ#20692)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Nate Nowack <[email protected]>
…refectHQ#20725)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…age E2E tests (PrefectHQ#20726)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…0727)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…ctHQ#20730)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…refectHQ#20653)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… link E2E tests (PrefectHQ#20737)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…` field for programmatic resume (PrefectHQ#20741)

Co-authored-by: Claude Sonnet 4.6 <[email protected]>
…fectHQ#20744)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Nate Nowack <[email protected]>
…trator` (PrefectHQ#20757)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
…#20761)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
devin-ai-integration bot and others added 19 commits March 11, 2026 11:20
…refectHQ#21086)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Nate Nowack <[email protected]>
…21085)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Alexander Streed <[email protected]>
Co-authored-by: alex.s <[email protected]>
…1090)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: alex.s <[email protected]>
Co-authored-by: alex.s <[email protected]>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: alex.s <[email protected]>
Co-authored-by: alex.s <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: bot_apk <[email protected]>
Start flow run subprocesses in their own process group
(start_new_session=True) and use os.killpg instead of os.kill when
cancelling. This ensures child processes spawned by ShellOperation
(or anything else) get terminated along with the flow run process,
instead of being orphaned.

Fixes PrefectHQ#20979
@github-actions github-actions bot added the bug Something isn't working label Mar 13, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 13, 2026

Merging this PR will not alter performance

✅ 2 untouched benchmarks


Comparing wavebyrd:fix/kill-process-group-on-cancel (2ec67b1) with main (447059a)

Open in CodSpeed

Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

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

Thanks for opening a PR @wavebyrd!

Please run ruff on your changes so that the static analysis stage in our checks passes. Also, it looks like these changes are causing the ' pytest ' process to be killed during the test run. That will need to be fixed before we can approve and merge this PR.

@wavebyrd wavebyrd force-pushed the fix/kill-process-group-on-cancel branch from 2ec67b1 to 813f9b0 Compare March 13, 2026 21:37
@wavebyrd wavebyrd requested a review from aaazzam as a code owner March 13, 2026 21:37
@github-actions github-actions bot added upstream dependency An upstream issue caused by a bug in one of our dependencies migration labels Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working migration upstream dependency An upstream issue caused by a bug in one of our dependencies