8000 bpo-28254: _posixsubprocess uses PyGC_Enable/PyGC_Disable (GH-25693) · python/cpython@103d5e4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 103d5e4

Browse files
vstinnervstinner
authored
bpo-28254: _posixsubprocess uses PyGC_Enable/PyGC_Disable (GH-25693)
1 parent 3b52c8d commit 103d5e4

File tree

2 files changed

+5
-123
lines changed

2 files changed

+5
-123
lines changed

‎Lib/test/test_subprocess.py

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,8 +2147,6 @@ def raise_it():
21472147
def test_preexec_gc_module_failure(self):
21482148
# This tests the code that disables garbage collection if the child
21492149
# process will execute any Python.
2150-
def raise_runtime_error():
2151-
raise RuntimeError("this shouldn't escape")
21522150
enabled = gc.isenabled()
21532151
orig_gc_disable = gc.disable
21542152
orig_gc_isenabled = gc.isenabled
@@ -2165,16 +2163,6 @@ def raise_runtime_error():
21652163
subprocess.call([sys.executable, '-c', ''],
21662164
preexec_fn=lambda: None)
21672165
self.assertTrue(gc.isenabled(), "Popen left gc disabled.")
2168-
2169-
gc.disable = raise_runtime_error
2170-
self.assertRaises(RuntimeError, subprocess.Popen,
2171-
[sys.executable, '-c', ''],
2172-
preexec_fn=lambda: None)
2173-
2174-
del gc.isenabled # force an AttributeError
2175-
self.assertRaises(AttributeError, subprocess.Popen,
2176-
[sys.executable, '-c', ''],
2177-
preexec_fn=lambda: None)
21782166
finally:
21792167
gc.disable = orig_gc_disable
21802168
gc.isenabled = orig_gc_isenabled

‎Modules/_posixsubprocess.c

Lines changed: 5 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -69,47 +69,8 @@
6969

7070
#define POSIX_CALL(call) do { if ((call) == -1) goto error; } while (0)
7171

72-
typedef struct {
73-
PyObject* disable;
74-
PyObject* enable;
75-
PyObject* isenabled;
76-
} _posixsubprocessstate;
77-
7872
static struct PyModuleDef _posixsubprocessmodule;
7973

80-
static inline _posixsubprocessstate*
81-
get_posixsubprocess_state(PyObject *module)
82-
{
83-
void *state = PyModule_GetState(module);
84-
assert(state != NULL);
85-
return (_posixsubprocessstate *)state;
86-
}
87-
88-
/* If gc was disabled, call gc.enable(). Ignore errors. */
89-
static void
90-
_enable_gc(int need_to_reenable_gc, PyObject *gc_module, _posixsubprocessstate *state)
91-
{
92-
PyObject *result;
93-
PyObject *exctype, *val, *tb;
94-
95-
if (need_to_reenable_gc) {
96-
PyErr_Fetch(&exctype, &val, &tb);
97-
result = PyObject_CallMethodNoArgs(
98-
gc_module, state->enable);
99-
if (result == NULL) {
100-
/* We might have created a child process at this point, we
101-
* we have no good way to handle a failure to reenable GC
102-
* and return information about the child process. */
103-
PyErr_Print();
104-
}
105-
Py_XDECREF(result);
106-
if (exctype != NULL) {
107-
PyErr_Restore(exctype, val, tb);
108-
}
109-
}
110-
}
111-
112-
11374
/* Convert ASCII to a positive int, no libc call. no overflow. -1 on error. */
11475
static int
11576
_pos_int_from_ascii(const char *name)
@@ -780,7 +741,6 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
780741
Py_ssize_t arg_num, num_groups = 0;
781742
int need_after_fork = 0;
782743
int saved_errno = 0;
783-
_posixsubprocessstate *state = get_posixsubprocess_state(module);
784744

785745
if (!PyArg_ParseTuple(
786746
args, "OOpO!OOiiiiiiiiiiOOOiO:fork_exec",
@@ -820,30 +780,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
820780

821781
/* We need to call gc.disable() when we'll be calling preexec_fn */
822782
if (preexec_fn != Py_None) {
823-
PyObject *result;
824-
825-
gc_module = PyImport_ImportModule("gc");
826-
if (gc_module == NULL)
827-
return NULL;
828-
result = PyObject_CallMethodNoArgs(
829-
gc_module, state->isenabled);
830-
if (result == NULL) {
831-
Py_DECREF(gc_module);
832-
return NULL;
833-
}
834-
need_to_reenable_gc = PyObject_IsTrue(result);
835-
Py_DECREF(result);
836-
if (need_to_reenable_gc == -1) {
837-
Py_DECREF(gc_module);
838-
return NULL;
839-
}
840-
result = PyObject_CallMethodNoArgs(
841-
gc_module, state->disable);
842-
if (result == NULL) {
843-
Py_DECREF(gc_module);
844-
return NULL;
845-
}
846-
Py_DECREF(result);
783+
need_to_reenable_gc = PyGC_Disable();
847784
}
848785

849786
exec_array = _PySequence_BytesToCharpArray(executable_list);
@@ -1068,7 +1005,9 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
10681005
if (exec_array)
10691006
_Py_FreeCharPArray(exec_array);
10701007

1071-
_enable_gc(need_to_reenable_gc, gc_module, state);
1008+
if (need_to_reenable_gc) {
1009+
PyGC_Enable();
1010+
}
10721011
Py_XDECREF(gc_module);
10731012

10741013
return pid == -1 ? NULL : PyLong_FromPid(pid);
@@ -1108,67 +1047,22 @@ Raises: Only on an error in the parent process.\n\
11081047
PyDoc_STRVAR(module_doc,
11091048
"A POSIX helper for the subprocess module.");
11101049

1111-
static int
1112-
_posixsubprocess_exec(PyObject *module)
1113-
{
1114-
_posixsubprocessstate *state = get_posixsubprocess_state(module);
1115-
1116-
state->disable = PyUnicode_InternFromString("disable");
1117-
if (state->disable == NULL) {
1118-
return -1;
1119-
}
1120-
1121-
state->enable = PyUnicode_InternFromString("enable");
1122-
if (state->enable == NULL) {
1123-
return -1;
1124-
}
1125-
1126-
state->isenabled = PyUnicode_InternFromString("isenabled");
1127-
if (state->isenabled == NULL) {
1128-
return -1;
1129-
}
1130-
1131-
return 0;
1132-
}
1133-
11341050
static PyMethodDef module_methods[] = {
11351051
{"fork_exec", subprocess_fork_exec, METH_VARARGS, subprocess_fork_exec_doc},
11361052
{NULL, NULL} /* sentinel */
11371053
};
11381054

11391055
static PyModuleDef_Slot _posixsubprocess_slots[] = {
1140-
{Py_mod_exec, _posixsubprocess_exec},
11411056
{0, NULL}
11421057
};
11431058

1144-
static int _posixsubprocess_traverse(PyObject *m, visitproc visit, void *arg) {
1145-
Py_VISIT(get_posixsubprocess_state(m)->disable);
1146-
Py_VISIT(get_posixsubprocess_state(m)->enable);
1147-
Py_VISIT(get_posixsubprocess_state(m)->isenabled);
1148-
return 0;
1149-
}
1150-
1151-
static int _posixsubprocess_clear(PyObject *m) {
1152-
Py_CLEAR(get_posixsubprocess_state(m)->disable);
1153-
Py_CLEAR(get_posixsubprocess_state(m)->enable);
1154-
Py_CLEAR(get_posixsubprocess_state(m)->isenabled);
1155-
return 0;
1156-
}
1157-
1158-
static void _posixsubprocess_free(void *m) {
1159-
_posixsubprocess_clear((PyObject *)m);
1160-
}
1161-
11621059
static struct PyModuleDef _posixsubprocessmodule = {
11631060
PyModuleDef_HEAD_INIT,
11641061
.m_name = "_posixsubprocess",
11651062
.m_doc = module_doc,
1166-
.m_size = sizeof(_posixsubprocessstate),
1063+
.m_size = 0,
11671064
.m_methods = module_methods,
11681065
.m_slots = _posixsubprocess_slots,
1169-
.m_traverse = _posixsubprocess_traverse,
1170-
.m_clear = _posixsubprocess_clear,
1171-
.m_free = _posixsubprocess_free,
11721066
};
11731067

11741068
PyMODINIT_FUNC

0 commit comments

Comments
 (0)
0