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

Allow killing processes with SIGTERM #358

Closed
wants to merge 13 commits into from
Closed

Allow killing processes with SIGTERM #358

wants to merge 13 commits into from

Conversation

lionel-
Copy link
Member

@lionel- lionel- commented Apr 6, 2023

Branched from #357

  • Add cleanup_signal argument to run() and process$new(). This instructs processx to use softer termination on GC so that the callr cleanup handler may run.

  • Add signal arguments to kill() and kill_tree() methods. Used internally to support the above, and externally by event loops.

  • Since the kill signal may now be ignored by the process, poll for SIGCHLD with a timeout. This is implemented using process_wait() and tested with a new dynlib sigtermignore that installs a SIGTERM handler that ignores the signal.

I'm not 100% happy with the API implemented here. I can call kill_tree(signal = SIGTERM) in an event loop and register a subsequent SIGKILL with a delay, this part is fine. However, when processes created with cleanup_signal = SIGTERM fail to properly quit on GC, the processes might not be killed. Also it seems that close_connections = TRUE also causes the process to finish (e.g. it causes processes linked to sigtermignore to terminate), and I'm not sure how this interacts with SIGTERM cleanup.

Would it make sense to revive the grace argument and do staged termination with SIGTERM and SIGKILL by default?

Base automatically changed from sigterm-cleanup to main April 14, 2023 13:27
@lionel-
Copy link
Member Author

lionel- commented Apr 27, 2023

Would it make sense to revive the grace argument and do staged termination with SIGTERM and SIGKILL by default?

Done in #367 and r-lib/ps#149.

@lionel- lionel- closed this Apr 27, 2023
@lionel- lionel- deleted the kill-tree-signal branch April 27, 2023 11:04
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.

1 participant