From bb72040396a6bbd1e55ad1ae27acd9ab5bac20bb Mon Sep 17 00:00:00 2001 From: John McCardle Date: Mon, 16 Feb 2026 20:58:09 -0500 Subject: [PATCH] Migrate static PyTypeObject to inline, delete PyTypeCache workarounds All 27 PyTypeObject declarations in namespace mcrfpydef headers changed from `static` to `inline` (C++17), ensuring a single global instance across translation units. This fixes the root cause of stale-type-pointer segfaults where only the McRFPy_API.cpp copy was PyType_Ready'd. Replaced ~20 PyTypeCache call sites and 2 PyRAII::PyTypeRef lookups with direct &mcrfpydef::Type references. Deleted PyTypeCache.h/.cpp, PyObjectUtils.h, and PyRAII.h (all were workarounds for the static bug). 228/228 tests pass. Co-Authored-By: Claude Opus 4.6 --- src/3d/Billboard.cpp | 12 +--- src/GridLayers.h | 4 +- src/McRFPy_API.cpp | 9 --- src/PyAnimation.h | 2 +- src/PyColor.cpp | 29 +++----- src/PyColor.h | 2 +- src/PyDiscreteMap.h | 2 +- src/PyFont.h | 2 +- src/PyHeightMap.h | 2 +- src/PyKeyboard.h | 2 +- src/PyMouse.h | 2 +- src/PyMusic.h | 2 +- src/PyObjectUtils.h | 76 -------------------- src/PyRAII.h | 138 ------------------------------------- src/PySceneObject.h | 2 +- src/PySound.h | 2 +- src/PyTexture.cpp | 16 +---- src/PyTexture.h | 2 +- src/PyTimer.h | 2 +- src/PyTypeCache.cpp | 135 ------------------------------------ src/PyTypeCache.h | 64 ----------------- src/PyVector.cpp | 28 ++++---- src/PyVector.h | 2 +- src/PyWindow.h | 2 +- src/UIArc.h | 2 +- src/UICaption.h | 2 +- src/UICircle.h | 2 +- src/UICollection.cpp | 1 - src/UIDrawable.h | 3 - src/UIEntity.cpp | 1 - src/UIEntity.h | 2 +- src/UIEntityCollection.cpp | 100 ++++++--------------------- src/UIFrame.h | 2 +- src/UIGrid.cpp | 14 +--- src/UIGrid.h | 2 +- src/UILine.h | 2 +- src/UISprite.h | 2 +- 37 files changed, 74 insertions(+), 600 deletions(-) delete mode 100644 src/PyObjectUtils.h delete mode 100644 src/PyRAII.h delete mode 100644 src/PyTypeCache.cpp delete mode 100644 src/PyTypeCache.h diff --git a/src/3d/Billboard.cpp b/src/3d/Billboard.cpp index 5937212..f91f96e 100644 --- a/src/3d/Billboard.cpp +++ b/src/3d/Billboard.cpp @@ -5,7 +5,6 @@ #include "MeshLayer.h" // For MeshVertex #include "../platform/GLContext.h" #include "../PyTexture.h" -#include "../PyTypeCache.h" #include #include @@ -355,8 +354,8 @@ int Billboard::init(PyObject* self, PyObject* args, PyObject* kwds) { // Handle texture if (textureObj && textureObj != Py_None) { - PyTypeObject* textureType = PyTypeCache::Texture(); - if (textureType && PyObject_IsInstance(textureObj, (PyObject*)textureType)) { + PyTypeObject* textureType = &mcrfpydef::PyTextureType; + if (PyObject_IsInstance(textureObj, (PyObject*)textureType)) { PyTextureObject* texPy = (PyTextureObject*)textureObj; if (texPy->data) { selfObj->data->setTexture(texPy->data); @@ -443,12 +442,7 @@ int Billboard::set_texture(PyObject* self, PyObject* value, void* closure) { obj->data->setTexture(nullptr); return 0; } - // Use PyTypeCache to get properly initialized type object - PyTypeObject* textureType = PyTypeCache::Texture(); - if (!textureType) { - PyErr_SetString(PyExc_RuntimeError, "Texture type not initialized"); - return -1; - } + PyTypeObject* textureType = &mcrfpydef::PyTextureType; if (PyObject_IsInstance(value, (PyObject*)textureType)) { PyTextureObject* texPy = (PyTextureObject*)value; if (texPy->data) { diff --git a/src/GridLayers.h b/src/GridLayers.h index 49109df..8ed414c 100644 --- a/src/GridLayers.h +++ b/src/GridLayers.h @@ -246,7 +246,7 @@ public: namespace mcrfpydef { // ColorLayer type - static PyTypeObject PyColorLayerType = { + inline PyTypeObject PyColorLayerType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.ColorLayer", .tp_basicsize = sizeof(PyColorLayerObject), @@ -299,7 +299,7 @@ namespace mcrfpydef { }; // TileLayer type - static PyTypeObject PyTileLayerType = { + inline PyTypeObject PyTileLayerType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.TileLayer", .tp_basicsize = sizeof(PyTileLayerObject), diff --git a/src/McRFPy_API.cpp b/src/McRFPy_API.cpp index 2b0abf2..d1e8d80 100644 --- a/src/McRFPy_API.cpp +++ b/src/McRFPy_API.cpp @@ -3,7 +3,6 @@ #include "McRFPy_Automation.h" // Note: McRFPy_Libtcod.h removed in #215 - functionality moved to mcrfpy.bresenham() #include "McRFPy_Doc.h" -#include "PyTypeCache.h" // Thread-safe cached Python types #include "platform.h" #include "PyAnimation.h" #include "PyDrawable.h" @@ -756,14 +755,6 @@ PyObject* PyInit_mcrfpy() // - line() functionality replaced by mcrfpy.bresenham() // - compute_fov() redundant with Grid.compute_fov() - // Initialize PyTypeCache for thread-safe type lookups - // This must be done after all types are added to the module - if (!PyTypeCache::initialize(m)) { - // Failed to initialize type cache - this is a critical error - // Error message already set by PyTypeCache::initialize - return NULL; - } - //McRFPy_API::mcrf_module = m; return m; } diff --git a/src/PyAnimation.h b/src/PyAnimation.h index cc38e95..b941d0a 100644 --- a/src/PyAnimation.h +++ b/src/PyAnimation.h @@ -38,7 +38,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyAnimationType = { + inline PyTypeObject PyAnimationType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Animation", .tp_basicsize = sizeof(PyAnimationObject), diff --git a/src/PyColor.cpp b/src/PyColor.cpp index 7bc8b10..9a284e0 100644 --- a/src/PyColor.cpp +++ b/src/PyColor.cpp @@ -1,7 +1,5 @@ #include "PyColor.h" #include "McRFPy_API.h" -#include "PyObjectUtils.h" -#include "PyRAII.h" #include "McRFPy_Doc.h" #include #include @@ -288,33 +286,28 @@ int PyColor::set_member(PyObject* obj, PyObject* value, void* closure) PyColorObject* PyColor::from_arg(PyObject* args) { - // Use RAII for type reference management - PyRAII::PyTypeRef type("Color", McRFPy_API::mcrf_module); - if (!type) { - return NULL; - } - + PyTypeObject* type = &mcrfpydef::PyColorType; + // Check if args is already a Color instance - if (PyObject_IsInstance(args, (PyObject*)type.get())) { + if (PyObject_IsInstance(args, (PyObject*)type)) { Py_INCREF(args); // Return new reference so caller can safely DECREF return (PyColorObject*)args; } - - // Create new Color object using RAII - PyRAII::PyObjectRef obj(type->tp_alloc(type.get(), 0), true); + + // Create new Color object + PyObject* obj = type->tp_alloc(type, 0); if (!obj) { return NULL; } - + // Initialize the object - int err = init((PyColorObject*)obj.get(), args, NULL); + int err = init((PyColorObject*)obj, args, NULL); if (err) { - // obj will be automatically cleaned up when it goes out of scope + Py_DECREF(obj); return NULL; } - - // Release ownership and return - return (PyColorObject*)obj.release(); + + return (PyColorObject*)obj; } // Color helper method implementations diff --git a/src/PyColor.h b/src/PyColor.h index 0ade897..601090c 100644 --- a/src/PyColor.h +++ b/src/PyColor.h @@ -39,7 +39,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyColorType = { + inline PyTypeObject PyColorType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Color", .tp_basicsize = sizeof(PyColorObject), diff --git a/src/PyDiscreteMap.h b/src/PyDiscreteMap.h index c6b70b5..3541f2b 100644 --- a/src/PyDiscreteMap.h +++ b/src/PyDiscreteMap.h @@ -77,7 +77,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyDiscreteMapType = { + inline PyTypeObject PyDiscreteMapType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.DiscreteMap", .tp_basicsize = sizeof(PyDiscreteMapObject), diff --git a/src/PyFont.h b/src/PyFont.h index df88423..f567717 100644 --- a/src/PyFont.h +++ b/src/PyFont.h @@ -30,7 +30,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyFontType = { + inline PyTypeObject PyFontType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Font", .tp_basicsize = sizeof(PyFontObject), diff --git a/src/PyHeightMap.h b/src/PyHeightMap.h index 0e53fa3..ce83ade 100644 --- a/src/PyHeightMap.h +++ b/src/PyHeightMap.h @@ -88,7 +88,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyHeightMapType = { + inline PyTypeObject PyHeightMapType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.HeightMap", .tp_basicsize = sizeof(PyHeightMapObject), diff --git a/src/PyKeyboard.h b/src/PyKeyboard.h index aa749f7..ce273ae 100644 --- a/src/PyKeyboard.h +++ b/src/PyKeyboard.h @@ -21,7 +21,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyKeyboardType = { + inline PyTypeObject PyKeyboardType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Keyboard", .tp_basicsize = sizeof(PyKeyboardObject), diff --git a/src/PyMouse.h b/src/PyMouse.h index 18aed50..370282b 100644 --- a/src/PyMouse.h +++ b/src/PyMouse.h @@ -35,7 +35,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyMouseType = { + inline PyTypeObject PyMouseType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Mouse", .tp_basicsize = sizeof(PyMouseObject), diff --git a/src/PyMusic.h b/src/PyMusic.h index 65b3873..48a0fb3 100644 --- a/src/PyMusic.h +++ b/src/PyMusic.h @@ -62,7 +62,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyMusicType = { + inline PyTypeObject PyMusicType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Music", .tp_basicsize = sizeof(PyMusicObject), diff --git a/src/PyObjectUtils.h b/src/PyObjectUtils.h deleted file mode 100644 index e3ff840..0000000 --- a/src/PyObjectUtils.h +++ /dev/null @@ -1,76 +0,0 @@ -#pragma once -#include "Common.h" -#include "Python.h" -#include "McRFPy_API.h" -#include "PyRAII.h" - -namespace PyObjectUtils { - - // Template for getting Python type object from module - template - PyTypeObject* getPythonType(const char* typeName) { - PyTypeObject* type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, typeName); - if (!type) { - PyErr_Format(PyExc_RuntimeError, "Could not find %s type in module", typeName); - } - return type; - } - - // Generic function to create a Python object of given type - inline PyObject* createPyObjectGeneric(const char* typeName) { - PyTypeObject* type = getPythonType(typeName); - if (!type) return nullptr; - - PyObject* obj = type->tp_alloc(type, 0); - Py_DECREF(type); - - return obj; - } - - // Helper function to allocate and initialize a Python object with data - template - PyObject* createPyObjectWithData(const char* typeName, DataType data) { - PyTypeObject* type = getPythonType(typeName); - if (!type) return nullptr; - - PyObjType* obj = (PyObjType*)type->tp_alloc(type, 0); - Py_DECREF(type); - - if (obj) { - obj->data = data; - } - return (PyObject*)obj; - } - - // Function to convert UIDrawable to appropriate Python object - // This is moved to UICollection.cpp to avoid circular dependencies - - // RAII-based object creation example - inline PyObject* createPyObjectGenericRAII(const char* typeName) { - PyRAII::PyTypeRef type(typeName, McRFPy_API::mcrf_module); - if (!type) { - PyErr_Format(PyExc_RuntimeError, "Could not find %s type in module", typeName); - return nullptr; - } - - PyObject* obj = type->tp_alloc(type.get(), 0); - // Return the new reference (caller owns it) - return obj; - } - - // Example of using PyObjectRef for safer reference management - template - PyObject* createPyObjectWithDataRAII(const char* typeName, DataType data) { - PyRAII::PyObjectRef obj = PyRAII::createObject(typeName, McRFPy_API::mcrf_module); - if (!obj) { - PyErr_Format(PyExc_RuntimeError, "Could not create %s object", typeName); - return nullptr; - } - - // Access the object through the RAII wrapper - ((PyObjType*)obj.get())->data = data; - - // Release ownership to return to Python - return obj.release(); - } -} \ No newline at end of file diff --git a/src/PyRAII.h b/src/PyRAII.h deleted file mode 100644 index 2e5e7b3..0000000 --- a/src/PyRAII.h +++ /dev/null @@ -1,138 +0,0 @@ -#pragma once -#include "Python.h" -#include - -namespace PyRAII { - - // RAII wrapper for PyObject* that automatically manages reference counting - class PyObjectRef { - private: - PyObject* ptr; - - public: - // Constructors - PyObjectRef() : ptr(nullptr) {} - - explicit PyObjectRef(PyObject* p, bool steal_ref = false) : ptr(p) { - if (ptr && !steal_ref) { - Py_INCREF(ptr); - } - } - - // Copy constructor - PyObjectRef(const PyObjectRef& other) : ptr(other.ptr) { - if (ptr) { - Py_INCREF(ptr); - } - } - - // Move constructor - PyObjectRef(PyObjectRef&& other) noexcept : ptr(other.ptr) { - other.ptr = nullptr; - } - - // Destructor - ~PyObjectRef() { - Py_XDECREF(ptr); - } - - // Copy assignment - PyObjectRef& operator=(const PyObjectRef& other) { - if (this != &other) { - Py_XDECREF(ptr); - ptr = other.ptr; - if (ptr) { - Py_INCREF(ptr); - } - } - return *this; - } - - // Move assignment - PyObjectRef& operator=(PyObjectRef&& other) noexcept { - if (this != &other) { - Py_XDECREF(ptr); - ptr = other.ptr; - other.ptr = nullptr; - } - return *this; - } - - // Access operators - PyObject* get() const { return ptr; } - PyObject* operator->() const { return ptr; } - PyObject& operator*() const { return *ptr; } - operator bool() const { return ptr != nullptr; } - - // Release ownership (for returning to Python) - PyObject* release() { - PyObject* temp = ptr; - ptr = nullptr; - return temp; - } - - // Reset with new pointer - void reset(PyObject* p = nullptr, bool steal_ref = false) { - if (p != ptr) { - Py_XDECREF(ptr); - ptr = p; - if (ptr && !steal_ref) { - Py_INCREF(ptr); - } - } - } - }; - - // Helper class for managing PyTypeObject* references from module lookups - class PyTypeRef { - private: - PyTypeObject* type; - - public: - PyTypeRef() : type(nullptr) {} - - explicit PyTypeRef(const char* typeName, PyObject* module) { - type = (PyTypeObject*)PyObject_GetAttrString(module, typeName); - // GetAttrString returns a new reference, so we own it - } - - ~PyTypeRef() { - Py_XDECREF((PyObject*)type); - } - - // Delete copy operations to prevent accidental reference issues - PyTypeRef(const PyTypeRef&) = delete; - PyTypeRef& operator=(const PyTypeRef&) = delete; - - // Allow move operations - PyTypeRef(PyTypeRef&& other) noexcept : type(other.type) { - other.type = nullptr; - } - - PyTypeRef& operator=(PyTypeRef&& other) noexcept { - if (this != &other) { - Py_XDECREF((PyObject*)type); - type = other.type; - other.type = nullptr; - } - return *this; - } - - PyTypeObject* get() const { return type; } - PyTypeObject* operator->() const { return type; } - operator bool() const { return type != nullptr; } - }; - - // Convenience function to create a new object with RAII - template - PyObjectRef createObject(const char* typeName, PyObject* module) { - PyTypeRef type(typeName, module); - if (!type) { - return PyObjectRef(); - } - - PyObject* obj = type->tp_alloc(type.get(), 0); - // tp_alloc returns a new reference, so we steal it - return PyObjectRef(obj, true); - } -} \ No newline at end of file diff --git a/src/PySceneObject.h b/src/PySceneObject.h index 487a656..0057120 100644 --- a/src/PySceneObject.h +++ b/src/PySceneObject.h @@ -47,7 +47,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PySceneType = { + inline PyTypeObject PySceneType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Scene", .tp_basicsize = sizeof(PySceneObject), diff --git a/src/PySound.h b/src/PySound.h index 12ba78a..b24a605 100644 --- a/src/PySound.h +++ b/src/PySound.h @@ -59,7 +59,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PySoundType = { + inline PyTypeObject PySoundType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Sound", .tp_basicsize = sizeof(PySoundObject), diff --git a/src/PyTexture.cpp b/src/PyTexture.cpp index 29c1991..39ccc37 100644 --- a/src/PyTexture.cpp +++ b/src/PyTexture.cpp @@ -1,7 +1,6 @@ #include "PyTexture.h" #include "McRFPy_API.h" #include "McRFPy_Doc.h" -#include "PyTypeCache.h" #include #include @@ -93,13 +92,8 @@ sf::Sprite PyTexture::sprite(int index, sf::Vector2f pos, sf::Vector2f s) PyObject* PyTexture::pyObject() { - PyTypeObject* type = PyTypeCache::Texture(); - if (!type) { - PyErr_SetString(PyExc_RuntimeError, "Failed to get Texture type from cache"); - return NULL; - } + PyTypeObject* type = &mcrfpydef::PyTextureType; PyObject* obj = PyTexture::pynew(type, Py_None, Py_None); - // PyTypeCache returns borrowed reference — no DECREF needed if (!obj) { return NULL; @@ -275,12 +269,7 @@ PyObject* PyTexture::composite(PyObject* cls, PyObject* args, PyObject* kwds) std::vector images; unsigned int tex_w = 0, tex_h = 0; - // Use PyTypeCache for reliable, leak-free isinstance check - PyTypeObject* texture_type = PyTypeCache::Texture(); - if (!texture_type) { - PyErr_SetString(PyExc_RuntimeError, "Failed to get Texture type from cache"); - return NULL; - } + PyTypeObject* texture_type = &mcrfpydef::PyTextureType; for (Py_ssize_t i = 0; i < count; i++) { PyObject* item = PyList_GetItem(layers_list, i); @@ -311,7 +300,6 @@ PyObject* PyTexture::composite(PyObject* cls, PyObject* args, PyObject* kwds) } images.push_back(std::move(img)); } - // PyTypeCache returns borrowed reference — no DECREF needed // Alpha-composite all layers bottom-to-top sf::Image result; diff --git a/src/PyTexture.h b/src/PyTexture.h index ff41373..dcadcb8 100644 --- a/src/PyTexture.h +++ b/src/PyTexture.h @@ -60,7 +60,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyTextureType = { + inline PyTypeObject PyTextureType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Texture", .tp_basicsize = sizeof(PyTextureObject), diff --git a/src/PyTimer.h b/src/PyTimer.h index 438b83a..c1e8b18 100644 --- a/src/PyTimer.h +++ b/src/PyTimer.h @@ -48,7 +48,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyTimerType = { + inline PyTypeObject PyTimerType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Timer", .tp_basicsize = sizeof(PyTimerObject), diff --git a/src/PyTypeCache.cpp b/src/PyTypeCache.cpp deleted file mode 100644 index 22d18a9..0000000 --- a/src/PyTypeCache.cpp +++ /dev/null @@ -1,135 +0,0 @@ -// PyTypeCache.cpp - Thread-safe Python type caching implementation -#include "PyTypeCache.h" - -// Static member definitions -std::atomic PyTypeCache::entity_type{nullptr}; -std::atomic PyTypeCache::grid_type{nullptr}; -std::atomic PyTypeCache::frame_type{nullptr}; -std::atomic PyTypeCache::caption_type{nullptr}; -std::atomic PyTypeCache::sprite_type{nullptr}; -std::atomic PyTypeCache::texture_type{nullptr}; -std::atomic PyTypeCache::color_type{nullptr}; -std::atomic PyTypeCache::vector_type{nullptr}; -std::atomic PyTypeCache::font_type{nullptr}; -std::atomic PyTypeCache::initialized{false}; -std::mutex PyTypeCache::init_mutex; - -PyTypeObject* PyTypeCache::cacheType(PyObject* module, const char* name, - std::atomic& cache) { - PyObject* type_obj = PyObject_GetAttrString(module, name); - if (!type_obj) { - PyErr_Format(PyExc_RuntimeError, - "PyTypeCache: Failed to get type '%s' from module", name); - return nullptr; - } - - if (!PyType_Check(type_obj)) { - Py_DECREF(type_obj); - PyErr_Format(PyExc_TypeError, - "PyTypeCache: '%s' is not a type object", name); - return nullptr; - } - - // Store in cache - we keep the reference permanently - // Using memory_order_release ensures the pointer is visible to other threads - // after they see initialized=true - cache.store((PyTypeObject*)type_obj, std::memory_order_release); - - return (PyTypeObject*)type_obj; -} - -bool PyTypeCache::initialize(PyObject* module) { - std::lock_guard lock(init_mutex); - - // Double-check pattern - might have been initialized while waiting for lock - if (initialized.load(std::memory_order_acquire)) { - return true; - } - - // Cache all types - if (!cacheType(module, "Entity", entity_type)) return false; - if (!cacheType(module, "Grid", grid_type)) return false; - if (!cacheType(module, "Frame", frame_type)) return false; - if (!cacheType(module, "Caption", caption_type)) return false; - if (!cacheType(module, "Sprite", sprite_type)) return false; - if (!cacheType(module, "Texture", texture_type)) return false; - if (!cacheType(module, "Color", color_type)) return false; - if (!cacheType(module, "Vector", vector_type)) return false; - if (!cacheType(module, "Font", font_type)) return false; - - // Mark as initialized - release ensures all stores above are visible - initialized.store(true, std::memory_order_release); - - return true; -} - -void PyTypeCache::finalize() { - std::lock_guard lock(init_mutex); - - if (!initialized.load(std::memory_order_acquire)) { - return; - } - - // Release all cached references - auto release = [](std::atomic& cache) { - PyTypeObject* type = cache.exchange(nullptr, std::memory_order_acq_rel); - if (type) { - Py_DECREF(type); - } - }; - - release(entity_type); - release(grid_type); - release(frame_type); - release(caption_type); - release(sprite_type); - release(texture_type); - release(color_type); - release(vector_type); - release(font_type); - - initialized.store(false, std::memory_order_release); -} - -bool PyTypeCache::isInitialized() { - return initialized.load(std::memory_order_acquire); -} - -// Type accessors - lock-free reads after initialization -// Using memory_order_acquire ensures we see the pointer stored during init - -PyTypeObject* PyTypeCache::Entity() { - return entity_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Grid() { - return grid_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Frame() { - return frame_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Caption() { - return caption_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Sprite() { - return sprite_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Texture() { - return texture_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Color() { - return color_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Vector() { - return vector_type.load(std::memory_order_acquire); -} - -PyTypeObject* PyTypeCache::Font() { - return font_type.load(std::memory_order_acquire); -} diff --git a/src/PyTypeCache.h b/src/PyTypeCache.h deleted file mode 100644 index 633cb54..0000000 --- a/src/PyTypeCache.h +++ /dev/null @@ -1,64 +0,0 @@ -#pragma once -// PyTypeCache.h - Thread-safe caching of Python type objects -// -// This module provides a centralized, thread-safe way to cache Python type -// references. It eliminates the refcount leaks from repeated -// PyObject_GetAttrString calls and is compatible with free-threading (PEP 703). -// -// Usage: -// PyTypeObject* entity_type = PyTypeCache::Entity(); -// if (!entity_type) return NULL; // Error already set -// -// The cache is populated during module initialization and the types are -// held for the lifetime of the interpreter. - -#include "Python.h" -#include -#include - -class PyTypeCache { -public: - // Initialize the cache - call once during module init after types are ready - // Returns false and sets Python error on failure - static bool initialize(PyObject* module); - - // Finalize - release references (call during module cleanup if needed) - static void finalize(); - - // Type accessors - return borrowed references (no DECREF needed) - // These are thread-safe and lock-free after initialization - static PyTypeObject* Entity(); - static PyTypeObject* Grid(); - static PyTypeObject* Frame(); - static PyTypeObject* Caption(); - static PyTypeObject* Sprite(); - static PyTypeObject* Texture(); - static PyTypeObject* Color(); - static PyTypeObject* Vector(); - static PyTypeObject* Font(); - - // Check if initialized - static bool isInitialized(); - -private: - // Cached type pointers - atomic for thread-safe reads - static std::atomic entity_type; - static std::atomic grid_type; - static std::atomic frame_type; - static std::atomic caption_type; - static std::atomic sprite_type; - static std::atomic texture_type; - static std::atomic color_type; - static std::atomic vector_type; - static std::atomic font_type; - - // Initialization flag - static std::atomic initialized; - - // Mutex for initialization (only used during init, not for reads) - static std::mutex init_mutex; - - // Helper to fetch and cache a type - static PyTypeObject* cacheType(PyObject* module, const char* name, - std::atomic& cache); -}; diff --git a/src/PyVector.cpp b/src/PyVector.cpp index 391d141..8967d93 100644 --- a/src/PyVector.cpp +++ b/src/PyVector.cpp @@ -1,7 +1,5 @@ #include "PyVector.h" -#include "PyObjectUtils.h" #include "McRFPy_Doc.h" -#include "PyRAII.h" #include PyGetSetDef PyVector::getsetters[] = { @@ -262,20 +260,16 @@ int PyVector::set_member(PyObject* obj, PyObject* value, void* closure) PyVectorObject* PyVector::from_arg(PyObject* args) { - // Use RAII for type reference management - PyRAII::PyTypeRef type("Vector", McRFPy_API::mcrf_module); - if (!type) { - return NULL; - } + PyTypeObject* type = &mcrfpydef::PyVectorType; // Check if args is already a Vector instance - if (PyObject_IsInstance(args, (PyObject*)type.get())) { + if (PyObject_IsInstance(args, (PyObject*)type)) { Py_INCREF(args); // Return new reference so caller can safely DECREF return (PyVectorObject*)args; } - // Create new Vector object using RAII - PyRAII::PyObjectRef obj(type->tp_alloc(type.get(), 0), true); + // Create new Vector object + PyObject* obj = type->tp_alloc(type, 0); if (!obj) { return NULL; } @@ -283,25 +277,27 @@ PyVectorObject* PyVector::from_arg(PyObject* args) // Handle different input types if (PyTuple_Check(args)) { // It's already a tuple, pass it directly to init - int err = init((PyVectorObject*)obj.get(), args, NULL); + int err = init((PyVectorObject*)obj, args, NULL); if (err) { - // obj will be automatically cleaned up when it goes out of scope + Py_DECREF(obj); return NULL; } } else { // Wrap single argument in a tuple for init - PyRAII::PyObjectRef tuple(PyTuple_Pack(1, args), true); + PyObject* tuple = PyTuple_Pack(1, args); if (!tuple) { + Py_DECREF(obj); return NULL; } - int err = init((PyVectorObject*)obj.get(), tuple.get(), NULL); + int err = init((PyVectorObject*)obj, tuple, NULL); + Py_DECREF(tuple); if (err) { + Py_DECREF(obj); return NULL; } } - // Release ownership and return - return (PyVectorObject*)obj.release(); + return (PyVectorObject*)obj; } // Arithmetic operations diff --git a/src/PyVector.h b/src/PyVector.h index 42c3ee3..300c611 100644 --- a/src/PyVector.h +++ b/src/PyVector.h @@ -63,7 +63,7 @@ namespace mcrfpydef { extern PyNumberMethods PyVector_as_number; extern PySequenceMethods PyVector_as_sequence; - static PyTypeObject PyVectorType = { + inline PyTypeObject PyVectorType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Vector", .tp_basicsize = sizeof(PyVectorObject), diff --git a/src/PyWindow.h b/src/PyWindow.h index ad69a83..09a4852 100644 --- a/src/PyWindow.h +++ b/src/PyWindow.h @@ -47,7 +47,7 @@ public: }; namespace mcrfpydef { - static PyTypeObject PyWindowType = { + inline PyTypeObject PyWindowType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Window", .tp_basicsize = sizeof(PyWindowObject), diff --git a/src/UIArc.h b/src/UIArc.h index 74019cb..b6061ca 100644 --- a/src/UIArc.h +++ b/src/UIArc.h @@ -111,7 +111,7 @@ public: extern PyMethodDef UIArc_methods[]; namespace mcrfpydef { - static PyTypeObject PyUIArcType = { + inline PyTypeObject PyUIArcType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Arc", .tp_basicsize = sizeof(PyUIArcObject), diff --git a/src/UICaption.h b/src/UICaption.h index b66c847..537156b 100644 --- a/src/UICaption.h +++ b/src/UICaption.h @@ -50,7 +50,7 @@ public: extern PyMethodDef UICaption_methods[]; namespace mcrfpydef { - static PyTypeObject PyUICaptionType = { + inline PyTypeObject PyUICaptionType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Caption", .tp_basicsize = sizeof(PyUICaptionObject), diff --git a/src/UICircle.h b/src/UICircle.h index 966b6f3..1f7b58a 100644 --- a/src/UICircle.h +++ b/src/UICircle.h @@ -100,7 +100,7 @@ public: extern PyMethodDef UICircle_methods[]; namespace mcrfpydef { - static PyTypeObject PyUICircleType = { + inline PyTypeObject PyUICircleType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Circle", .tp_basicsize = sizeof(PyUICircleObject), diff --git a/src/UICollection.cpp b/src/UICollection.cpp index e02380b..383646c 100644 --- a/src/UICollection.cpp +++ b/src/UICollection.cpp @@ -9,7 +9,6 @@ #include "UIArc.h" #include "3d/Viewport3D.h" #include "McRFPy_API.h" -#include "PyObjectUtils.h" #include "PythonObjectCache.h" #include #include diff --git a/src/UIDrawable.h b/src/UIDrawable.h index fbddba8..3d2b7ad 100644 --- a/src/UIDrawable.h +++ b/src/UIDrawable.h @@ -339,9 +339,6 @@ typedef struct { } PyUICollectionIterObject; namespace mcrfpydef { - // DEPRECATED: RET_PY_INSTANCE macro has been replaced with template functions in PyObjectUtils.h - // The macro was difficult to debug and used static type references that could cause initialization order issues. - // Use PyObjectUtils::convertDrawableToPython() or PyObjectUtils::createPyObject() instead. //TODO: add this method to class scope; move implementation to .cpp file /* diff --git a/src/UIEntity.cpp b/src/UIEntity.cpp index 196b6b1..eeb8896 100644 --- a/src/UIEntity.cpp +++ b/src/UIEntity.cpp @@ -4,7 +4,6 @@ #include #include #include -#include "PyObjectUtils.h" #include "PyVector.h" #include "PythonObjectCache.h" #include "PyFOV.h" diff --git a/src/UIEntity.h b/src/UIEntity.h index 37ca504..1eb508c 100644 --- a/src/UIEntity.h +++ b/src/UIEntity.h @@ -124,7 +124,7 @@ public: extern PyMethodDef UIEntity_all_methods[]; namespace mcrfpydef { - static PyTypeObject PyUIEntityType = { + inline PyTypeObject PyUIEntityType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Entity", .tp_basicsize = sizeof(PyUIEntityObject), diff --git a/src/UIEntityCollection.cpp b/src/UIEntityCollection.cpp index a51d1b7..cf49e9c 100644 --- a/src/UIEntityCollection.cpp +++ b/src/UIEntityCollection.cpp @@ -7,7 +7,6 @@ #include "UIEntity.h" #include "UIGrid.h" #include "McRFPy_API.h" -#include "PyTypeCache.h" #include "PythonObjectCache.h" #include #include @@ -49,11 +48,7 @@ PyObject* UIEntityCollectionIter::next(PyUIEntityCollectionIterObject* self) } // Otherwise create and return a new Python Entity object - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; auto o = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0); if (!o) return NULL; @@ -119,11 +114,7 @@ PyObject* UIEntityCollection::getitem(PyUIEntityCollectionObject* self, Py_ssize } // Otherwise, create a new base Entity object - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; auto o = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0); if (!o) return NULL; @@ -174,12 +165,8 @@ int UIEntityCollection::setitem(PyUIEntityCollectionObject* self, Py_ssize_t ind return 0; } - // Type checking using cached type - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return -1; - } + // Type checking + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; if (!PyObject_IsInstance(value, (PyObject*)entity_type)) { PyErr_SetString(PyExc_TypeError, "EntityCollection can only contain Entity objects"); @@ -219,9 +206,9 @@ int UIEntityCollection::contains(PyUIEntityCollectionObject* self, PyObject* val return -1; } - // Type checking using cached type - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type || !PyObject_IsInstance(value, (PyObject*)entity_type)) { + // Type checking + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; + if (!PyObject_IsInstance(value, (PyObject*)entity_type)) { return 0; // Not an Entity, can't be in collection } @@ -257,12 +244,7 @@ PyObject* UIEntityCollection::concat(PyUIEntityCollectionObject* self, PyObject* return NULL; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - Py_DECREF(result_list); - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; // Add all elements from self Py_ssize_t idx = 0; @@ -296,11 +278,7 @@ PyObject* UIEntityCollection::inplace_concat(PyUIEntityCollectionObject* self, P return NULL; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; // First, validate ALL items before modifying anything Py_ssize_t other_len = PySequence_Length(other); @@ -380,12 +358,7 @@ PyObject* UIEntityCollection::subscript(PyUIEntityCollectionObject* self, PyObje return NULL; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - Py_DECREF(result_list); - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; auto it = self->data->begin(); for (Py_ssize_t i = 0, cur = start; i < slicelength; i++, cur += step) { @@ -474,11 +447,7 @@ int UIEntityCollection::ass_subscript(PyUIEntityCollectionObject* self, PyObject return -1; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return -1; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; // Validate all items first std::vector> new_items; @@ -577,11 +546,7 @@ PyMappingMethods UIEntityCollection::mpmethods = { PyObject* UIEntityCollection::append(PyUIEntityCollectionObject* self, PyObject* o) { - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; if (!PyObject_IsInstance(o, (PyObject*)entity_type)) { PyErr_SetString(PyExc_TypeError, "Only Entity objects can be added to EntityCollection"); @@ -627,11 +592,7 @@ PyObject* UIEntityCollection::append(PyUIEntityCollectionObject* self, PyObject* PyObject* UIEntityCollection::remove(PyUIEntityCollectionObject* self, PyObject* o) { - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; if (!PyObject_IsInstance(o, (PyObject*)entity_type)) { PyErr_SetString(PyExc_TypeError, "EntityCollection.remove requires an Entity object"); @@ -675,12 +636,7 @@ PyObject* UIEntityCollection::extend(PyUIEntityCollectionObject* self, PyObject* return NULL; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - Py_DECREF(iterator); - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; // FIXED: Validate ALL items first before modifying anything // (Following the pattern from inplace_concat) @@ -785,11 +741,7 @@ PyObject* UIEntityCollection::pop(PyUIEntityCollectionObject* self, PyObject* ar list->erase(it); // Create Python object for the entity - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; PyUIEntityObject* py_entity = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0); if (!py_entity) { @@ -817,11 +769,7 @@ PyObject* UIEntityCollection::insert(PyUIEntityCollectionObject* self, PyObject* return NULL; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; if (!PyObject_IsInstance(o, (PyObject*)entity_type)) { PyErr_SetString(PyExc_TypeError, "EntityCollection.insert requires an Entity object"); @@ -874,11 +822,7 @@ PyObject* UIEntityCollection::index_method(PyUIEntityCollectionObject* self, PyO return NULL; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; if (!PyObject_IsInstance(value, (PyObject*)entity_type)) { PyErr_SetString(PyExc_TypeError, "EntityCollection.index requires an Entity object"); @@ -910,8 +854,8 @@ PyObject* UIEntityCollection::count(PyUIEntityCollectionObject* self, PyObject* return NULL; } - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type || !PyObject_IsInstance(value, (PyObject*)entity_type)) { + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; + if (!PyObject_IsInstance(value, (PyObject*)entity_type)) { return PyLong_FromLong(0); } @@ -969,11 +913,7 @@ PyObject* UIEntityCollection::find(PyUIEntityCollectionObject* self, PyObject* a std::string pattern(name); bool has_wildcard = (pattern.find('*') != std::string::npos); - PyTypeObject* entity_type = PyTypeCache::Entity(); - if (!entity_type) { - PyErr_SetString(PyExc_RuntimeError, "Entity type not initialized in cache"); - return NULL; - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; if (has_wildcard) { PyObject* results = PyList_New(0); diff --git a/src/UIFrame.h b/src/UIFrame.h index 1e753b4..93caf39 100644 --- a/src/UIFrame.h +++ b/src/UIFrame.h @@ -76,7 +76,7 @@ public: extern PyMethodDef UIFrame_methods[]; namespace mcrfpydef { - static PyTypeObject PyUIFrameType = { + inline PyTypeObject PyUIFrameType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Frame", .tp_basicsize = sizeof(PyUIFrameObject), diff --git a/src/UIGrid.cpp b/src/UIGrid.cpp index fbd38e9..06d5a37 100644 --- a/src/UIGrid.cpp +++ b/src/UIGrid.cpp @@ -4,7 +4,6 @@ #include "McRFPy_API.h" #include "PythonObjectCache.h" #include "PyAlignment.h" -#include "PyTypeCache.h" // Thread-safe cached Python types #include "UIEntity.h" #include "Profiler.h" #include "PyFOV.h" @@ -1928,16 +1927,7 @@ PyObject* UIGrid::py_entities_in_radius(PyUIGridObject* self, PyObject* args, Py PyObject* result = PyList_New(entities.size()); if (!result) return PyErr_NoMemory(); - // Cache Entity type for efficiency - static PyTypeObject* cached_entity_type = nullptr; - if (!cached_entity_type) { - cached_entity_type = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Entity"); - if (!cached_entity_type) { - Py_DECREF(result); - return NULL; - } - Py_INCREF(cached_entity_type); - } + PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType; for (size_t i = 0; i < entities.size(); i++) { auto& entity = entities[i]; @@ -1948,7 +1938,7 @@ PyObject* UIGrid::py_entities_in_radius(PyUIGridObject* self, PyObject* args, Py PyList_SET_ITEM(result, i, entity->self); } else { // Create new Python Entity wrapper - auto pyEntity = (PyUIEntityObject*)cached_entity_type->tp_alloc(cached_entity_type, 0); + auto pyEntity = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0); if (!pyEntity) { Py_DECREF(result); return PyErr_NoMemory(); diff --git a/src/UIGrid.h b/src/UIGrid.h index 318e71c..f80e130 100644 --- a/src/UIGrid.h +++ b/src/UIGrid.h @@ -255,7 +255,7 @@ public: extern PyMethodDef UIGrid_all_methods[]; namespace mcrfpydef { - static PyTypeObject PyUIGridType = { + inline PyTypeObject PyUIGridType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Grid", .tp_basicsize = sizeof(PyUIGridObject), diff --git a/src/UILine.h b/src/UILine.h index af85b6c..43e976a 100644 --- a/src/UILine.h +++ b/src/UILine.h @@ -97,7 +97,7 @@ public: extern PyMethodDef UILine_methods[]; namespace mcrfpydef { - static PyTypeObject PyUILineType = { + inline PyTypeObject PyUILineType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Line", .tp_basicsize = sizeof(PyUILineObject), diff --git a/src/UISprite.h b/src/UISprite.h index f409a23..d96f271 100644 --- a/src/UISprite.h +++ b/src/UISprite.h @@ -83,7 +83,7 @@ public: extern PyMethodDef UISprite_methods[]; namespace mcrfpydef { - static PyTypeObject PyUISpriteType = { + inline PyTypeObject PyUISpriteType = { .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0}, .tp_name = "mcrfpy.Sprite", .tp_basicsize = sizeof(PyUISpriteObject),