From 271e017401075d1e4faad287daf863ac384ce849 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 18 Nov 2020 20:33:59 +0100 Subject: [PATCH 1/3] bpo-1635741: Port gc module to multiphase initialization Signed-off-by: Christian Heimes --- ...2020-11-18-20-33-35.bpo-1635741.B4ztSk.rst | 1 + Modules/gcmodule.c | 68 +++++++++---------- 2 files changed, 33 insertions(+), 36 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2020-11-18-20-33-35.bpo-1635741.B4ztSk.rst diff --git a/Misc/NEWS.d/next/C API/2020-11-18-20-33-35.bpo-1635741.B4ztSk.rst b/Misc/NEWS.d/next/C API/2020-11-18-20-33-35.bpo-1635741.B4ztSk.rst new file mode 100644 index 00000000000000..bce80c86de5019 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2020-11-18-20-33-35.bpo-1635741.B4ztSk.rst @@ -0,0 +1 @@ +Port :mod:`gc` extension module to multiphase initialization (:pep:`489`) diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index e6ad0f2dd42227..93ee3990b962b3 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -1992,59 +1992,55 @@ static PyMethodDef GcMethods[] = { {NULL, NULL} /* Sentinel */ }; -static struct PyModuleDef gcmodule = { - PyModuleDef_HEAD_INIT, - "gc", /* m_name */ - gc__doc__, /* m_doc */ - -1, /* m_size */ - GcMethods, /* m_methods */ - NULL, /* m_reload */ - NULL, /* m_traverse */ - NULL, /* m_clear */ - NULL /* m_free */ -}; - -PyMODINIT_FUNC -PyInit_gc(void) +static int +gcmodule_exec(PyObject *module) { GCState *gcstate = get_gc_state(); - PyObject *m = PyModule_Create(&gcmodule); - - if (m == NULL) { - return NULL; - } - + gcstate->garbage = PyList_New(0); if (gcstate->garbage == NULL) { - gcstate->garbage = PyList_New(0); - if (gcstate->garbage == NULL) { - return NULL; - } + return -1; } - Py_INCREF(gcstate->garbage); - if (PyModule_AddObject(m, "garbage", gcstate->garbage) < 0) { - return NULL; + if (PyModule_AddObjectRef(module, "garbage", gcstate->garbage) < 0) { + return -1; } + gcstate->callbacks = PyList_New(0); if (gcstate->callbacks == NULL) { - gcstate->callbacks = PyList_New(0); - if (gcstate->callbacks == NULL) { - return NULL; - } + return -1; } - Py_INCREF(gcstate->callbacks); - if (PyModule_AddObject(m, "callbacks", gcstate->callbacks) < 0) { - return NULL; + if (PyModule_AddObjectRef(module, "callbacks", gcstate->callbacks) < 0) { + return -1; } -#define ADD_INT(NAME) if (PyModule_AddIntConstant(m, #NAME, NAME) < 0) { return NULL; } +#define ADD_INT(NAME) if (PyModule_AddIntConstant(module, #NAME, NAME) < 0) { return -1; } ADD_INT(DEBUG_STATS); ADD_INT(DEBUG_COLLECTABLE); ADD_INT(DEBUG_UNCOLLECTABLE); ADD_INT(DEBUG_SAVEALL); ADD_INT(DEBUG_LEAK); #undef ADD_INT - return m; + return 0; +} + +static PyModuleDef_Slot gcmodule_slots[] = { + {Py_mod_exec, gcmodule_exec}, + {0, NULL} +}; + +static struct PyModuleDef gcmodule = { + PyModuleDef_HEAD_INIT, + .m_name = "gc", + .m_doc = gc__doc__, + .m_size = 0, /* special case, state is part of interpreter state */ + .m_methods = GcMethods, + .m_slots = gcmodule_slots +}; + +PyMODINIT_FUNC +PyInit_gc(void) +{ + return PyModuleDef_Init(&gcmodule); } /* Public API to invoke gc.collect() from C */ From 331d6762eb67388535a9b7df08412bf60f8efb4c Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 19 Nov 2020 08:44:15 +0100 Subject: [PATCH 2/3] Fix reference leak Signed-off-by: Christian Heimes --- Modules/gcmodule.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 93ee3990b962b3..8ba7bd45f9db35 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -1997,14 +1997,17 @@ gcmodule_exec(PyObject *module) { GCState *gcstate = get_gc_state(); - gcstate->garbage = PyList_New(0); + /* Initialized by _PyGC_Init() early in interpreter lifecycle */ if (gcstate->garbage == NULL) { + PyErr_SetString(PyExc_SystemError, + "GC garbage bin is not initialized"); return -1; } if (PyModule_AddObjectRef(module, "garbage", gcstate->garbage) < 0) { return -1; } + assert(gcstate->callbacks == NULL); gcstate->callbacks = PyList_New(0); if (gcstate->callbacks == NULL) { return -1; From 7ad98d9bbb48ce1a0bcdd6c39f457bbcf958d46c Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 19 Nov 2020 09:00:59 +0100 Subject: [PATCH 3/3] Create gc callbacks in _PyGC_init() --- Modules/gcmodule.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 8ba7bd45f9db35..45201435f24605 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -165,12 +165,17 @@ PyStatus _PyGC_Init(PyThreadState *tstate) { GCState *gcstate = &tstate->interp->gc; + + gcstate->garbage = PyList_New(0); if (gcstate->garbage == NULL) { - gcstate->garbage = PyList_New(0); - if (gcstate->garbage == NULL) { - return _PyStatus_NO_MEMORY(); - } + return _PyStatus_NO_MEMORY(); + } + + gcstate->callbacks = PyList_New(0); + if (gcstate->callbacks == NULL) { + return _PyStatus_NO_MEMORY(); } + return _PyStatus_OK(); } @@ -1997,21 +2002,13 @@ gcmodule_exec(PyObject *module) { GCState *gcstate = get_gc_state(); - /* Initialized by _PyGC_Init() early in interpreter lifecycle */ - if (gcstate->garbage == NULL) { - PyErr_SetString(PyExc_SystemError, - "GC garbage bin is not initialized"); - return -1; - } + /* garbage and callbacks are initialized by _PyGC_Init() early in + * interpreter lifecycle. */ + assert(gcstate->garbage != NULL); if (PyModule_AddObjectRef(module, "garbage", gcstate->garbage) < 0) { return -1; } - - assert(gcstate->callbacks == NULL); - gcstate->callbacks = PyList_New(0); - if (gcstate->callbacks == NULL) { - return -1; - } + assert(gcstate->callbacks != NULL); if (PyModule_AddObjectRef(module, "callbacks", gcstate->callbacks) < 0) { return -1; } @@ -2035,7 +2032,7 @@ static struct PyModuleDef gcmodule = { PyModuleDef_HEAD_INIT, .m_name = "gc", .m_doc = gc__doc__, - .m_size = 0, /* special case, state is part of interpreter state */ + .m_size = 0, // per interpreter state, see: get_gc_state() .m_methods = GcMethods, .m_slots = gcmodule_slots };