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

This also does some cleanup.

Author: ericsnowcurrently

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
 }

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);
  }
 }
 

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);
 }

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;
  }

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) {

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);
 }
 
 

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

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);
 }