Merge pull request #220 from robotpy/safe-object

Add gilsafe_object to simplify storing python objects in containers

Author: virtuald

robotpy_build/include/gilsafe_object.h

@@ -0,0 +1,141 @@
+
+#pragma once
+
+#include <pybind11/pybind11.h>
+
+namespace py = pybind11;
+
+// Py_IsFinalizing is public API in 3.13
+#if PY_VERSION_HEX < 0x03130000
+#define Py_IsFinalizing _Py_IsFinalizing
+#endif
+
+namespace rpy {
+
+/*
+    This object holds a python object, and can be stored in C++ containers that
+    aren't pybind11 aware.
+
+    It is very inefficient -- it will acquire and release the GIL each time
+    a move/copy operation occurs! You should only use this object type as a
+    last resort.
+
+    Assigning, moves, copies, and destruction acquire the GIL; only converting
+    this back into a python object requires holding the GIL.
+*/
+template <typename T>
+class gilsafe_t final {
+    py::object o;
+
+public:
+
+    //
+    // These operations require the caller to hold the GIL
+    //
+
+    // copy conversion
+    operator py::object() const & {
+        return o;
+    }
+
+    // move conversion
+    operator py::object() const && {
+        return std::move(o);
+    }
+
+    //
+    // These operations do not require the caller to hold the GIL
+    //
+
+    gilsafe_t() = default;
+
+    ~gilsafe_t() {
+        if (o) {
+            // If the interpreter is alive, acquire the GIL, otherwise just leak
+            // the object to avoid a crash
+            if (!Py_IsFinalizing()) {
+                py::gil_scoped_acquire lock;
+                o.dec_ref();
+            }
+
+            o.release();
+        }
+    }
+
+    // Copy constructor; always increases the reference count
+    gilsafe_t(const gilsafe_t &other) {
+        py::gil_scoped_acquire lock;
+        o = other.o;
+    }
+
+    // Copy constructor; always increases the reference count
+    gilsafe_t(const py::object &other) {
+        py::gil_scoped_acquire lock;
+        o = other;
+    }
+
+    gilsafe_t(const py::handle &other) {
+        py::gil_scoped_acquire lock;
+        o = py::reinterpret_borrow<py::object>(other);
+    }
+
+    // Move constructor; steals object from ``other`` and preserves its reference count
+    gilsafe_t(gilsafe_t &&other) noexcept : o(std::move(other.o)) {}
+
+    // Move constructor; steals object from ``other`` and preserves its reference count
+    gilsafe_t(py::object &&other)  noexcept : o(std::move(other)) {}
+
+    // copy assignment
+    gilsafe_t &operator=(const gilsafe_t& other) {
+        if (!o.is(other.o)) {
+            py::gil_scoped_acquire lock;
+            o = other.o;
+        }
+        return *this;
+    }
+
+    // move assignment
+    gilsafe_t &operator=(gilsafe_t&& other) noexcept {
+        if (this != &other) {
+            py::gil_scoped_acquire lock;
+            o = std::move(other.o);
+        }
+        return *this;
+    }
+
+    explicit operator bool() const {
+        return (bool)o;
+    }
+};
+
+// convenience alias
+using gilsafe_object = gilsafe_t<py::object>;
+
+} // namespace rpy
+
+
+
+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+PYBIND11_NAMESPACE_BEGIN(detail)
+
+template <typename T>
+struct type_caster<rpy::gilsafe_t<T>> {
+    bool load(handle src, bool convert) {
+        value = src;
+        return true;
+    }
+
+    static handle cast(const handle &src, return_value_policy /* policy */, handle /* parent */) {
+        return src.inc_ref();
+    }
+
+    PYBIND11_TYPE_CASTER(rpy::gilsafe_t<T>, handle_type_name<T>::name);
+};
+
+template <typename T>
+struct handle_type_name<rpy::gilsafe_t<T>> {
+    static constexpr auto name = handle_type_name<T>::name;
+};
+
+PYBIND11_NAMESPACE_END(detail)
+PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

tests/cpp/pyproject.toml.tmpl

@@ -53,6 +53,7 @@ generate = [
     { enums = "enums.h" },
     { factory = "factory.h" },
     { fields = "fields.h" },
+    { gilsafe_container = "gilsafe_container.h" },
     { keepalive = "keepalive.h" },
     { ignore = "ignore.h" },
     { ignored_by_default = "ignored_by_default.h" },

tests/cpp/rpytest/ft/__init__.py

@@ -20,6 +20,7 @@
     GCEnum,
     GEnum,
     GEnumMath,
+    GilsafeContainer,
     HasFactory,
     HasOperator,
     HasOperatorNoDefault,
@@ -135,6 +136,7 @@
     "GCEnum",
     "GEnum",
     "GEnumMath",
+    "GilsafeContainer",
     "HasFactory",
     "HasOperator",
     "HasOperatorNoDefault",

tests/cpp/rpytest/ft/include/gilsafe_container.h

@@ -0,0 +1,27 @@
+
+#pragma once
+
+#include <gilsafe_object.h>
+
+class GilsafeContainer {
+    rpy::gilsafe_object m_o;
+public:
+    void assign(rpy::gilsafe_object o) {
+        m_o = o;
+    }
+
+    static void check() {
+        auto c = std::make_unique<GilsafeContainer>();
+
+        py::gil_scoped_acquire a;
+
+        py::object v = py::none();
+
+        {
+            py::gil_scoped_release r;
+            c->assign(v);
+            c.reset();
+        }
+    }
+
+};

tests/test_ft_misc.py

@@ -130,6 +130,15 @@ def test_factory():
     assert o.m_x == 5
 
 
+#
+# gilsafe_container.h
+#
+
+
+def test_gilsafe_container():
+    ft.GilsafeContainer.check()
+
+
 #
 # inline_code.h
 #