Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactive processes kill the process and its children on Ctrl+C. #13678

Merged
merged 2 commits into from
Nov 19, 2021

Conversation

stuhood
Copy link
Member

@stuhood stuhood commented Nov 19, 2021

As described in #12603, an interactive process which spawns children will currently orphan those children when it is killed. Despite spawning processes with daemon=True, the example in the description works when run directly as a PEX because the shell interrupts all processes in the parent's process group.

To get to a place where everything is terminated (even if not yet as gracefully as we would like: see #13230), this change wraps interactive processes in a new ManagedChild wrapper, which helps to ensure that the root process is in a new PGID, and then kills that PGID on drop.

If this strategy proves to be useful, we can extend it to nailgun servers and local processes, but this is kept intentionally small for cherry-picking.

[ci skip-build-wheels]

@stuhood stuhood changed the title Add a process wrapper to kill a process and its children on Drop. Wrap interactive processes to kill a process and its children on Drop. Nov 19, 2021
@stuhood stuhood force-pushed the stuhood/kill-pgroup-on-drop branch from 4ccc1eb to 5de2701 Compare November 19, 2021 04:55
@stuhood stuhood changed the title Wrap interactive processes to kill a process and its children on Drop. Interactive processes kill the process and its children on Ctrl+C. Nov 19, 2021
@stuhood stuhood marked this pull request as ready for review November 19, 2021 05:11
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@stuhood stuhood force-pushed the stuhood/kill-pgroup-on-drop branch from 5de2701 to e6048e4 Compare November 19, 2021 05:20
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