Further thread safety changes for Python 3.13 free threading

See #1683

direct/src/dcparser/dcClass_ext.cxx

@@ -522,31 +522,35 @@ client_format_generate_CMU(PyObject *distobj, DOID_TYPE do_id,
   }
 
   // Also specify the optional fields.
-  int num_optional_fields = 0;
   if (PyObject_IsTrue(optional_fields)) {
-    num_optional_fields = PySequence_Size(optional_fields);
-  }
-  packer.raw_pack_uint16(num_optional_fields);
-
-  for (int i = 0; i < num_optional_fields; i++) {
-    PyObject *py_field_name = PySequence_GetItem(optional_fields, i);
-    std::string field_name = PyUnicode_AsUTF8(py_field_name);
-    Py_XDECREF(py_field_name);
-
-    DCField *field = _this->get_field_by_name(field_name);
-    if (field == nullptr) {
-      std::ostringstream strm;
-      strm << "No field named " << field_name << " in class " << _this->get_name()
-           << "\n";
-      nassert_raise(strm.str());
-      return Datagram();
-    }
-    packer.raw_pack_uint16(field->get_number());
-    packer.begin_pack(field);
-    if (!pack_required_field(packer, distobj, field)) {
-      return Datagram();
+    optional_fields = PySequence_Tuple(optional_fields);
+    Py_ssize_t num_optional_fields = PyTuple_GET_SIZE(optional_fields);
+    packer.raw_pack_uint16(num_optional_fields);
+
+    for (Py_ssize_t i = 0; i < num_optional_fields; i++) {
+      PyObject *py_field_name = PyTuple_GET_ITEM(optional_fields, i);
+      std::string field_name = PyUnicode_AsUTF8(py_field_name);
+
+      DCField *field = _this->get_field_by_name(field_name);
+      if (field == nullptr) {
+        std::ostringstream strm;
+        strm << "No field named " << field_name << " in class " << _this->get_name()
+             << "\n";
+        nassert_raise(strm.str());
+        Py_DECREF(optional_fields);
+        return Datagram();
+      }
+      packer.raw_pack_uint16(field->get_number());
+      packer.begin_pack(field);
+      if (!pack_required_field(packer, distobj, field)) {
+        Py_DECREF(optional_fields);
+        return Datagram();
+      }
+      packer.end_pack();
     }
-    packer.end_pack();
+    Py_DECREF(optional_fields);
+  } else {
+    packer.raw_pack_uint16(0);
   }
 
   return Datagram(packer.get_data(), packer.get_length());
@@ -602,13 +606,13 @@ ai_format_generate(PyObject *distobj, DOID_TYPE do_id,
 
   // Also specify the optional fields.
   if (has_optional_fields) {
-    int num_optional_fields = PySequence_Size(optional_fields);
+    optional_fields = PySequence_Tuple(optional_fields);
+    Py_ssize_t num_optional_fields = PyTuple_GET_SIZE(optional_fields);
     packer.raw_pack_uint16(num_optional_fields);
 
-    for (int i = 0; i < num_optional_fields; ++i) {
-      PyObject *py_field_name = PySequence_GetItem(optional_fields, i);
+    for (Py_ssize_t i = 0; i < num_optional_fields; ++i) {
+      PyObject *py_field_name = PyTuple_GET_ITEM(optional_fields, i);
       std::string field_name = PyUnicode_AsUTF8(py_field_name);
-      Py_XDECREF(py_field_name);
 
       DCField *field = _this->get_field_by_name(field_name);
       if (field == nullptr) {
@@ -616,6 +620,7 @@ ai_format_generate(PyObject *distobj, DOID_TYPE do_id,
         strm << "No field named " << field_name << " in class "
              << _this->get_name() << "\n";
         nassert_raise(strm.str());
+        Py_DECREF(optional_fields);
         return Datagram();
       }
 
@@ -623,10 +628,13 @@ ai_format_generate(PyObject *distobj, DOID_TYPE do_id,
 
       packer.begin_pack(field);
       if (!pack_required_field(packer, distobj, field)) {
+        Py_DECREF(optional_fields);
         return Datagram();
       }
       packer.end_pack();
     }
+
+    Py_DECREF(optional_fields);
   }
 
   return Datagram(packer.get_data(), packer.get_length());

direct/src/dcparser/dcPacker_ext.cxx

@@ -136,6 +136,7 @@ pack_object(PyObject *object) {
       // The supplied object is not an instance of the expected class object,
       // but it is a sequence.  This is case (2).
       _this->push();
+      Py_BEGIN_CRITICAL_SECTION(object);
       int size = PySequence_Size(object);
       for (int i = 0; i < size; ++i) {
         PyObject *element = PySequence_GetItem(object, i);
@@ -146,6 +147,7 @@ pack_object(PyObject *object) {
           std::cerr << "Unable to extract item " << i << " from sequence.\n";
         }
       }
+      Py_END_CRITICAL_SECTION();
       _this->pop();
     } else {
       // The supplied object is not a sequence, and we weren't expecting a

dtool/Config.cmake

@@ -265,7 +265,7 @@ if(BUILD_INTERROGATE)
     panda3d-interrogate
 
     GIT_REPOSITORY https://github.com/panda3d/interrogate.git
-    GIT_TAG 742ea56a1934a7a05ee2518787cf6816f30ee096
+    GIT_TAG c343350a6e210029cfe3fd8468e530e1ea6bcead
 
     PREFIX ${_interrogate_dir}
     CMAKE_ARGS

dtool/src/interrogatedb/dtool_super_base.cxx

@@ -61,8 +61,18 @@ static void Dtool_FreeInstance_DTOOL_SUPER_BASE(PyObject *self) {
  */
 Dtool_PyTypedObject *Dtool_GetSuperBase() {
   Dtool_TypeMap *type_map = Dtool_GetGlobalTypeMap();
+
+  // If we don't have the GIL, we have to protect this with a lock to make
+  // sure that there is only one DTOOL_SUPER_BASE instance in the world.
+#ifdef Py_GIL_DISABLED
+  PyMutex_Lock(&type_map->_lock);
+#endif
+
   auto it = type_map->find("DTOOL_SUPER_BASE");
   if (it != type_map->end()) {
+#ifdef Py_GIL_DISABLED
+    PyMutex_Unlock(&type_map->_lock);
+#endif
     return it->second;
   }
 
@@ -148,6 +158,9 @@ Dtool_PyTypedObject *Dtool_GetSuperBase() {
 
   if (PyType_Ready((PyTypeObject *)&super_base_type) < 0) {
     PyErr_SetString(PyExc_TypeError, "PyType_Ready(Dtool_DTOOL_SUPER_BASE)");
+#ifdef Py_GIL_DISABLED
+    PyMutex_Unlock(&type_map->_lock);
+#endif
     return nullptr;
   }
   Py_INCREF(&super_base_type._PyType);
@@ -155,6 +168,9 @@ Dtool_PyTypedObject *Dtool_GetSuperBase() {
   PyDict_SetItemString(super_base_type._PyType.tp_dict, "DtoolGetSuperBase", PyCFunction_New(&methods[0], (PyObject *)&super_base_type));
 
   (*type_map)["DTOOL_SUPER_BASE"] = &super_base_type;
+#ifdef Py_GIL_DISABLED
+  PyMutex_Unlock(&type_map->_lock);
+#endif
   return &super_base_type;
 }
 

dtool/src/interrogatedb/py_panda.cxx

@@ -375,23 +375,26 @@ static PyObject *Dtool_EnumType_Repr(PyObject *self) {
  * should be a tuple of (name, value) pairs.
  */
 PyTypeObject *Dtool_EnumType_Create(const char *name, PyObject *names, const char *module) {
-  static PyObject *enum_class = nullptr;
 #if PY_VERSION_HEX >= 0x03040000
-  static PyObject *enum_meta = nullptr;
-  static PyObject *enum_create = nullptr;
-  if (enum_meta == nullptr) {
-    PyObject *enum_module = PyImport_ImportModule("enum");
-    nassertr_always(enum_module != nullptr, nullptr);
+  PyObject *enum_module = PyImport_ImportModule("enum");
+  nassertr_always(enum_module != nullptr, nullptr);
 
-    enum_class = PyObject_GetAttrString(enum_module, "Enum");
-    enum_meta = PyObject_GetAttrString(enum_module, "EnumMeta");
-    enum_create = PyObject_GetAttrString(enum_meta, "_create_");
-    nassertr(enum_meta != nullptr, nullptr);
-  }
+  PyObject *enum_meta = PyObject_GetAttrString(enum_module, "EnumMeta");
+  nassertr(enum_meta != nullptr, nullptr);
+
+  PyObject *enum_class = PyObject_GetAttrString(enum_module, "Enum");
+  Py_DECREF(enum_module);
+  nassertr(enum_class != nullptr, nullptr);
+
+  PyObject *enum_create = PyObject_GetAttrString(enum_meta, "_create_");
+  Py_DECREF(enum_meta);
 
   PyObject *result = PyObject_CallFunction(enum_create, (char *)"OsN", enum_class, name, names);
+  Py_DECREF(enum_create);
+  Py_DECREF(enum_class);
   nassertr(result != nullptr, nullptr);
 #else
+  static PyObject *enum_class = nullptr;
   static PyObject *name_str;
   static PyObject *name_sunder_str;
   static PyObject *value_str;
@@ -528,16 +531,39 @@ PyObject *DTool_CreatePyInstance(void *local_this, Dtool_PyTypedObject &in_class
  * Returns a borrowed reference to the global type dictionary.
  */
 Dtool_TypeMap *Dtool_GetGlobalTypeMap() {
+#if PY_VERSION_HEX >= 0x030d0000 // 3.13
+  PyObject *istate_dict = PyInterpreterState_GetDict(PyInterpreterState_Get());
+  PyObject *key = PyUnicode_InternFromString("_interrogate_types");
+  PyObject *capsule = PyDict_GetItem(istate_dict, key);
+  if (capsule != nullptr) {
+    Py_DECREF(key);
+    return (Dtool_TypeMap *)PyCapsule_GetPointer(capsule, nullptr);
+  }
+#else
   PyObject *capsule = PySys_GetObject((char *)"_interrogate_types");
   if (capsule != nullptr) {
     return (Dtool_TypeMap *)PyCapsule_GetPointer(capsule, nullptr);
-  } else {
-    Dtool_TypeMap *type_map = new Dtool_TypeMap;
-    capsule = PyCapsule_New((void *)type_map, nullptr, nullptr);
-    PySys_SetObject((char *)"_interrogate_types", capsule);
+  }
+#endif
+
+  Dtool_TypeMap *type_map = new Dtool_TypeMap;
+  capsule = PyCapsule_New((void *)type_map, nullptr, nullptr);
+
+#if PY_VERSION_HEX >= 0x030d0000 // 3.13
+  PyObject *result;
+  if (PyDict_SetDefaultRef(istate_dict, key, capsule, &result) != 0) {
+    // Another thread already beat us to it.
     Py_DECREF(capsule);
-    return type_map;
+    delete type_map;
+    capsule = result;
+    type_map = (Dtool_TypeMap *)PyCapsule_GetPointer(capsule, nullptr);
   }
+  Py_DECREF(key);
+#endif
+
+  PySys_SetObject((char *)"_interrogate_types", capsule);
+  Py_DECREF(capsule);
+  return type_map;
 }
 
 /**
@@ -594,6 +620,10 @@ PyObject *Dtool_PyModuleInitHelper(const LibraryDef *defs[], const char *modulen
 
   Dtool_TypeMap *type_map = Dtool_GetGlobalTypeMap();
 
+#ifdef Py_GIL_DISABLED
+  PyMutex_Lock(&type_map->_lock);
+#endif
+
   // the module level function inits....
   MethodDefmap functions;
   for (size_t i = 0; defs[i] != nullptr; i++) {
@@ -627,12 +657,19 @@ PyObject *Dtool_PyModuleInitHelper(const LibraryDef *defs[], const char *modulen
         if (it != type_map->end()) {
           types->type = it->second;
         } else {
-          return PyErr_Format(PyExc_NameError, "name '%s' is not defined", types->name);
+          PyErr_Format(PyExc_NameError, "name '%s' is not defined", types->name);
+#ifdef Py_GIL_DISABLED
+          PyMutex_Unlock(&type_map->_lock);
+#endif
+          return nullptr;
         }
         ++types;
       }
     }
   }
+#ifdef Py_GIL_DISABLED
+  PyMutex_Unlock(&type_map->_lock);
+#endif
 
   PyMethodDef *newdef = new PyMethodDef[functions.size() + 1];
   MethodDefmap::iterator mi;

dtool/src/interrogatedb/py_panda.h

@@ -181,7 +181,14 @@ static void Dtool_FreeInstance_##CLASS_NAME(PyObject *self) {\
 // forward declared of typed object.  We rely on the fact that typed objects
 // are uniquly defined by an integer.
 
+#if PY_VERSION_HEX >= 0x030d0000
+class Dtool_TypeMap : public std::map<std::string, Dtool_PyTypedObject *> {
+public:
+  PyMutex _lock { 0 };
+};
+#else
 typedef std::map<std::string, Dtool_PyTypedObject *> Dtool_TypeMap;
+#endif
 
 EXPCL_PYPANDA Dtool_TypeMap *Dtool_GetGlobalTypeMap();
 

makepanda/makepandacore.py

@@ -592,8 +592,8 @@ def GetInterrogateDir():
             return INTERROGATE_DIR
 
         dir = os.path.join(GetOutputDir(), "tmp", "interrogate")
-        if not os.path.isdir(os.path.join(dir, "panda3d_interrogate-0.1.1.dist-info")):
-            oscmd("\"%s\" -m pip install --force-reinstall -t \"%s\" panda3d-interrogate==0.1.1" % (sys.executable, dir))
+        if not os.path.isdir(os.path.join(dir, "panda3d_interrogate-0.2.0.dist-info")):
+            oscmd("\"%s\" -m pip install --force-reinstall -t \"%s\" panda3d-interrogate==0.2.0" % (sys.executable, dir))
 
         INTERROGATE_DIR = dir
 

panda/src/collide/collisionPolygon_ext.cxx

@@ -72,6 +72,9 @@ convert_points(pvector<LPoint3> &vec, PyObject *points) {
     return false;
   }
 
+  bool success = true;
+
+  Py_BEGIN_CRITICAL_SECTION(seq);
   PyObject **items = PySequence_Fast_ITEMS(seq);
   Py_ssize_t len = PySequence_Fast_GET_SIZE(seq);
   void *ptr;
@@ -90,13 +93,14 @@ convert_points(pvector<LPoint3> &vec, PyObject *points) {
     }
     else {
       Dtool_Raise_TypeError("Argument must be of LPoint3 type.");
-      Py_DECREF(seq);
-      return false;
+      success = false;
+      break;
     }
   }
 
+  Py_END_CRITICAL_SECTION();
   Py_DECREF(seq);
-  return true;
+  return success;
 }
 
 #endif

panda/src/express/pointerToArray_ext.I

@@ -106,12 +106,13 @@ __init__(PyObject *self, PyObject *source) {
   // We need to initialize the this pointer before we can call push_back.
   DtoolInstance_INIT_PTR(self, this->_this);
 
+  Py_BEGIN_CRITICAL_SECTION(source);
   Py_ssize_t size = PySequence_Size(source);
   this->_this->reserve(size);
   for (Py_ssize_t i = 0; i < size; ++i) {
     PyObject *item = PySequence_GetItem(source, i);
     if (item == nullptr) {
-      return;
+      break;
     }
     PyObject *result = PyObject_CallFunctionObjArgs(push_back, self, item, nullptr);
     Py_DECREF(item);
@@ -121,10 +122,11 @@ __init__(PyObject *self, PyObject *source) {
       PyErr_Format(PyExc_TypeError,
                    "Element %zd in sequence passed to PointerToArray "
                    "constructor could not be added", i);
-      return;
+      break;
     }
     Py_DECREF(result);
   }
+  Py_END_CRITICAL_SECTION();
 }
 
 /**

panda/src/gobj/textureCollection_ext.cxx

@@ -31,13 +31,14 @@ __init__(PyObject *self, PyObject *sequence) {
     return;
   }
 
+  Py_BEGIN_CRITICAL_SECTION(fast);
   Py_ssize_t size = PySequence_Fast_GET_SIZE(fast);
   _this->reserve(size);
 
   for (int i = 0; i < size; ++i) {
     PyObject *item = PySequence_Fast_GET_ITEM(fast, i);
     if (item == nullptr) {
-      return;
+      break;
     }
 
     Texture *tex;
@@ -48,13 +49,12 @@ __init__(PyObject *self, PyObject *sequence) {
       stream << "Element " << i << " in sequence passed to TextureCollection constructor is not a Texture";
       std::string str = stream.str();
       PyErr_SetString(PyExc_TypeError, str.c_str());
-      Py_DECREF(fast);
-      return;
+      break;
     } else {
       _this->add_texture(tex);
     }
   }
-
+  Py_END_CRITICAL_SECTION();
   Py_DECREF(fast);
 }
 

panda/src/pgraph/nodePathCollection_ext.cxx

@@ -36,13 +36,14 @@ __init__(PyObject *self, PyObject *sequence) {
     return;
   }
 
+  Py_BEGIN_CRITICAL_SECTION(fast);
   Py_ssize_t size = PySequence_Fast_GET_SIZE(fast);
   _this->reserve(size);
 
   for (int i = 0; i < size; ++i) {
     PyObject *item = PySequence_Fast_GET_ITEM(fast, i);
     if (item == nullptr) {
-      return;
+      break;
     }
 
     NodePath *path;
@@ -52,13 +53,12 @@ __init__(PyObject *self, PyObject *sequence) {
       stream << "Element " << i << " in sequence passed to NodePathCollection constructor is not a NodePath";
       std::string str = stream.str();
       PyErr_SetString(PyExc_TypeError, str.c_str());
-      Py_DECREF(fast);
-      return;
+      break;
     } else {
       _this->add_path(*path);
     }
   }
-
+  Py_END_CRITICAL_SECTION();
   Py_DECREF(fast);
 }
 

panda/src/pgraph/pythonLoaderFileType.cxx

@@ -89,26 +89,25 @@ init(PyObject *loader) {
       return false;
     }
 
-    PyObject *sequence = PySequence_Fast(extensions, "extensions must be a sequence");
-    PyObject **items = PySequence_Fast_ITEMS(sequence);
-    Py_ssize_t num_items = PySequence_Fast_GET_SIZE(sequence);
+    PyObject *tuple = PySequence_Tuple(extensions);
+    Py_ssize_t num_items = PyTuple_GET_SIZE(tuple);
     Py_DECREF(extensions);
 
     if (num_items == 0) {
       PyErr_SetString(PyExc_ValueError, "extensions list may not be empty");
-      Py_DECREF(sequence);
+      Py_DECREF(tuple);
       return false;
     }
 
     bool found_extension = false;
 
     for (Py_ssize_t i = 0; i < num_items; ++i) {
-      PyObject *extension = items[i];
+      PyObject *extension = PyTuple_GET_ITEM(tuple, i);
       const char *extension_str;
       Py_ssize_t extension_len;
       extension_str = PyUnicode_AsUTF8AndSize(extension, &extension_len);
       if (extension_str == nullptr) {
-        Py_DECREF(sequence);
+        Py_DECREF(tuple);
         return false;
       }
 
@@ -127,7 +126,7 @@ init(PyObject *loader) {
         }
       }
     }
-    Py_DECREF(sequence);
+    Py_DECREF(tuple);
 
     if (!found_extension) {
       PyObject *repr = PyObject_Repr(loader);

panda/src/pgraph/shaderInput_ext.cxx

@@ -269,12 +269,20 @@ __init__(CPT_InternalName name, PyObject *value, int priority) {
 
   } else if (PySequence_Check(value) && !PyUnicode_CheckExact(value)) {
     // Iterate over the sequence to make sure all have the same type.
+#ifdef Py_GIL_DISABLED
+    PyObject *fast = PySequence_Tuple(value);
+#else
     PyObject *fast = PySequence_Fast(value, "unknown type passed to ShaderInput");
+#endif
     if (fast == nullptr) {
       return;
     }
 
+#ifdef Py_GIL_DISABLED
+    Py_ssize_t num_items = PyTuple_GET_SIZE(fast);
+#else
     Py_ssize_t num_items = PySequence_Fast_GET_SIZE(fast);
+#endif
     if (num_items <= 0) {
       // We can't determine the type of a list of size 0.
       _this->_type = ShaderInput::M_numeric;
@@ -289,6 +297,9 @@ __init__(CPT_InternalName name, PyObject *value, int priority) {
     for (Py_ssize_t i = 0; i < num_items; ++i) {
       PyObject *item = items[i];
 
+      //FIXME: if these items are not tuples, this is not thread-safe in the
+      // free-threaded build.  Convert everything to tuples, and push to a
+      // vector?
       if (PySequence_Check(item)) {
         Py_ssize_t itemsize = PySequence_Size(item);
         if (known_itemsize >= 0 && itemsize != known_itemsize) {
@@ -310,7 +321,7 @@ __init__(CPT_InternalName name, PyObject *value, int priority) {
             Dtool_Raise_TypeError("unknown element type in sequence passed as element of sequence passed to ShaderInput");
             Py_DECREF(subitem);
             Py_DECREF(fast);
-            break;
+            return;
           }
           Py_DECREF(subitem);
         }

panda/src/putil/typedWritable_ext.cxx

@@ -349,17 +349,24 @@ find_global_decode(PyObject *this_class, const char *func_name) {
 
   PyObject *bases = PyObject_GetAttrString(this_class, "__bases__");
   if (bases != nullptr) {
-    if (PySequence_Check(bases)) {
-      Py_ssize_t size = PySequence_Size(bases);
-      for (Py_ssize_t i = 0; i < size; ++i) {
-        PyObject *base = PySequence_GetItem(bases, i);
-        if (base != nullptr) {
-          PyObject *func = find_global_decode(base, func_name);
-          Py_DECREF(base);
-          if (func != nullptr) {
-            Py_DECREF(bases);
-            return func;
-          }
+    {
+      PyObject *tuple = PySequence_Tuple(bases);
+      Py_DECREF(bases);
+      if (tuple == nullptr) {
+        PyErr_Clear();
+        return nullptr;
+      }
+      bases = tuple;
+    }
+
+    Py_ssize_t size = PyTuple_GET_SIZE(bases);
+    for (Py_ssize_t i = 0; i < size; ++i) {
+      PyObject *base = PyTuple_GET_ITEM(bases, i);
+      if (base != nullptr) {
+        PyObject *func = find_global_decode(base, func_name);
+        if (func != nullptr) {
+          Py_DECREF(bases);
+          return func;
         }
       }
     }