Skip to content

Commit afdd513

Browse files
committed
Issue #25764: Preserve subprocess fork exception when preexec_fn used
Also fix handling of failure to release the import lock.
1 parent c7217d7 commit afdd513

3 files changed

Lines changed: 37 additions & 17 deletions

File tree

Lib/test/test_subprocess.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,6 +1416,22 @@ def raise_runtime_error():
14161416
if not enabled:
14171417
gc.disable()
14181418

1419+
def test_preexec_fork_failure(self):
1420+
# The internal code did not preserve the previous exception when
1421+
# re-enabling garbage collection
1422+
try:
1423+
from resource import getrlimit, setrlimit, RLIMIT_NPROC
1424+
except ImportError as err:
1425+
self.skipTest(err) # RLIMIT_NPROC is specific to Linux and BSD
1426+
limits = getrlimit(RLIMIT_NPROC)
1427+
[_, hard] = limits
1428+
setrlimit(RLIMIT_NPROC, (0, hard))
1429+
self.addCleanup(setrlimit, RLIMIT_NPROC, limits)
1430+
# Forking should raise EAGAIN, translated to BlockingIOError
1431+
with self.assertRaises(BlockingIOError):
1432+
subprocess.call([sys.executable, '-c', ''],
1433+
preexec_fn=lambda: None)
1434+
14191435
def test_args_string(self):
14201436
# args is a string
14211437
fd, fname = tempfile.mkstemp()

Misc/NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ Core and Builtins
115115
Library
116116
-------
117117

118+
- Issue #25764: In the subprocess module, preserve any exception caused by
119+
fork() failure when preexec_fn is used.
120+
118121
- Issue #6478: _strptime's regexp cache now is reset after changing timezone
119122
with time.tzset().
120123

Modules/_posixsubprocess.c

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,25 @@
4747
#define POSIX_CALL(call) do { if ((call) == -1) goto error; } while (0)
4848

4949

50-
/* Given the gc module call gc.enable() and return 0 on success. */
50+
/* If gc was disabled, call gc.enable(). Return 0 on success. */
5151
static int
52-
_enable_gc(PyObject *gc_module)
52+
_enable_gc(int need_to_reenable_gc, PyObject *gc_module)
5353
{
5454
PyObject *result;
5555
_Py_IDENTIFIER(enable);
56+
PyObject *exctype, *val, *tb;
5657

57-
result = _PyObject_CallMethodId(gc_module, &PyId_enable, NULL);
58-
if (result == NULL)
59-
return 1;
60-
Py_DECREF(result);
58+
if (need_to_reenable_gc) {
59+
PyErr_Fetch(&exctype, &val, &tb);
60+
result = _PyObject_CallMethodId(gc_module, &PyId_enable, NULL);
61+
if (exctype != NULL) {
62+
PyErr_Restore(exctype, val, tb);
63+
}
64+
if (result == NULL) {
65+
return 1;
66+
}
67+
Py_DECREF(result);
68+
}
6169
return 0;
6270
}
6371

@@ -691,6 +699,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
691699
_PyImport_ReleaseLock() < 0 && !PyErr_Occurred()) {
692700
PyErr_SetString(PyExc_RuntimeError,
693701
"not holding the import lock");
702+
pid = -1;
694703
}
695704
import_lock_held = 0;
696705

@@ -702,9 +711,8 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
702711
_Py_FreeCharPArray(exec_array);
703712

704713
/* Reenable gc in the parent process (or if fork failed). */
705-
if (need_to_reenable_gc && _enable_gc(gc_module)) {
706-
Py_XDECREF(gc_module);
707-
return NULL;
714+
if (_enable_gc(need_to_reenable_gc, gc_module)) {
715+
pid = -1;
708716
}
709717
Py_XDECREF(preexec_fn_args_tuple);
710718
Py_XDECREF(gc_module);
@@ -726,14 +734,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
726734
Py_XDECREF(converted_args);
727735
Py_XDECREF(fast_args);
728736
Py_XDECREF(preexec_fn_args_tuple);
729-
730-
/* Reenable gc if it was disabled. */
731-
if (need_to_reenable_gc) {
732-
PyObject *exctype, *val, *tb;
733-
PyErr_Fetch(&exctype, &val, &tb);
734-
_enable_gc(gc_module);
735-
PyErr_Restore(exctype, val, tb);
736-
}
737+
_enable_gc(need_to_reenable_gc, gc_module);
737738
Py_XDECREF(gc_module);
738739
return NULL;
739740
}

0 commit comments

Comments
 (0)