gh-84436: Immortalize in _PyStructSequence_InitBuiltinWithFlags() (gh…

…-104054) This also does some cleanup.
python · May 1, 2023 · 59bc36a · 59bc36a
1 parent d448fcb
commit 59bc36a
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 59 deletions.
diff --git a/Include/internal/pycore_structseq.h b/Include/internal/pycore_structseq.h
@@ -15,7 +15,7 @@ PyAPI_FUNC(PyTypeObject *) _PyStructSequence_NewType(
     PyStructSequence_Desc *desc,
     unsigned long tp_flags);
 
-PyAPI_FUNC(int) _PyStructSequence_InitBuiltinWithFlags(
+extern int _PyStructSequence_InitBuiltinWithFlags(
     PyTypeObject *type,
     PyStructSequence_Desc *desc,
     unsigned long tp_flags);
@@ -27,7 +27,7 @@ _PyStructSequence_InitBuiltin(PyTypeObject *type,
     return _PyStructSequence_InitBuiltinWithFlags(type, desc, 0);
 }
 
-extern void _PyStructSequence_FiniType(PyTypeObject *type);
+extern void _PyStructSequence_FiniBuiltin(PyTypeObject *type);
 
 #ifdef __cplusplus
 }

diff --git a/Objects/floatobject.c b/Objects/floatobject.c
@@ -12,7 +12,7 @@
 #include "pycore_object.h"        // _PyObject_Init()
 #include "pycore_pymath.h"        // _PY_SHORT_FLOAT_REPR
 #include "pycore_pystate.h"       // _PyInterpreterState_GET()
-#include "pycore_structseq.h"     // _PyStructSequence_FiniType()
+#include "pycore_structseq.h"     // _PyStructSequence_FiniBuiltin()
 
 #include <ctype.h>
 #include <float.h>
@@ -2029,7 +2029,7 @@ void
 _PyFloat_FiniType(PyInterpreterState *interp)
 {
     if (_Py_IsMainInterpreter(interp)) {
-        _PyStructSequence_FiniType(&FloatInfoType);
+        _PyStructSequence_FiniBuiltin(&FloatInfoType);
     }
 }
 

diff --git a/Objects/longobject.c b/Objects/longobject.c
@@ -9,7 +9,7 @@
 #include "pycore_object.h"        // _PyObject_Init()
 #include "pycore_pystate.h"       // _Py_IsMainInterpreter()
 #include "pycore_runtime.h"       // _PY_NSMALLPOSINTS
-#include "pycore_structseq.h"     // _PyStructSequence_FiniType()
+#include "pycore_structseq.h"     // _PyStructSequence_FiniBuiltin()
 
 #include <ctype.h>
 #include <float.h>
@@ -6367,5 +6367,5 @@ _PyLong_FiniTypes(PyInterpreterState *interp)
         return;
     }
 
-    _PyStructSequence_FiniType(&Int_InfoType);
+    _PyStructSequence_FiniBuiltin(&Int_InfoType);
 }
diff --git a/Objects/structseq.c b/Objects/structseq.c
@@ -433,12 +433,10 @@ initialize_structseq_dict(PyStructSequence_Desc *desc, PyObject* dict,
 
 static PyMemberDef *
 initialize_members(PyStructSequence_Desc *desc,
-                   Py_ssize_t *pn_members, Py_ssize_t *pn_unnamed_members)
+                   Py_ssize_t n_members, Py_ssize_t n_unnamed_members)
 {
     PyMemberDef *members;
-    Py_ssize_t n_members, n_unnamed_members;
 
-    n_members = count_members(desc, &n_unnamed_members);
     members = PyMem_NEW(PyMemberDef, n_members - n_unnamed_members + 1);
     if (members == NULL) {
         PyErr_NoMemory();
@@ -463,8 +461,6 @@ initialize_members(PyStructSequence_Desc *desc,
     }
     members[k].name = NULL;
 
-    *pn_members = n_members;
-    *pn_unnamed_members = n_unnamed_members;
     return members;
 }
 
@@ -510,39 +506,58 @@ _PyStructSequence_InitBuiltinWithFlags(PyTypeObject *type,
                                        PyStructSequence_Desc *desc,
                                        unsigned long tp_flags)
 {
-    if (type->tp_flags & Py_TPFLAGS_READY) {
-        if (_PyStaticType_InitBuiltin(type) < 0) {
-            goto failed_init_builtin;
+    Py_ssize_t n_unnamed_members;
+    Py_ssize_t n_members = count_members(desc, &n_unnamed_members);
+    PyMemberDef *members = NULL;
+
+    int initialized = 1;
+    if ((type->tp_flags & Py_TPFLAGS_READY) == 0) {
+        assert(type->tp_name == NULL);
+        assert(type->tp_members == NULL);
+        assert(type->tp_base == NULL);
+
+        members = initialize_members(desc, n_members, n_unnamed_members);
+        if (members == NULL) {
+            goto error;
         }
-        return 0;
-    }
+        initialize_static_fields(type, desc, members, tp_flags);
 
-    PyMemberDef *members;
-    Py_ssize_t n_members, n_unnamed_members;
-
-    members = initialize_members(desc, &n_members, &n_unnamed_members);
-    if (members == NULL) {
-        return -1;
+        _Py_SetImmortal(type);
+        initialized = 0;
+    }
+#ifndef NDEBUG
+    else {
+        // Ensure that the type was initialized.
+        assert(type->tp_name != NULL);
+        assert(type->tp_members != NULL);
+        assert(type->tp_base == &PyTuple_Type);
+        assert((type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN));
+        assert(_Py_IsImmortal(type));
     }
-    initialize_static_fields(type, desc, members, tp_flags);
+#endif
 
-    Py_INCREF(type);  // XXX It should be immortal.
     if (_PyStaticType_InitBuiltin(type) < 0) {
-        PyMem_Free(members);
-        goto failed_init_builtin;
+        PyErr_Format(PyExc_RuntimeError,
+                     "Can't initialize builtin type %s",
+                     desc->name);
+        goto error;
+    }
+    // This should be dropped if tp_dict is made per-interpreter.
+    if (initialized) {
+        return 0;
     }
 
     if (initialize_structseq_dict(
             desc, type->tp_dict, n_members, n_unnamed_members) < 0) {
-        PyMem_Free(members);
-        return -1;
+        goto error;
     }
+
     return 0;
 
-failed_init_builtin:
-    PyErr_Format(PyExc_RuntimeError,
-                 "Can't initialize builtin type %s",
-                 desc->name);
+error:
+    if (members != NULL) {
+        PyMem_Free(members);
+    }
     return -1;
 }
 
@@ -566,7 +581,8 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc)
         return -1;
     }
 
-    members = initialize_members(desc, &n_members, &n_unnamed_members);
+    n_members = count_members(desc, &n_unnamed_members);
+    members = initialize_members(desc, n_members, n_unnamed_members);
     if (members == NULL) {
         return -1;
     }
@@ -585,35 +601,32 @@ PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc)
 }
 
 
+/* This is exposed in the internal API, not the public API.
+   It is only called on builtin static types, which are all
+   initialized via _PyStructSequence_InitBuiltinWithFlags(). */
+
 void
-_PyStructSequence_FiniType(PyTypeObject *type)
+_PyStructSequence_FiniBuiltin(PyTypeObject *type)
 {
     // Ensure that the type is initialized
     assert(type->tp_name != NULL);
     assert(type->tp_base == &PyTuple_Type);
+    assert((type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN));
+    assert(_Py_IsImmortal(type));
 
     // Cannot delete a type if it still has subclasses
     if (_PyType_HasSubclasses(type)) {
+        // XXX Shouldn't this be an error?
         return;
     }
 
-    // Undo PyStructSequence_NewType()
-    type->tp_name = NULL;
-    PyMem_Free(type->tp_members);
-
     _PyStaticType_Dealloc(type);
-    assert(Py_REFCNT(type) == 1);
-    // Undo Py_INCREF(type) of _PyStructSequence_InitType().
-    // Don't use Py_DECREF(): static type must not be deallocated
-    Py_SET_REFCNT(type, 0);
-#ifdef Py_REF_DEBUG
-    _Py_DecRefTotal(_PyInterpreterState_GET());
-#endif
 
-    // Make sure that _PyStructSequence_InitType() will initialize
-    // the type again
-    assert(Py_REFCNT(type) == 0);
-    assert(type->tp_name == NULL);
+    // Undo _PyStructSequence_InitBuiltinWithFlags().
+    type->tp_name = NULL;
+    PyMem_Free(type->tp_members);
+    type->tp_members = NULL;
+    type->tp_base = NULL;
 }
 
 
@@ -627,7 +640,8 @@ _PyStructSequence_NewType(PyStructSequence_Desc *desc, unsigned long tp_flags)
     Py_ssize_t n_members, n_unnamed_members;
 
     /* Initialize MemberDefs */
-    members = initialize_members(desc, &n_members, &n_unnamed_members);
+    n_members = count_members(desc, &n_unnamed_members);
+    members = initialize_members(desc, n_members, n_unnamed_members);
     if (members == NULL) {
         return NULL;
     }

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -7031,6 +7031,7 @@ PyType_Ready(PyTypeObject *type)
 int
 _PyStaticType_InitBuiltin(PyTypeObject *self)
 {
+    assert(_Py_IsImmortal((PyObject *)self));
     assert(!(self->tp_flags & Py_TPFLAGS_HEAPTYPE));
 
     if (self->tp_flags & Py_TPFLAGS_READY) {

diff --git a/Python/errors.c b/Python/errors.c
@@ -6,7 +6,7 @@
 #include "pycore_initconfig.h"    // _PyStatus_ERR()
 #include "pycore_pyerrors.h"      // _PyErr_Format()
 #include "pycore_pystate.h"       // _PyThreadState_GET()
-#include "pycore_structseq.h"     // _PyStructSequence_FiniType()
+#include "pycore_structseq.h"     // _PyStructSequence_FiniBuiltin()
 #include "pycore_sysmodule.h"     // _PySys_Audit()
 #include "pycore_traceback.h"     // _PyTraceBack_FromFrame()
 
@@ -1357,7 +1357,7 @@ _PyErr_FiniTypes(PyInterpreterState *interp)
         return;
     }
 
-    _PyStructSequence_FiniType(&UnraisableHookArgsType);
+    _PyStructSequence_FiniBuiltin(&UnraisableHookArgsType);
 }
 
 

diff --git a/Python/sysmodule.c b/Python/sysmodule.c
@@ -3492,13 +3492,13 @@ void
 _PySys_Fini(PyInterpreterState *interp)
 {
     if (_Py_IsMainInterpreter(interp)) {
-        _PyStructSequence_FiniType(&VersionInfoType);
-        _PyStructSequence_FiniType(&FlagsType);
+        _PyStructSequence_FiniBuiltin(&VersionInfoType);
+        _PyStructSequence_FiniBuiltin(&FlagsType);
 #if defined(MS_WINDOWS)
-        _PyStructSequence_FiniType(&WindowsVersionType);
+        _PyStructSequence_FiniBuiltin(&WindowsVersionType);
 #endif
-        _PyStructSequence_FiniType(&Hash_InfoType);
-        _PyStructSequence_FiniType(&AsyncGenHooksType);
+        _PyStructSequence_FiniBuiltin(&Hash_InfoType);
+        _PyStructSequence_FiniBuiltin(&AsyncGenHooksType);
 #ifdef __EMSCRIPTEN__
         Py_CLEAR(EmscriptenInfoType);
 #endif

diff --git a/Python/thread.c b/Python/thread.c
@@ -7,7 +7,7 @@
 
 #include "Python.h"
 #include "pycore_pystate.h"       // _PyInterpreterState_GET()
-#include "pycore_structseq.h"     // _PyStructSequence_FiniType()
+#include "pycore_structseq.h"     // _PyStructSequence_FiniBuiltin()
 #include "pycore_pythread.h"
 
 #ifndef DONT_HAVE_STDIO_H
@@ -195,5 +195,5 @@ _PyThread_FiniType(PyInterpreterState *interp)
         return;
     }
 
-    _PyStructSequence_FiniType(&ThreadInfoType);
+    _PyStructSequence_FiniBuiltin(&ThreadInfoType);
 }