This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: sys.exit documents argument as "integer" but actually requires "subtype of int"
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: Arfrever, belopolsky, gdr@garethrees.org, luc, mark.dickinson, python-dev, refi64, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2012-03-21 00:07 by gdr@garethrees.org, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
exit.patch gdr@garethrees.org, 2014-02-04 15:39 review
exit2.patch mark.dickinson, 2017-02-01 18:19 review
Messages (28)
msg156470 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2012-03-21 00:07
The documentation for sys.exit says, "The optional argument arg can be an integer giving the exit status (defaulting to zero), or another type of object".

However, the arguments that are treated as exit statuses are actually "subtypes of int".

So, a bool argument is fine:

    $ python2.7 -c "import sys; sys.exit(False)"; echo $?
    0

But a long argument is not:

    $ python2.7 -c "import sys; sys.exit(long(0))"; echo $?
    0
    1

The latter behaviour can be surprising since functions like os.spawnv may return the exit status of the executed process as a long on some platforms, so that if you try to pass on the exit code via

    code = os.spawnv(...)
    sys.exit(code)

you may get a mysterious surprise: code is 0 but exit code is 1.

It would be simple to change line 1112 of pythonrun.c from

    if (PyInt_Check(value))

to

    if (PyInt_Check(value) || PyLong_Check(value))

(This issue is not present in Python 3 because there is no longer a distinction between int and long.)
msg156481 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-03-21 08:27
> It would be simple to change line 1112 of pythonrun.c from
>
>    if (PyInt_Check(value))
>
> to
>
>    if (PyInt_Check(value) || PyLong_Check(value))

Wouldn't you also have to deal with possible errors from the PyInt_AsLong call?  E.g., after sys.exit(2**64), an OverflowException would be set.  I don't know if not dealing with that exception (perhaps with PyErr_Clear) before exit might cause issues, though it seems to work in practice (with the -1 value indicating an error being turned into an exit code of 255).
msg156484 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2012-03-21 09:55
> Wouldn't you also have to deal with possible errors from the PyInt_AsLong call?

Good point. But I note that Python 3 just does

    exitcode = (int)PyLong_AsLong(value);

so maybe it's not important to do error handling here.
msg156513 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-03-21 20:19
> so maybe it's not important to do error handling here.

Hmm, seems it's not.  And dealing with OverflowError is hardly likely to be a concern in practice anyway.

+1 for the suggested fix.
msg156726 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-03-25 00:28
Patch in first message; patch file needed. I don't know if sys.exit is actually tested. This is really 2.7 specific.
msg210242 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2014-02-04 15:39
Patch attached. I added a test case to Lib/test/test_sys.py.
msg241884 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-04-23 20:18
I am -1 on the patch.  (int)PyLong_AsLong(value) can silently convert non-zero error code to zero.  

I would leave 2.7 as is and limit allowable values to a range supported by 
the platform.

Note that ANSI C standard only specifies two values: EXIT_SUCCESS and EXIT_FAILURE, that may be passed to exit().
msg241885 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-23 20:22
There are _PyInt_AsInt() and _PyLong_AsInt().
msg241886 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-04-23 20:26
The key issue here is not to report success for nonzero values.

I consider the following a bug:

$ python3
Python 3.4.2 (default, Oct 30 2014, 08:51:12)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.54)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.exit(256)
$ echo $?
0
msg241887 - (view) Author: Ryan Gonzalez (refi64) * Date: 2015-04-23 20:29
Now we're getting away from the original issue. This wasn't created to handle edge cases for sys.exit; is was created to make it accept long values under Python 2.
msg241888 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-23 20:54
> The key issue here is not to report success for nonzero values.

This is different issue.
msg241892 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-04-23 21:35
+1 for the fix.

Alexander, create a new issue for the problem of converting non-zero values to zero.
msg241894 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-04-23 22:31
"errors should not pass silently"

The "fix" makes the problem worse.

Why would anyone want to pass a long integer to exit?

I bet the user who discovered this problem had something like 0L or 1L coming from a lazily written C extension.
msg241896 - (view) Author: Ryan Gonzalez (refi64) * Date: 2015-04-23 23:17
>
> "errors should not pass silently"
>
> The "fix" makes the problem worse.
>
> Why would anyone want to pass a long integer to exit?
>
> I bet the user who discovered this problem had something like 0L or 1L
> coming from a lazily written C extension.

Or the person is using a Python code generator that makes everything a long
for portability reasons! Hy does this.

int and long are supposed to be indistinguishable. Quote by Guido from the
time when I figured out the re module's group function doesn't take longs:

...but any place where an int is accepted but a long is not, is a bug. The
language has been on a long trip to making the two types interchangeable
and this seems one of the last remnants.

Notice the word *interchangeable*.

Also, I find that the current behavior is even more confusing.
msg241902 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-04-24 00:23
Passing anything other than one of the os.EX_* constants to sys.exit() is a bad idea.  In most cases you can get away with 0 and ±1.  Anything beyond 8 bit signed range is a gamble.  Passing a computed integer value is even more problematic.  With the current behavior, you at least get some diagnostic when a computed long finds its way to sys.axit().  With the proposed patch these errors will pass silently.
msg241904 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-04-24 00:50
Linux is not the only O/S that Python runs on.

There should be no difference between int and long.

Possible doc fix being tracked in issue24045.

Gareth, please ignore my comments about adding guards on the return value -- it is up to the O/S to use or adjust whatever Python returns.
msg241945 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2015-04-24 15:15
I previously wrote:
------------------
> Gareth, please ignore my comments about adding guards on the return value -- it is up
> to the O/S to use or adjust whatever Python returns.

Let me clarify that a bit: we need to protect against overflow from <long> to <int>; protecting against overflow from <int> to <O/S returncode size> is a separate issue.
msg241947 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-04-24 15:30
> Alexander, create a new issue for the problem of converting non-zero values to zero.

See #24052.
msg268225 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2016-06-11 16:26
Let's not allow the perfect to be the enemy of the good here.

The issue I reported is a very specific one: in Python 2.7, if you pass a long to sys.exit, then the value of the long is not used as the exit code. This is bad because functions like os.spawnv that return exit codes (that you might reasonably want to pass on to sys.exit) can return them as long.

My patch only proposes to address this one issue. In order to keep the impact as small as possible, I do not propose to make any other changes, or address any other problems.

But in the comments here people have brought up THREE other issues:

1. Alexander Belopolsky expresses the concern that "(int)PyLong_AsLong(value) can silently convert non-zero error code to zero."

This is not a problem introduced by my patch -- the current code is:

    exitcode = (int)PyInt_AsLong(value)

which has exactly the same problem (because PyIntObject stores its value as a long). So this concern (even if valid) is not a reason to reject my patch.

2. Ethan Furman wrote: "we need to protect against overflow from <long> to <int>"

But again, this is not a problem introduced by my patch. The current code says:

    exitcode = (int)PyInt_AsLong(value);

and my patch does not change this line. The possibility of this overflow is not a reason to reject my patch.

3. Alexander says, "Passing anything other than one of the os.EX_* constants to sys.exit() is a bad idea"

First, this is not a problem introduced by my patch. The existing code in Python 2.7 allows you to specify other exit codes. So this problem (if it is a problem) is not a reason to reject my patch.

Second, this claim is surely not right -- when a subprocess fails it often makes sense to pass on the exit code of the subprocess, whatever that is. This is exactly the use case that I mentioned in my original report (that is, passing on the exit code from os.spawnv to sys.exit).
msg286619 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2017-02-01 12:20
Is there any chance of making progress on this issue? Is there anything wrong with my patch? Did I omit any relevant point in my message of 2016-06-11 16:26? It would be nice if this were not left in limbo for another four years.
msg286629 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-02-01 13:41
Summary of overt and implied votes:
Core Devs:
+1 Mark Dickenson
+1 Ethan Furman
?? Serhiy Storchaka
-1 Alexander Belopolsky (at first, but unclear after)

Other:
+1 Ryan Gonzales, quoting Guido about equivalence of ints and longs.

The sys.exit docstring says "If the status is an integer, it will be used as the system exit status."  In 2.7, 'integer' clearly includes longs.

Does anyone know the Windows equivalent of 'echo $?'?

Gareth, I think the problem is that this is a minor 2.7-only, non-security issue and therefore a low priority for most Core Devs.
msg286630 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2017-02-01 13:52
In Windows, under cmd.exe, you can use %errorlevel%
msg286632 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-01 14:04
Count me as +1. The patch LGTM.

See also issue28998.
msg286672 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-01 18:19
The test portion of the existing patch doesn't apply cleanly to 2.7 tip. Here's an updated patch that does, with slightly expanded tests and a Misc/NEWS entry.

There was discussion above about two overflow cases: C long to C int, and C int to byte, but with this patch there's now a 3rd overflow case to consider, namely Python long to C long.  In this case PyInt_AsLong sets an exception and returns -1, and since we don't check for exceptions that -1 then gets used as the exit code. I'm not sure whether that's the right thing to do or not, but it does match what Python 3 currently does, and that's good enough for me.
msg286673 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2017-02-01 18:34
Thanks for the revised patch, Mark. The new tests look good.
msg286814 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-02-02 19:32
New changeset 14682d00b09a by Mark Dickinson in branch '2.7':
Issue #14376: sys.exit now accepts longs as well as ints. Thanks Gareth Rees.
https://hg.python.org/cpython/rev/14682d00b09a
msg286816 - (view) Author: Gareth Rees (gdr@garethrees.org) * (Python triager) Date: 2017-02-02 19:41
Thank you, Mark (and everyone else who helped).
msg286824 - (view) Author: Roundup Robot (python-dev) (Python triager) Date:
New changeset 12b49507177368204085bd6e2b464f45e3b569e2 by Mark Dickinson in branch '2.7':
Issue #14376: sys.exit now accepts longs as well as ints. Thanks Gareth Rees.
https://github.com/python/cpython/commit/12b49507177368204085bd6e2b464f45e3b569e2
History
Date User Action Args
2022-04-11 14:57:28adminsetgithub: 58584
2017-02-02 20:02:41python-devsetmessages: + msg286824
2017-02-02 19:41:41gdr@garethrees.orgsetmessages: + msg286816
2017-02-02 19:33:23mark.dickinsonsetstatus: open -> closed
assignee: mark.dickinson
resolution: fixed
stage: test needed -> resolved
2017-02-02 19:32:12python-devsetnosy: + python-dev
messages: + msg286814
2017-02-01 18:34:03gdr@garethrees.orgsetmessages: + msg286673
2017-02-01 18:19:47mark.dickinsonsetfiles: + exit2.patch

messages: + msg286672
2017-02-01 14:04:43serhiy.storchakasetmessages: + msg286632
2017-02-01 13:52:56gdr@garethrees.orgsetmessages: + msg286630
2017-02-01 13:41:48terry.reedysetmessages: + msg286629
2017-02-01 12:20:11gdr@garethrees.orgsetmessages: + msg286619
2016-08-28 01:53:52lucsetnosy: + luc
2016-06-11 16:26:16gdr@garethrees.orgsetmessages: + msg268225
2015-07-21 07:16:47ethan.furmansetnosy: - ethan.furman
2015-05-03 05:55:23Arfreversetnosy: + Arfrever
2015-04-24 15:30:56belopolskysetmessages: + msg241947
2015-04-24 15:15:11ethan.furmansetmessages: + msg241945
2015-04-24 00:50:13ethan.furmansetmessages: + msg241904
2015-04-24 00:23:33belopolskysetmessages: + msg241902
2015-04-23 23:17:52refi64setmessages: + msg241896
2015-04-23 22:31:52belopolskysetmessages: + msg241894
2015-04-23 21:35:29ethan.furmansetmessages: + msg241892
2015-04-23 21:31:55ethan.furmansetnosy: + ethan.furman
2015-04-23 20:54:43serhiy.storchakasetmessages: + msg241888
2015-04-23 20:29:45refi64setnosy: + refi64
messages: + msg241887
2015-04-23 20:26:01belopolskysetmessages: + msg241886
2015-04-23 20:22:11serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg241885
2015-04-23 20:18:04belopolskysetnosy: + belopolsky
messages: + msg241884
2014-02-04 15:39:26gdr@garethrees.orgsetfiles: + exit.patch

messages: + msg210242
2012-03-25 00:28:48terry.reedysetnosy: + terry.reedy
messages: + msg156726

keywords: + patch
stage: test needed
2012-03-22 05:24:03eric.araujosetversions: - Python 2.6
2012-03-21 20:19:11mark.dickinsonsetmessages: + msg156513
2012-03-21 09:55:10gdr@garethrees.orgsetmessages: + msg156484
2012-03-21 08:27:15mark.dickinsonsetnosy: + mark.dickinson
messages: + msg156481
2012-03-21 00:07:59gdr@garethrees.orgcreate