[Python-checkins] bpo-28254: _posixsubprocess uses PyGC_Enable/PyGC_Disable (GH-25693)
vstinner
webhook-mailer at python.org
Wed Apr 28 13:09:34 EDT 2021
https://github.com/python/cpython/commit/103d5e420dd90489933ad9da8bb1d6008773384d
commit: 103d5e420dd90489933ad9da8bb1d6008773384d
branch: master
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2021-04-28T19:09:29+02:00
summary:
bpo-28254: _posixsubprocess uses PyGC_Enable/PyGC_Disable (GH-25693)
files:
M Lib/test/test_subprocess.py
M Modules/_posixsubprocess.c
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 7c79365f41191..27ccd3e5cb3a8 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -2147,8 +2147,6 @@ def raise_it():
def test_preexec_gc_module_failure(self):
# This tests the code that disables garbage collection if the child
# process will execute any Python.
- def raise_runtime_error():
- raise RuntimeError("this shouldn't escape")
enabled = gc.isenabled()
orig_gc_disable = gc.disable
orig_gc_isenabled = gc.isenabled
@@ -2165,16 +2163,6 @@ def raise_runtime_error():
subprocess.call([sys.executable, '-c', ''],
preexec_fn=lambda: None)
self.assertTrue(gc.isenabled(), "Popen left gc disabled.")
-
- gc.disable = raise_runtime_error
- self.assertRaises(RuntimeError, subprocess.Popen,
- [sys.executable, '-c', ''],
- preexec_fn=lambda: None)
-
- del gc.isenabled # force an AttributeError
- self.assertRaises(AttributeError, subprocess.Popen,
- [sys.executable, '-c', ''],
- preexec_fn=lambda: None)
finally:
gc.disable = orig_gc_disable
gc.isenabled = orig_gc_isenabled
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 3b0651620e551..a58159a277bea 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -69,47 +69,8 @@
#define POSIX_CALL(call) do { if ((call) == -1) goto error; } while (0)
-typedef struct {
- PyObject* disable;
- PyObject* enable;
- PyObject* isenabled;
-} _posixsubprocessstate;
-
static struct PyModuleDef _posixsubprocessmodule;
-static inline _posixsubprocessstate*
-get_posixsubprocess_state(PyObject *module)
-{
- void *state = PyModule_GetState(module);
- assert(state != NULL);
- return (_posixsubprocessstate *)state;
-}
-
-/* If gc was disabled, call gc.enable(). Ignore errors. */
-static void
-_enable_gc(int need_to_reenable_gc, PyObject *gc_module, _posixsubprocessstate *state)
-{
- PyObject *result;
- PyObject *exctype, *val, *tb;
-
- if (need_to_reenable_gc) {
- PyErr_Fetch(&exctype, &val, &tb);
- result = PyObject_CallMethodNoArgs(
- gc_module, state->enable);
- if (result == NULL) {
- /* We might have created a child process at this point, we
- * we have no good way to handle a failure to reenable GC
- * and return information about the child process. */
- PyErr_Print();
- }
- Py_XDECREF(result);
- if (exctype != NULL) {
- PyErr_Restore(exctype, val, tb);
- }
- }
-}
-
-
/* Convert ASCII to a positive int, no libc call. no overflow. -1 on error. */
static int
_pos_int_from_ascii(const char *name)
@@ -780,7 +741,6 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
Py_ssize_t arg_num, num_groups = 0;
int need_after_fork = 0;
int saved_errno = 0;
- _posixsubprocessstate *state = get_posixsubprocess_state(module);
if (!PyArg_ParseTuple(
args, "OOpO!OOiiiiiiiiiiOOOiO:fork_exec",
@@ -820,30 +780,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
/* We need to call gc.disable() when we'll be calling preexec_fn */
if (preexec_fn != Py_None) {
- PyObject *result;
-
- gc_module = PyImport_ImportModule("gc");
- if (gc_module == NULL)
- return NULL;
- result = PyObject_CallMethodNoArgs(
- gc_module, state->isenabled);
- if (result == NULL) {
- Py_DECREF(gc_module);
- return NULL;
- }
- need_to_reenable_gc = PyObject_IsTrue(result);
- Py_DECREF(result);
- if (need_to_reenable_gc == -1) {
- Py_DECREF(gc_module);
- return NULL;
- }
- result = PyObject_CallMethodNoArgs(
- gc_module, state->disable);
- if (result == NULL) {
- Py_DECREF(gc_module);
- return NULL;
- }
- Py_DECREF(result);
+ need_to_reenable_gc = PyGC_Disable();
}
exec_array = _PySequence_BytesToCharpArray(executable_list);
@@ -1068,7 +1005,9 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
if (exec_array)
_Py_FreeCharPArray(exec_array);
- _enable_gc(need_to_reenable_gc, gc_module, state);
+ if (need_to_reenable_gc) {
+ PyGC_Enable();
+ }
Py_XDECREF(gc_module);
return pid == -1 ? NULL : PyLong_FromPid(pid);
@@ -1108,67 +1047,22 @@ Raises: Only on an error in the parent process.\n\
PyDoc_STRVAR(module_doc,
"A POSIX helper for the subprocess module.");
-static int
-_posixsubprocess_exec(PyObject *module)
-{
- _posixsubprocessstate *state = get_posixsubprocess_state(module);
-
- state->disable = PyUnicode_InternFromString("disable");
- if (state->disable == NULL) {
- return -1;
- }
-
- state->enable = PyUnicode_InternFromString("enable");
- if (state->enable == NULL) {
- return -1;
- }
-
- state->isenabled = PyUnicode_InternFromString("isenabled");
- if (state->isenabled == NULL) {
- return -1;
- }
-
- return 0;
-}
-
static PyMethodDef module_methods[] = {
{"fork_exec", subprocess_fork_exec, METH_VARARGS, subprocess_fork_exec_doc},
{NULL, NULL} /* sentinel */
};
static PyModuleDef_Slot _posixsubprocess_slots[] = {
- {Py_mod_exec, _posixsubprocess_exec},
{0, NULL}
};
-static int _posixsubprocess_traverse(PyObject *m, visitproc visit, void *arg) {
- Py_VISIT(get_posixsubprocess_state(m)->disable);
- Py_VISIT(get_posixsubprocess_state(m)->enable);
- Py_VISIT(get_posixsubprocess_state(m)->isenabled);
- return 0;
-}
-
-static int _posixsubprocess_clear(PyObject *m) {
- Py_CLEAR(get_posixsubprocess_state(m)->disable);
- Py_CLEAR(get_posixsubprocess_state(m)->enable);
- Py_CLEAR(get_posixsubprocess_state(m)->isenabled);
- return 0;
-}
-
-static void _posixsubprocess_free(void *m) {
- _posixsubprocess_clear((PyObject *)m);
-}
-
static struct PyModuleDef _posixsubprocessmodule = {
PyModuleDef_HEAD_INIT,
.m_name = "_posixsubprocess",
.m_doc = module_doc,
- .m_size = sizeof(_posixsubprocessstate),
+ .m_size = 0,
.m_methods = module_methods,
.m_slots = _posixsubprocess_slots,
- .m_traverse = _posixsubprocess_traverse,
- .m_clear = _posixsubprocess_clear,
- .m_free = _posixsubprocess_free,
};
PyMODINIT_FUNC
More information about the Python-checkins
mailing list