Skip to content

[supervisor] Terminate reparented processes during shutdown #2920

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

Merged
merged 4 commits into from
Jan 15, 2021

Conversation

csweichel
Copy link
Contributor

@csweichel csweichel commented Jan 15, 2021

This PR provides the same SIGTERM/SIGKILL shutdown mechanism to background processes started in terminals, as we provide to the terminals themselves.

As a consequence, one can now start tasks like:

tasks:
  - before: sudo docker-up &
    # convenience for non-prebuild workspaces, not necessary to make this termination mechanism work
    command: fg

and expect the Docker daemon to terminate gracefully on workspace shutdown (within the terminal shutdown time: 10 seconds).

This PR also slightly extends the supervisor terminal API and introduces annotations on terminals (user-defined metadata). We now also report the terminal shell's PID.

How does it work

In bash, background processes get disowned when the shell is terminated, and subsequently reparented to their parent process (i.e. supervisor). During supervisor shutdown, we now make the reaper send sigterm to any process that gets reparented to supervisor and is still running. In addition, we explicitly SIGTERM all child processes prior to shutting down. This includes former bash background processes.

How to test

  1. (Optional) Enable Feature Preview ... this adds another test for [supervisor] execve into ring3 #2664
  2. Start a workspace on this test repo: https://cw-supervisor-2654.staging.gitpod-dev.com/#https://github.com/csweichel/test-repo/tree/proper-shutdown
  3. Stop the workspace and notice the "proper-shutdown.txt" file in the list of dirty files

@csweichel csweichel marked this pull request as ready for review January 15, 2021 10:40
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

It works nicely with and without feature preview. Code changes look good too!

@csweichel csweichel merged commit a495679 into master Jan 15, 2021
@csweichel csweichel deleted the cw/supervisor-2654 branch January 15, 2021 13:11
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