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

On KeyboardInterrupt, the exit code should mirror the signal number #58437

Closed
pitrou opened this issue Mar 8, 2012 · 10 comments
Closed

On KeyboardInterrupt, the exit code should mirror the signal number #58437

pitrou opened this issue Mar 8, 2012 · 10 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented Mar 8, 2012

BPO 14229
Nosy @loewis, @pitrou, @merwok

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-10-13.22:26:13.579>
created_at = <Date 2012-03-08.20:21:24.946>
labels = ['interpreter-core', 'type-feature']
title = 'On KeyboardInterrupt, the exit code should mirror the signal number'
updated_at = <Date 2012-10-13.22:26:13.578>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2012-10-13.22:26:13.578>
actor = 'nadeem.vawda'
assignee = 'none'
closed = True
closed_date = <Date 2012-10-13.22:26:13.579>
closer = 'nadeem.vawda'
components = ['Interpreter Core']
creation = <Date 2012-03-08.20:21:24.946>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 14229
keywords = []
message_count = 10.0
messages = ['155174', '155209', '155244', '155245', '155353', '155354', '156446', '156447', '156458', '157304']
nosy_count = 7.0
nosy_names = ['loewis', 'pitrou', 'nadeem.vawda', 'eric.araujo', 'Arfrever', 'neologix', 'rosslagerwall']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14229'
versions = ['Python 3.3']

@pitrou
Copy link
Member Author

pitrou commented Mar 8, 2012

Compare:

$ ./python -c "import subprocess, signal, time; p = subprocess.Popen(['cat']); time.sleep(1); p.send_signal(signal.SIGINT); print(p.wait())"
-2

with:

$ ./python -c "import subprocess, signal, time; p = subprocess.Popen(['python', '-c', 'input()']); time.sleep(1); p.send_signal(signal.SIGINT); print(p.wait())"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
KeyboardInterrupt
1

Python's behaviour apparently breaks a common assumption towards Unix processes (see bug reported at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=652926). A solution would be to add a signal number attribute to KeyboardInterrupt, and use that value when computing the process exit code.

@pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Mar 8, 2012
@loewis
Copy link
Mannequin

loewis mannequin commented Mar 9, 2012

I'm not so sure that Python is in violation of the convention here. The exit code should report *unhandled* signals, indicating that the process didn't provide an exit code at all (as it didn't call exit(2)). You are supposed to use WIFEXITED/WIFSIGNALED/WIFSTOPPED on the status code, and interpret it as an exit code only if WIFEXITED.

Since Python *does* handle the signal, and exits "regularly", we shouldn't (and probably even can't) claim that we didn't exit (unless we raise the signal again instead of exiting).

@pitrou
Copy link
Member Author

pitrou commented Mar 9, 2012

Another problem with my suggestion is that C exit() ANDs the status with 255 before transmitting it to the parent.

@pitrou
Copy link
Member Author

pitrou commented Mar 9, 2012

(faulthandler works around that problem by restoring the previous signal handler and calling raise(), apparently)

@merwok
Copy link
Member

merwok commented Mar 10, 2012

The Debian bug has some more convincing examples:

any Python program which is expected to implement the interface of (say) "diff" is buggy, because
it may exit status 1 ("comparison successful; differences found") when it should have died with
SIGINT ("comparison not completed due to interrupt signal"); this could in principle cause data loss
in some applications.

@pitrou
Copy link
Member Author

pitrou commented Mar 10, 2012

Since Python *does* handle the signal, and exits "regularly", we
shouldn't

You are arguing from a legal point of view, but from a pratical point of view Python doesn't really "handle" the signal: it just defers the exit until after all internal structures are cleaned up.

@neologix
Copy link
Mannequin

neologix mannequin commented Mar 20, 2012

I agree with Martin: we really do handle the signal, and as such, the
only way to convey the relevant information to the parent as to which
signal caused the exit would be to re-raise it, which is really ugly
and probably not a good idea.

Processes that want default behavior upon signal reception (so that
they can use WEXITSTATUS(), WCOREDUMP() and friends) can always use
SIG_DFL:

$ python -c "import subprocess, signal, time; p =
subprocess.Popen(['python', '-c', 'import signal;
signal.signal(signal.SIGINT, signal.SIG_DFL); input()']);
time.sleep(1); p.send_signal(signal.SIGINT); print(p.wait())"
-2

@pitrou
Copy link
Member Author

pitrou commented Mar 20, 2012

I agree with Martin: we really do handle the signal, and as such, the
only way to convey the relevant information to the parent as to which
signal caused the exit would be to re-raise it, which is really ugly
and probably not a good idea.

Why would it be ugly? faulthandler does exactly that.

@neologix
Copy link
Mannequin

neologix mannequin commented Mar 20, 2012

> I agree with Martin: we really do handle the signal, and as such, the
> only way to convey the relevant information to the parent as to which
> signal caused the exit would be to re-raise it, which is really ugly
> and probably not a good idea.

Why would it be ugly? faulthandler does exactly that.

Because calling exit() is the right way to end a process. For example,
it does the following:

  • atexit()-registered finalizers are run
  • stdio streams are flushed and closed (although it could probably
    done by the interpreter)
  • files created with tmpfile() are removed (on POSIX systems, they're
    removed after creation, but you can imagine an implementation where
    they would need to be explicitely removed upon close)

This would not be performed if the signal is raised.
Since the user has the possibility of restoring default signal
handlers with SIG_DFL, I think we could stcik with the current
behavior.

@pitrou
Copy link
Member Author

pitrou commented Apr 1, 2012

Because calling exit() is the right way to end a process. For example,
it does the following:

  • atexit()-registered finalizers are run
  • stdio streams are flushed and closed (although it could probably
    done by the interpreter)
  • files created with tmpfile() are removed (on POSIX systems, they're
    removed after creation, but you can imagine an implementation where
    they would need to be explicitely removed upon close)

This would not be performed if the signal is raised.
Since the user has the possibility of restoring default signal
handlers with SIG_DFL, I think we could stcik with the current
behavior.

Ah, ok, I agree with you, then.

@nadeemvawda nadeemvawda mannequin closed this as completed Oct 13, 2012
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants