Issue #22290: Fix error handling in the _posixsubprocess module.
* Don't call the garbage collector with an exception set: it causes an assertion to fail in debug mode. * Enhance also error handling if allocating an array for the executable list failed. * Add an unit test for 4 different errors in the _posixsubprocess module.
This commit is contained in:
parent
340c749a3a
commit
8f437aac06
|
@ -2216,6 +2216,39 @@ class POSIXProcessTestCase(BaseTestCase):
|
|||
|
||||
self.assertNotIn(fd, remaining_fds)
|
||||
|
||||
@support.cpython_only
|
||||
def test_fork_exec(self):
|
||||
# Issue #22290: fork_exec() must not crash on memory allocation failure
|
||||
# or other errors
|
||||
import _posixsubprocess
|
||||
gc_enabled = gc.isenabled()
|
||||
try:
|
||||
# Use a preexec function and enable the garbage collector
|
||||
# to force fork_exec() to re-enable the garbage collector
|
||||
# on error.
|
||||
func = lambda: None
|
||||
gc.enable()
|
||||
|
||||
executable_list = "exec" # error: must be a sequence
|
||||
|
||||
for args, exe_list, cwd, env_list in (
|
||||
(123, [b"exe"], None, [b"env"]),
|
||||
([b"arg"], 123, None, [b"env"]),
|
||||
([b"arg"], [b"exe"], 123, [b"env"]),
|
||||
([b"arg"], [b"exe"], None, 123),
|
||||
):
|
||||
with self.assertRaises(TypeError):
|
||||
_posixsubprocess.fork_exec(
|
||||
args, exe_list,
|
||||
True, [], cwd, env_list,
|
||||
-1, -1, -1, -1,
|
||||
1, 2, 3, 4,
|
||||
True, True, func)
|
||||
finally:
|
||||
if not gc_enabled:
|
||||
gc.disable()
|
||||
|
||||
|
||||
|
||||
@unittest.skipUnless(mswindows, "Windows specific tests")
|
||||
class Win32ProcessTestCase(BaseTestCase):
|
||||
|
|
|
@ -538,6 +538,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
|
|||
int need_to_reenable_gc = 0;
|
||||
char *const *exec_array, *const *argv = NULL, *const *envp = NULL;
|
||||
Py_ssize_t arg_num;
|
||||
int import_lock_held = 0;
|
||||
|
||||
if (!PyArg_ParseTuple(
|
||||
args, "OOpOOOiiiiiiiiiiO:fork_exec",
|
||||
|
@ -590,10 +591,8 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
|
|||
}
|
||||
|
||||
exec_array = _PySequence_BytesToCharpArray(executable_list);
|
||||
if (!exec_array) {
|
||||
Py_XDECREF(gc_module);
|
||||
return NULL;
|
||||
}
|
||||
if (!exec_array)
|
||||
goto cleanup;
|
||||
|
||||
/* Convert args and env into appropriate arguments for exec() */
|
||||
/* These conversions are done in the parent process to avoid allocating
|
||||
|
@ -635,6 +634,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
|
|||
if (!preexec_fn_args_tuple)
|
||||
goto cleanup;
|
||||
_PyImport_AcquireLock();
|
||||
import_lock_held = 1;
|
||||
}
|
||||
|
||||
if (cwd_obj != Py_None) {
|
||||
|
@ -682,6 +682,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
|
|||
PyErr_SetString(PyExc_RuntimeError,
|
||||
"not holding the import lock");
|
||||
}
|
||||
import_lock_held = 0;
|
||||
|
||||
/* Parent process */
|
||||
if (envp)
|
||||
|
@ -704,18 +705,25 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
|
|||
return PyLong_FromPid(pid);
|
||||
|
||||
cleanup:
|
||||
if (import_lock_held)
|
||||
_PyImport_ReleaseLock();
|
||||
if (envp)
|
||||
_Py_FreeCharPArray(envp);
|
||||
if (argv)
|
||||
_Py_FreeCharPArray(argv);
|
||||
if (exec_array)
|
||||
_Py_FreeCharPArray(exec_array);
|
||||
Py_XDECREF(converted_args);
|
||||
Py_XDECREF(fast_args);
|
||||
Py_XDECREF(preexec_fn_args_tuple);
|
||||
|
||||
/* Reenable gc if it was disabled. */
|
||||
if (need_to_reenable_gc)
|
||||
if (need_to_reenable_gc) {
|
||||
PyObject *exctype, *val, *tb;
|
||||
PyErr_Fetch(&exctype, &val, &tb);
|
||||
_enable_gc(gc_module);
|
||||
PyErr_Restore(exctype, val, tb);
|
||||
}
|
||||
Py_XDECREF(gc_module);
|
||||
return NULL;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue