diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 4307b61ca36aaf..1df5f413e06c99 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -229,6 +229,8 @@ struct _is { int finalizing; struct _ceval_state ceval; + // bpo-46070: Even if each PyInterpreterState has a GC state, + // _PyGC_GetState() only uses the state of the main interpreter. struct _gc_runtime_state gc; // sys.modules dictionary diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 90d98134b8919b..100db494f2fab9 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -57,6 +57,16 @@ _PyObject_InitVar(PyVarObject *op, PyTypeObject *typeobj, Py_ssize_t size) } +static inline struct _gc_runtime_state* +_PyGC_GetState(void) +{ + // bpo-46070: Even if each PyInterpreterState has a GC state, + // _PyGC_GetState() only uses the state of the main interpreter. + PyInterpreterState *interp = _PyRuntime.interpreters.main; + return &interp->gc; +} + + /* Tell the GC to track this object. * * The object must not be tracked by the GC. @@ -87,8 +97,8 @@ static inline void _PyObject_GC_TRACK( "object is in generation which is garbage collected", filename, lineno, __func__); - PyInterpreterState *interp = _PyInterpreterState_GET(); - PyGC_Head *generation0 = interp->gc.generation0; + struct _gc_runtime_state *state = _PyGC_GetState(); + PyGC_Head *generation0 = state->generation0; PyGC_Head *last = (PyGC_Head*)(generation0->_gc_prev); _PyGCHead_SET_NEXT(last, gc); _PyGCHead_SET_PREV(gc, last); diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index 524be9d4cbb940..e230b5f4f3afe6 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -75,7 +75,7 @@ extern PyStatus _Py_HashRandomization_Init(const PyConfig *); extern PyStatus _PyTypes_Init(void); extern PyStatus _PyTypes_InitSlotDefs(void); extern PyStatus _PyImportZip_Init(PyThreadState *tstate); -extern PyStatus _PyGC_Init(PyInterpreterState *interp); +extern PyStatus _PyGC_Init(void); extern PyStatus _PyAtExit_Init(PyInterpreterState *interp); @@ -96,7 +96,7 @@ extern void _PySignal_Fini(void); extern void _PyExc_Fini(PyInterpreterState *interp); extern void _PyImport_Fini(void); extern void _PyImport_Fini2(void); -extern void _PyGC_Fini(PyInterpreterState *interp); +extern void _PyGC_Fini(void); extern void _PyType_Fini(PyInterpreterState *interp); extern void _Py_HashRandomization_Fini(void); extern void _PyUnicode_Fini(PyInterpreterState *interp); @@ -113,7 +113,7 @@ extern PyStatus _PyGILState_Init(_PyRuntimeState *runtime); extern PyStatus _PyGILState_SetTstate(PyThreadState *tstate); extern void _PyGILState_Fini(PyInterpreterState *interp); -PyAPI_FUNC(void) _PyGC_DumpShutdownStats(PyInterpreterState *interp); +PyAPI_FUNC(void) _PyGC_DumpShutdownStats(void); PyAPI_FUNC(PyStatus) _Py_PreInitializeFromPyArgv( const PyPreConfig *src_config, diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-01-12-23-13-39.bpo-46070.cQp0eC.rst b/Misc/NEWS.d/next/Core and Builtins/2022-01-12-23-13-39.bpo-46070.cQp0eC.rst new file mode 100644 index 00000000000000..c55578ff87f5d8 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-01-12-23-13-39.bpo-46070.cQp0eC.rst @@ -0,0 +1,3 @@ +Fix a random crash involving subinterpreters on Windows. Revert the change +which made the gc module state per interpreter: the gc module state is +shared again by all interpreters. Patch by Victor Stinner. diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index e5e5aa3287b0d6..38c71277724d35 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -131,8 +131,7 @@ gc_decref(PyGC_Head *g) static GCState * get_gc_state(void) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - return &interp->gc; + return _PyGC_GetState(); } @@ -161,9 +160,9 @@ _PyGC_InitState(GCState *gcstate) PyStatus -_PyGC_Init(PyInterpreterState *interp) +_PyGC_Init(void) { - GCState *gcstate = &interp->gc; + GCState *gcstate = get_gc_state(); gcstate->garbage = PyList_New(0); if (gcstate->garbage == NULL) { @@ -1193,7 +1192,7 @@ gc_collect_main(PyThreadState *tstate, int generation, PyGC_Head finalizers; /* objects with, & reachable from, __del__ */ PyGC_Head *gc; _PyTime_t t1 = 0; /* initialize to prevent a compiler warning */ - GCState *gcstate = &tstate->interp->gc; + GCState *gcstate = get_gc_state(); // gc_collect_main() must not be called before _PyGC_Init // or after _PyGC_Fini() @@ -1367,7 +1366,7 @@ invoke_gc_callback(PyThreadState *tstate, const char *phase, assert(!_PyErr_Occurred(tstate)); /* we may get called very early */ - GCState *gcstate = &tstate->interp->gc; + GCState *gcstate = get_gc_state(); if (gcstate->callbacks == NULL) { return; } @@ -1419,7 +1418,7 @@ gc_collect_with_callback(PyThreadState *tstate, int generation) static Py_ssize_t gc_collect_generations(PyThreadState *tstate) { - GCState *gcstate = &tstate->interp->gc; + GCState *gcstate = get_gc_state(); /* Find the oldest generation (highest numbered) where the count * exceeds the threshold. Objects in the that generation and * generations younger than it will be collected. */ @@ -1540,7 +1539,7 @@ gc_collect_impl(PyObject *module, int generation) return -1; } - GCState *gcstate = &tstate->interp->gc; + GCState *gcstate = get_gc_state(); Py_ssize_t n; if (gcstate->collecting) { /* already collecting, don't do anything */ @@ -1761,10 +1760,9 @@ static PyObject * gc_get_objects_impl(PyObject *module, Py_ssize_t generation) /*[clinic end generated code: output=48b35fea4ba6cb0e input=ef7da9df9806754c]*/ { - PyThreadState *tstate = _PyThreadState_GET(); int i; PyObject* result; - GCState *gcstate = &tstate->interp->gc; + GCState *gcstate = get_gc_state(); if (PySys_Audit("gc.get_objects", "n", generation) < 0) { return NULL; @@ -1778,16 +1776,16 @@ gc_get_objects_impl(PyObject *module, Py_ssize_t generation) /* If generation is passed, we extract only that generation */ if (generation != -1) { if (generation >= NUM_GENERATIONS) { - _PyErr_Format(tstate, PyExc_ValueError, - "generation parameter must be less than the number of " - "available generations (%i)", - NUM_GENERATIONS); + PyErr_Format(PyExc_ValueError, + "generation parameter must be less than the number of " + "available generations (%i)", + NUM_GENERATIONS); goto error; } if (generation < 0) { - _PyErr_SetString(tstate, PyExc_ValueError, - "generation parameter cannot be negative"); + PyErr_SetString(PyExc_ValueError, + "generation parameter cannot be negative"); goto error; } @@ -2080,9 +2078,7 @@ PyGC_IsEnabled(void) Py_ssize_t PyGC_Collect(void) { - PyThreadState *tstate = _PyThreadState_GET(); - GCState *gcstate = &tstate->interp->gc; - + GCState *gcstate = get_gc_state(); if (!gcstate->enabled) { return 0; } @@ -2095,6 +2091,7 @@ PyGC_Collect(void) else { PyObject *exc, *value, *tb; gcstate->collecting = 1; + PyThreadState *tstate = _PyThreadState_GET(); _PyErr_Fetch(tstate, &exc, &value, &tb); n = gc_collect_with_callback(tstate, NUM_GENERATIONS - 1); _PyErr_Restore(tstate, exc, value, tb); @@ -2113,7 +2110,7 @@ _PyGC_CollectNoFail(PyThreadState *tstate) during interpreter shutdown (and then never finish it). See http://bugs.python.org/issue8713#msg195178 for an example. */ - GCState *gcstate = &tstate->interp->gc; + GCState *gcstate = get_gc_state(); if (gcstate->collecting) { return 0; } @@ -2126,9 +2123,9 @@ _PyGC_CollectNoFail(PyThreadState *tstate) } void -_PyGC_DumpShutdownStats(PyInterpreterState *interp) +_PyGC_DumpShutdownStats(void) { - GCState *gcstate = &interp->gc; + GCState *gcstate = get_gc_state(); if (!(gcstate->debug & DEBUG_SAVEALL) && gcstate->garbage != NULL && PyList_GET_SIZE(gcstate->garbage) > 0) { const char *message; @@ -2163,9 +2160,9 @@ _PyGC_DumpShutdownStats(PyInterpreterState *interp) } void -_PyGC_Fini(PyInterpreterState *interp) +_PyGC_Fini(void) { - GCState *gcstate = &interp->gc; + GCState *gcstate = get_gc_state(); Py_CLEAR(gcstate->garbage); Py_CLEAR(gcstate->callbacks); } @@ -2235,10 +2232,9 @@ PyObject_IS_GC(PyObject *obj) static PyObject * _PyObject_GC_Alloc(int use_calloc, size_t basicsize) { - PyThreadState *tstate = _PyThreadState_GET(); - GCState *gcstate = &tstate->interp->gc; + GCState *gcstate = get_gc_state(); if (basicsize > PY_SSIZE_T_MAX - sizeof(PyGC_Head)) { - return _PyErr_NoMemory(tstate); + return PyErr_NoMemory(); } size_t size = sizeof(PyGC_Head) + basicsize; @@ -2250,7 +2246,7 @@ _PyObject_GC_Alloc(int use_calloc, size_t basicsize) g = (PyGC_Head *)PyObject_Malloc(size); } if (g == NULL) { - return _PyErr_NoMemory(tstate); + return PyErr_NoMemory(); } assert(((uintptr_t)g & 3) == 0); // g must be aligned 4bytes boundary @@ -2261,9 +2257,10 @@ _PyObject_GC_Alloc(int use_calloc, size_t basicsize) gcstate->enabled && gcstate->generations[0].threshold && !gcstate->collecting && - !_PyErr_Occurred(tstate)) + !PyErr_Occurred()) { gcstate->collecting = 1; + PyThreadState *tstate = _PyThreadState_GET(); gc_collect_generations(tstate); gcstate->collecting = 0; } diff --git a/Objects/object.c b/Objects/object.c index ff816cd5b9a605..84e1d77c42315f 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2099,8 +2099,7 @@ Py_ReprLeave(PyObject *obj) void _PyTrash_deposit_object(PyObject *op) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - struct _gc_runtime_state *gcstate = &interp->gc; + struct _gc_runtime_state *gcstate = _PyGC_GetState(); _PyObject_ASSERT(op, _PyObject_IS_GC(op)); _PyObject_ASSERT(op, !_PyObject_GC_IS_TRACKED(op)); @@ -2127,8 +2126,7 @@ _PyTrash_thread_deposit_object(PyObject *op) void _PyTrash_destroy_chain(void) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - struct _gc_runtime_state *gcstate = &interp->gc; + struct _gc_runtime_state *gcstate = _PyGC_GetState(); while (gcstate->trash_delete_later) { PyObject *op = gcstate->trash_delete_later; diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index eeaf20b4617a20..225a581766289d 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -801,10 +801,12 @@ pycore_interp_init(PyThreadState *tstate) return status; } - // The GC must be initialized before the first GC collection. - status = _PyGC_Init(interp); - if (_PyStatus_EXCEPTION(status)) { - return status; + if (_Py_IsMainInterpreter(interp)) { + // The GC must be initialized before the first GC collection. + status = _PyGC_Init(); + if (_PyStatus_EXCEPTION(status)) { + return status; + } } status = pycore_init_types(interp); @@ -1526,7 +1528,7 @@ finalize_modules(PyThreadState *tstate) // Dump GC stats before it's too late, since it uses the warnings // machinery. - _PyGC_DumpShutdownStats(interp); + _PyGC_DumpShutdownStats(); if (weaklist != NULL) { // Now, if there are any modules left alive, clear their globals to diff --git a/Python/pystate.c b/Python/pystate.c index df98eb11bb0a49..4141cbb73c069c 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -327,7 +327,9 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) /* Last garbage collection on this interpreter */ _PyGC_CollectNoFail(tstate); - _PyGC_Fini(interp); + if (_Py_IsMainInterpreter(interp)) { + _PyGC_Fini(); + } /* We don't clear sysdict and builtins until the end of this function. Because clearing other attributes can execute arbitrary Python code diff --git a/Python/traceback.c b/Python/traceback.c index 7d6f7f435a6ba6..bf52602ed0dc4e 100644 --- a/Python/traceback.c +++ b/Python/traceback.c @@ -5,6 +5,7 @@ #include "code.h" #include "pycore_interp.h" // PyInterpreterState.gc +#include "pycore_object.h" // _PyGC_GetState() #include "frameobject.h" // PyFrame_GetBack() #include "structmember.h" // PyMemberDef #include "osdefs.h" // SEP @@ -925,6 +926,7 @@ _Py_DumpTracebackThreads(int fd, PyInterpreterState *interp, return "unable to get the thread head state"; /* Dump the traceback of each thread */ + struct _gc_runtime_state *gcstate = _PyGC_GetState(); tstate = PyInterpreterState_ThreadHead(interp); nthreads = 0; _Py_BEGIN_SUPPRESS_IPH @@ -937,7 +939,7 @@ _Py_DumpTracebackThreads(int fd, PyInterpreterState *interp, break; } write_thread_id(fd, tstate, tstate == current_tstate); - if (tstate == current_tstate && tstate->interp->gc.collecting) { + if (tstate == current_tstate && gcstate->collecting) { PUTS(fd, " Garbage-collecting\n"); } dump_traceback(fd, tstate, 0);