gh-133548: fix handling of empty and 1-item tuples for sys.exit
#135789
Conversation
|
🤖 New build scheduled with the buildbot fleet by @picnixz for commit ae37e95 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135789%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
bcc2970 to
6b65314
Compare
| } | ||
| else { | ||
| PyErr_SetObject(PyExc_SystemExit, status); | ||
| } |
There was a problem hiding this comment.
Isn't this a problem with how PyErr_SetObject creates the exception? Shouldn't it be fixed there?
There was a problem hiding this comment.
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.
| @@ -917,7 +917,8 @@ sys_exit_impl(PyObject *module, PyObject *status) | |||
| { | |||
| /* Raise SystemExit so callers may catch it or clean up. */ | |||
| if (PyTuple_Check(status)) { | |||
| /* Make sure that tuples are not flattened during normalization. */ | |||
| /* Make sure that tuples are not flattened during normalization | |||
| * due to the fast path for tuples in _PyErr_CreateException(). */ | |||
There was a problem hiding this comment.
It's not a "fast path" if removing it changes behaviour. We need to understand whether _PyErr_CreateException has a bug or not.
cc @markshannon @iritkatriel
sys.exitunpacks its argument if it is a 0- or 1-element tuple #133548