classification
Title: Python doesn't exit with proper resultcode on SIGINT
Type: enhancement Stage: commit review
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: SamB, eryksun, foom, gregory.p.smith, jwilk, loewis, martin.panter, mwh, petri.lehtinen, pitrou, rnk, vstinner
Priority: normal Keywords: needs review, patch

Created on 2004-10-25 20:27 by foom, last changed 2019-02-23 18:43 by gregory.p.smith. This issue is now closed.

Files
File name Uploaded Description Edit
issue1054041-sigint-exit-gps01.diff gregory.p.smith, 2011-01-04 03:21
sig.py gregory.p.smith, 2019-02-14 19:31
winsig.bat eryksun, 2019-02-15 01:48
winsig.py eryksun, 2019-02-15 01:48
Pull Requests
URL Status Linked Edit
PR 11862 merged gregory.p.smith, 2019-02-15 07:52
PR 11963 merged gregory.p.smith, 2019-02-21 00:33
PR 11999 merged gregory.p.smith, 2019-02-23 18:27
Messages (23)
msg60590 - (view) Author: James Y Knight (foom) Date: 2004-10-25 20:27
If you kill a python process with SIGINT (e.g. control-c), it catches 
the signal, and raises a KeyboardInterrupt. If the KeyboardInterrupt 
propagates to the top level, python exits. However, it exits with a 
result of 1, not a result of SIGINT. This means that if you run 
python in a shell script, the script will not properly exit on C-c.

When exiting because of a KeyboardInterrupt, python ought to 
reraise the SIGINT, as follows, so that the exit code is correct for 
the caller:
        signal(SIGINT, SIG_DFL);
        kill(getpid(), SIGINT);

See also http://www.cons.org/cracauer/sigint.html for a more 
detailed discussion on the topic.
msg60591 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2005-04-07 09:26
Logged In: YES 
user_id=6656

Feel like writing a patch?
msg114385 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-19 17:59
I'll close this in a couple of weeks unless someone wants it kept open.
msg125287 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2011-01-04 03:21
Here's a patch that implements this behavior.  It is too late in the 3.2 beta/rc cycle to get this into 3.2.  Consider it for 3.3.  I'd like a review.
msg125593 - (view) Author: Reid Kleckner (rnk) (Python committer) Date: 2011-01-06 21:32
Looks good to me.  Do you need the TODO(gps)'s in there after implementing the behavior described?
msg125596 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-06 22:29
- should KeyboardInterrupt always exit with SIGINT, or only if it was actually raised by a signal handler?
- if _Py_UnhandledKeyboardInterrupt is defined in Modules/main.c but used in Python/pythonrun.c, can't it cause linker errors when embedding Python?
- please use PEP 8 (testKeyboardInterruptSignalExit -> test_some_long_name)
msg125605 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-06 23:44
I wonder whether there is a precedent of some system mapping SIGINT to
an exception. We could probably learn something from them.

> - should KeyboardInterrupt always exit with SIGINT, or only if it was
> actually raised by a signal handler?

IMO, if we give the illusion that the interpreter was actually killed,
we should equate KeyboardInterrupt with SIGINT; any uncaught
KeyboardInterrupt should consequently always lead to raising SIGINT.
msg125624 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2011-01-07 05:08
> IMO, if we give the illusion that the interpreter was actually killed,
> we should equate KeyboardInterrupt with SIGINT; any uncaught
> KeyboardInterrupt should consequently always lead to raising SIGINT.

Agreed.  Plus that is easier to implement and what I did.

I'll remove the left over TODO(gps) comments (oops) before this is
ever committed. I'm waiting until after 3.2 is released unless the
release manager jumps in and says otherwise.  remaining items:

 1. I need to add a second test case that writes the code to a file
and launches a subprocess executing that file instead of using -c
given that they are different code paths that each need testing.  For
variety I'll probably make that one send an actual SIGINT to the child
process rather than having it raise KeyboardInterrupt.

 2. The tests probably needs a decorator to limit their execution to posix.

 3. Do the signal and kill calls also need to be conditional based on
platform or is the function I put them in already posix-only?  If
necessary I'll protect them with #ifdefs so they don't break a windows
build.
msg136416 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2011-05-21 00:31
+        kill(getpid(), SIGINT);

kill() doesn't exist on Windows: use raise() which is more portable and doesn't require a PID argument.

We may need to do something on Windows for console applications: see SetConsoleCtrlHandler(),
http://msdn.microsoft.com/en-us/library/ms686016(v=vs.85).aspx

+        self.assertEqual(returncode, -signal.SIGINT,
+                         "not a SIGINT exit code. process stderr:\n%s" % stderr)

I don't think that such test can pass on Windows.
msg252910 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-13 04:27
This covers the same ground as Issue 14229, which was rejected with the argument that re-raising the signal is “ugly” and bypasses some things that are done by the standard OS exit() function.
msg335481 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-02-13 23:58
Taking a renewed look at this 8 years later... I agree, re-triggering the signal with the SIG_DFL handler would prevent us from doing the existing interpreter shutdown cleanup if we did it too early which would be a behavior change other than the exit value correction.

So we should delay the re-signaling kill(getpid(), SIGINT) call until we've completed that and are about to exit anyways.

The code has moved around a lot since i generated this patch on a 3.2-ish tree so it'll take me a while to untangle what would need to go where to create a PR.

Instead of kill(getpid(), SIGINT) or raise(SIGINT), we could just checking the _UnhandledKeyboardInterrupt flag we return our exit valye adjusting it to be the one a calling shell may be expecting to indicate a SIGINT.  That goes against the advice of https://www.cons.org/cracauer/sigint.html but is likely to work.

A question that leads to is what _is_ the correct value.  On Linux the magic 130 happens to be (SIGINT + 128).  Triggering the libc or kernel supplied SIG_DFL handler as a final act avoids us ever needing to know what possible mechanisms to indicate this to the parent process are preferred on a platform.  (if we know it is merely an exit value we could capture that in to a #define with a configure.ac check if the + 128 trick were deemed too magical despite being what everyone likely implements, standardized or not)
msg335482 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-02-14 00:06
> A question that leads to is what _is_ the correct value.

What do you think of using a short test program (ex: uses raise(SIGINT)) in ./configure to get the "default exit code" to define a constant, and then use the constant for exit() in Python?

I dislike the idea of raising signals in general. The exact behavior of signals depends too much on the OS, it's hard to get it right. But having a configurable exit code looks safe and simple enough.
msg335485 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-02-14 00:16
I expect that'll work as desired and avoids the re-raising of the signal.

Unless told otherwise I assume this should be a POSIX specific platform behavior.  ie: no return value alteration due to an uncaught KeyboardInterrupt on the Windows API build.
msg335488 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-02-14 01:24
For Windows, the default console control handler calls ExitProcess(STATUS_CONTROL_C_EXIT). If CMD is waiting on an application that exits with STATUS_CONTROL_C_EXIT, it prints "^C" to indicate the process was killed by Ctrl+C. For example:

    >>> STATUS_CONTROL_C_EXIT = 0xC000013A - 2**32
    >>> STATUS_CONTROL_C_EXIT
    -1073741510
    >>> sys.exit(STATUS_CONTROL_C_EXIT)
    ^C

    C:\>echo %errorlevel%
    -1073741510

Note that switching to SIG_DFL with raise(SIGINT) does not invoke the default console control handler in Windows. It just invokes the default raise() behavior, which is to call _exit(3). This exit status value of 3 is arbitrary and meaningless.
msg335516 - (view) Author: Jakub Wilk (jwilk) Date: 2019-02-14 10:53
This issue was reported because with the current behavior, "the script will not properly exit on C-c". Exiting with code 128+SIGINT will not fix this.

Please re-raise the signal.
msg335517 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2019-02-14 11:01
I'm assuming the calling shell uses waitpid() and then WIFSIGNALED()?
msg335556 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-02-14 19:31
jwilk: Confirmed.  The exit code is not enough, we must trigger the SIG_DFL handler.

to reproduce:

 while true; do ./sig.py ; done

with the attached sig.py.

pass a parameter to sig.py to have it exit 130 instead of triggering SIG_DFL...
msg335577 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-02-15 01:48
Gregory's last example reminded me that CMD checks for STATUS_CONTROL_C_EXIT for more than simply printing "^C". It also breaks out of a FOR loop when interactive and prompts to continue when executing a batch script.

Normally CMD also gets a console control event when the user presses Ctrl+C, so it knows about the Ctrl+C regardless of the child's exit status. One exception is when we start a process with a new console via CMD's `start` command. In this case CMD doesn't get a Ctrl+C event, since it's attached to a different console. Another exception is a simulated keyboard interrupt (e.g. from C raise SIGINT or Python _thread.interrupt_main). In these cases, CMD depends on the exit status value to determine whether the process was terminated by the default Ctrl+C handler. I've demonstrated this in the files winsig.bat and winsig.py. Put both in the same directory and run winsig.bat.
msg335721 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-02-16 20:57
New changeset 38f11cc3f62db11a4a24354bd06273322ac91afa by Gregory P. Smith in branch 'master':
bpo-1054041: Exit properly after an uncaught ^C. (#11862)
https://github.com/python/cpython/commit/38f11cc3f62db11a4a24354bd06273322ac91afa
msg335723 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-02-16 21:08
This is in for 3.8.

On earlier or unpatched Python versions: application owners have a workaround f they do not mind skipping a clean application shutdown (destructors) on posix platforms:
catch KeyboardInterrupt, reset SIGINT to SIG_DFL, kill(getpid(), SIGINT).  If you somehow need the python destructor cleanup (never guaranteed, so unwise to _depend_ on it) you could do that in a C atexit handler.

On Windows the workaround is easier without altering clean shutdown, catch KeyboardInterrupt and sys.exit(0xC000013A - 2**32).
msg335927 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-02-19 11:35
> if (PyOS_setsig(SIGINT, SIG_DFL) == SIG_ERR) {
>    perror("signal");  /* Impossible in normal environments. */

In my experience, Python is were impossible things happen. Would it be possible to write a better error message to explain what happened?

I expect that if PyOS_setsig() fails, the user cannot do much for that. It may be a bug in the libc and so cannot be fixed easily. Maybe we should simply ignore the error?
msg336024 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-02-19 23:29
i'm curious if this _ever_ comes up so i'd like to leave it in at least through some betas.  I don't believe it should be possible.  If it is, it'd probably be a restricted execution environment that fails all signal() calls.  all perror will do is emit an error message to that effect before continuing so we still exit with an error code as the final fallback.
msg336399 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-02-23 18:43
New changeset 06babb24225d41a76e4aee975380294ca1ee1d7c by Gregory P. Smith in branch 'master':
bpo-1054041: Add What's New docs. (GH-11999)
https://github.com/python/cpython/commit/06babb24225d41a76e4aee975380294ca1ee1d7c
History
Date User Action Args
2019-02-23 18:43:51gregory.p.smithsetmessages: + msg336399
2019-02-23 18:27:06gregory.p.smithsetpull_requests: + pull_request12029
2019-02-21 00:33:04gregory.p.smithsetpull_requests: + pull_request11989
2019-02-19 23:29:31gregory.p.smithsetmessages: + msg336024
2019-02-19 11:35:52vstinnersetmessages: + msg335927
2019-02-16 21:08:03gregory.p.smithsetstatus: open -> closed
messages: + msg335723

assignee: gregory.p.smith
resolution: fixed
stage: patch review -> commit review
2019-02-16 20:57:42gregory.p.smithsetmessages: + msg335721
2019-02-15 07:52:17gregory.p.smithsetstage: needs patch -> patch review
pull_requests: + pull_request11895
2019-02-15 01:48:56eryksunsetfiles: + winsig.py
2019-02-15 01:48:13eryksunsetfiles: + winsig.bat

messages: + msg335577
2019-02-14 19:31:02gregory.p.smithsetfiles: + sig.py

messages: + msg335556
2019-02-14 11:01:35pitrousetmessages: + msg335517
2019-02-14 10:53:06jwilksetmessages: + msg335516
2019-02-14 01:24:38eryksunsetnosy: + eryksun
messages: + msg335488
2019-02-14 00:16:06gregory.p.smithsetmessages: + msg335485
2019-02-14 00:06:50vstinnersetmessages: + msg335482
2019-02-14 00:02:17gregory.p.smithsetstage: patch review -> needs patch
2019-02-13 23:58:34gregory.p.smithsetmessages: + msg335481
2019-02-13 23:33:28gregory.p.smithsetversions: + Python 3.8, - Python 3.3
2015-10-13 04:27:03martin.pantersetnosy: + martin.panter
messages: + msg252910
2014-05-28 20:04:04SamBsetnosy: + SamB
2013-05-25 15:39:08jwilksetnosy: + jwilk
2011-07-02 20:01:21petri.lehtinensetstage: needs patch -> patch review
2011-05-21 00:31:56vstinnersetnosy: + vstinner
messages: + msg136416
2011-05-20 06:34:20petri.lehtinensetnosy: + petri.lehtinen
2011-01-07 05:08:16gregory.p.smithsetnosy: mwh, loewis, gregory.p.smith, foom, pitrou, rnk
messages: + msg125624
2011-01-06 23:44:04loewissetnosy: mwh, loewis, gregory.p.smith, foom, pitrou, rnk
messages: + msg125605
2011-01-06 22:29:15pitrousetnosy: + pitrou, loewis
messages: + msg125596
2011-01-06 21:32:10rnksetnosy: mwh, gregory.p.smith, foom, rnk
messages: + msg125593
2011-01-04 03:21:10gregory.p.smithsetfiles: + issue1054041-sigint-exit-gps01.diff

messages: + msg125287
keywords: + needs review, patch
nosy: mwh, gregory.p.smith, foom, rnk
2011-01-04 01:29:01pitrousetnosy: + rnk
2011-01-04 01:28:56pitrousetnosy: + gregory.p.smith, - BreamoreBoy
stage: test needed -> needs patch
type: behavior -> enhancement
versions: + Python 3.3, - Python 3.1, Python 2.7, Python 3.2
2010-08-19 22:14:59skrahsetstatus: pending -> open
2010-08-19 17:59:03BreamoreBoysetstatus: open -> pending
versions: + Python 3.1, Python 3.2
nosy: + BreamoreBoy

messages: + msg114385

type: enhancement -> behavior
2009-02-14 18:19:07ajaksu2setstage: test needed
type: enhancement
versions: + Python 2.7
2004-10-25 20:27:22foomcreate