python-trio / python-trio/trio

Should our strategy for handling KeyboardInterrupt be usable for other signals too?

Open
#134 8 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
7.3k
Forks
431
Avg merge
2d 17h
Merged PRs (30d)
6

Description

@hynek wants clean shutdown on SIGTERM, because runit uses that to kill supervised daemons: https://mobile.twitter.com/hynek/status/855318117630623744

What's our story here?

The Python default is that SIGINT does a clean(ish) shutdown, and SIGTERM blows away the process. I'm not really sure why that makes sense, but that's how it works, and ATM trio copies that. The it-just-works-like-Python factor is nice.

Of course one can do a manual clean shutdown on SIGTERM, by using trio.catch_signals – like the catch_signals example here: https://vorpus.org/blog/control-c-handling-in-python-and-trio/#what-if-you-want-a-manual-control-c-handler

One alternative someone might conceivably want would be to raise an exception in a similar way to what SIGINT does by default (possibly even KeyboardInterrupt, though that's a bit dirty since SIGTERM has nothing to do with the keyboard! It doesn't even have "interrupt" in the name...). In regular Python this is just a matter of installing a one-line signal handler. In trio it's... currently impossible. But I don't think it would be too hard to convert the current SIGINT machinery to a generic "deliver signal as an exception" machinery, so you could do, like, signal.signal(signal.SIGINT, trio.raising_signal_handler(KeyboardInterrupt)). Is this what people really want in production though? Maybe if combined with the restrict_keyboard_interrupt_to_checkpoints option... which then becomes somewhat misnamed. But if that's the only combination that makes sense, then maybe it shouldn't require multiple obscure switches to enable.

...there's also an argument that trio's default behavior should be to raise trio.TerminatedError or something on SIGTERM, which would be Another Weird Thing you have to learn when switching from regular python to trio, but would simplify this case to just setting the restrict_signal_exceptions_to_checkpoints option.

A potential downside would be if people expect SIGTERM to reliably kill wedged and looping processes, after SIGINT fails. I'm not sure how true this is: SIGINT/SIGTERM vs SIGKILL makes more sense to me as a distinction than SIGINT vs SIGTERM/SIGKILL.

Another idea that might be worth throwing into the soup is one that I thought a bit about before implementing trio's current SIGINT system, and someone on reddit reminded me of: having the Python-level handler keep track of how often it's been called, and if it gets called more than some threshold, switch to just killing the process (because it appears to be wedged), maybe after printing a message. This is probably only really useful for SIGINT though, since SIGTERM isn't generally used interactively...

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Trio's current SIGINT machinery and the trio.catch_signals example, then trace how signal handlers and checkpoint restriction are currently exposed. The issue is a design discussion, so done first requires a decided strategy for SIGTERM and other signals, followed by implementation and tests for the agreed behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.