8000 gh-133548: fix handling of empty and 1-item tuples for `sys.exit` by picnixz · Pull Request #135789 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-133548: fix handling of empty and 1-item tuples for sys.exit #135789

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+72 −32
Open

gh-133548: fix handling of empty and 1-item tuples for sys.exit #135789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae37e95
fix handling of empty and 1-item tuples for `sys.exit`
picnixz Jun 21, 2025
6b65314
reduce a bit the diff
picnixz Jun 23, 2025
1719856
add What's New entry
picnixz Jun 24, 2025
a5c1cd8
improve comment.
picnixz Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Doc/whatsnew/3.15.rst
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ Other language changes
* Several error messages incorrectly using the term "argument" have been corrected.
(Contributed by Stan Ulbrych in :gh:`133382`.)

* :func:`sys.exit` and :exc:`SystemExit` now correctly handle empty and 1-item
tuples arguments. Previously, such tuples were unpacked by :func:`!sys.exit`
but not by :exc:`!SystemExit` and the process status code was incorrectly
set.
(Contributed by Bénédikt Tran in :gh:`133548`.)


New modules
Expand Down
84 changes: 53 additions & 31 deletions Lib/test/test_sys.py
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -205,62 +205,84 @@ def test_excepthook(self):
# Python/pythonrun.c::PyErr_PrintEx() is tricky.


def raise_system_exit(*args, **kwargs):
raise SystemExit(*args, **kwargs)


class SysModuleTest(unittest.TestCase):

def tearDown(self):
test.support.reap_children()

def test_exit(self):
# call with two arguments
# call with two arguments is only forbidden for sys.exit()
self.assertRaises(TypeError, sys.exit, 42, 42)
with self.subTest('sys.exit'):
self.do_test_exit(sys.exit)
with self.subTest('raise SystemExit'):
self.do_test_exit(raise_system_exit)

def do_test_exit(self, sys_exit_raiser):
# call without argument
with self.assertRaises(SystemExit) as cm:
sys.exit()
sys_exit_raiser()
self.assertIsNone(cm.exception.code)
with self.assertRaises(SystemExit) as cm:
sys_exit_raiser(None)
self.assertIsNone(cm.exception.code)

# call with integer argument
with self.assertRaises(SystemExit) as cm:
sys_exit_raiser(42)
self.assertEqual(cm.exception.code, 42)

# gh-133548: call with tuple argument with one entry
with self.assertRaises(SystemExit) as cm:
sys_exit_raiser((42,))
self.assertEqual(cm.exception.code, (42,))

# call with string argument
with self.assertRaises(SystemExit) as cm:
sys_exit_raiser("exit")
self.assertEqual(cm.exception.code, "exit")

# call with tuple argument with two entries
with self.assertRaises(SystemExit) as cm:
sys_exit_raiser((42, 42))
self.assertEqual(cm.exception.args, ((42, 42),))
self.assertEqual(cm.exception.code, (42, 42))

rc, out, err = assert_python_ok('-c', 'import sys; sys.exit()')
def test_exit_message(self):
with self.subTest('sys.exit'):
self.do_test_exit_message("sys.exit")
with self.subTest('raise SystemExit'):
self.do_test_exit_message("raise SystemExit")

def do_test_exit_message(self, call_statement):
def sys_exit_impl(value='', prolog=''):
return f'import sys\n{prolog}\n{call_statement}({value})'

rc, out, err = assert_python_ok('-c', sys_exit_impl())
self.assertEqual(rc, 0)
self.assertEqual(out, b'')
self.assertEqual(err, b'')

# gh-125842: Windows uses 32-bit unsigned integers for exit codes
# so a -1 exit code is sometimes interpreted as 0xffff_ffff.
rc, out, err = assert_python_failure('-c', 'import sys; sys.exit(0xffff_ffff)')
rc, out, err = assert_python_failure('-c', sys_exit_impl(0xffff_ffff))
self.assertIn(rc, (-1, 0xff, 0xffff_ffff))
self.assertEqual(out, b'')
self.assertEqual(err, b'')

# Overflow results in a -1 exit code, which may be converted to 0xff
# or 0xffff_ffff.
rc, out, err = assert_python_failure('-c', 'import sys; sys.exit(2**128)')
rc, out, err = assert_python_failure('-c', sys_exit_impl(2**128))
self.assertIn(rc, (-1, 0xff, 0xffff_ffff))
self.assertEqual(out, b'')
self.assertEqual(err, b'')

# call with integer argument
with self.assertRaises(SystemExit) as cm:
sys.exit(42)
self.assertEqual(cm.exception.code, 42)

# call with tuple argument with one entry
# entry will be unpacked
with self.assertRaises(SystemExit) as cm:
sys.exit((42,))
self.assertEqual(cm.exception.code, 42)

# call with string argument
with self.assertRaises(SystemExit) as cm:
sys.exit("exit")
self.assertEqual(cm.exception.code, "exit")

# call with tuple argument with two entries
with self.assertRaises(SystemExit) as cm:
sys.exit((17, 23))
self.assertEqual(cm.exception.code, (17, 23))

# test that the exit machinery handles SystemExits properly
rc, out, err = assert_python_failure('-c', 'raise SystemExit(47)')
# test that the exit machinery handles custom codes properly
rc, out, err = assert_python_failure('-c', sys_exit_impl(47))
self.assertEqual(rc, 47)
self.assertEqual(out, b'')
self.assertEqual(err, b'')
Expand All @@ -274,19 +296,19 @@ def check_exit_message(code, expected, **env_vars):
# test that stderr buffer is flushed before the exit message is written
# into stderr
check_exit_message(
r'import sys; sys.stderr.write("unflushed,"); sys.exit("message")',
sys_exit_impl("'message'", 'sys.stderr.write("unflushed,")'),
b"unflushed,message")

# test that the exit message is written with backslashreplace error
# handler to stderr
check_exit_message(
r'import sys; sys.exit("surrogates:\uDCFF")',
sys_exit_impl(r"'surrogates:\uDCFF'"),
b"surrogates:\\udcff")

# test that the unicode message is encoded to the stderr encoding
# instead of the default encoding (utf8)
check_exit_message(
r'import sys; sys.exit("h\xe9")',
sys_exit_impl(r"'h\xe9'"),
b"h\xe9", PYTHONIOENCODING='latin-1')

@support.requires_subprocess()
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Library/2025-06-21-14-15-57.gh-issue-133548.zuMuLb.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
:func:`sys.exit` and :exc:`SystemExit` now correctly handle empty and 1-item
tuples arguments. Previously, such tuples were unpacked by :func:`!sys.exit`
but not by :exc:`!SystemExit` and the process status code was incorrectly
set. Patch by Bénédikt Tran.
11 changes: 10 additions & 1 deletion Python/sysmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,16 @@ sys_exit_impl(PyObject *module, PyObject *status)
/*[clinic end generated code: output=13870986c1ab2ec0 input=b86ca9497baa94f2]*/
{
/* Raise SystemExit so callers may catch it or clean up. */
PyErr_SetObject(PyExc_SystemExit, status);
if (PyTuple_Check(status)) {
/* Make sure that tuples are not flattened during normalization
* due to the fast path for tuples in _PyErr_CreateException(). */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a "fast path" if removing it changes behaviour. We need to understand whether _PyErr_CreateException has a bug or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was designed as such for conveniency. It was added in 3a84097 but AFAICT, that was just refactoring of c0dc92a (1997) which already had this fast path.

PyObject *exc = PyObject_CallOneArg(PyExc_SystemExit, status);
PyErr_SetObject(PyExc_SystemExit, exc);
Py_DECREF(exc);
}
else {
PyErr_SetObject(PyExc_SystemExit, status);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a problem with how PyErr_SetObject creates the exception? Shouldn't it be fixed there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a bit unfortunate because that's how _PyErr_CreateException handles tuples:

static PyObject*
_PyErr_CreateException(PyObject *exception_type, PyObject *value)
{
    PyObject *exc;

    if (value == NULL || value == Py_None) {
        exc = _PyObject_CallNoArgs(exception_type);
    }
    else if (PyTuple_Check(value)) {
        exc = PyObject_Call(exception_type, value, NULL);
    }
    else {
        exc = PyObject_CallOneArg(exception_type, value);
    }

    if (exc != NULL && !PyExceptionInstance_Check(exc)) {
        PyErr_Format(PyExc_TypeError,
                     "calling %R should have returned an instance of "
                     "BaseException, not %s",
                     exception_type, Py_TYPE(exc)->tp_name);
        Py_CLEAR(exc);
    }

    return exc;
}

Unless you want me to change _PyErr_CreateException to prevent the fast paths, it could be annoying for downstream users.

return NULL;
}

Expand Down
Loading
0