Skip to content

Commit

Permalink
Add gilsafe_object to simplify storing python objects in containers
Browse files Browse the repository at this point in the history
  • Loading branch information
virtuald committed Nov 4, 2023
1 parent b11aac5 commit 585217c
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 0 deletions.
133 changes: 133 additions & 0 deletions robotpy_build/include/gilsafe_object.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@

#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.
*/
class gilsafe_object 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_object() = default;

~gilsafe_object() {
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_object(const gilsafe_object &other) {
py::gil_scoped_acquire lock;
o = other.o;
}

// Copy constructor; always increases the reference count
gilsafe_object(const py::object &other) {
py::gil_scoped_acquire lock;
o = other;
}

gilsafe_object(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_object(gilsafe_object &&other) noexcept : o(std::move(other.o)) {}

// Move constructor; steals object from ``other`` and preserves its reference count
gilsafe_object(py::object &&other) noexcept : o(std::move(other)) {}

// copy assignment
gilsafe_object &operator=(const gilsafe_object& other) {
if (!o.is(other.o)) {
py::gil_scoped_acquire lock;
o = other.o;
}
return *this;
}

// move assignment
gilsafe_object &operator=(gilsafe_object&& other) noexcept {
if (this != &other) {
py::gil_scoped_acquire lock;
o = std::move(other.o);
}
return *this;
}
};

} // namespace rpy



PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail)

template <>
struct type_caster<rpy::gilsafe_object> {
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_object, const_name("object"));
};

template <>
struct handle_type_name<rpy::gilsafe_object> {
static constexpr auto name = const_name("object");
};

PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
1 change: 1 addition & 0 deletions tests/cpp/pyproject.toml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down
2 changes: 2 additions & 0 deletions tests/cpp/rpytest/ft/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
GCEnum,
GEnum,
GEnumMath,
GilsafeContainer,
HasFactory,
HasOperator,
HasOperatorNoDefault,
Expand Down Expand Up @@ -135,6 +136,7 @@
"GCEnum",
"GEnum",
"GEnumMath",
"GilsafeContainer",
"HasFactory",
"HasOperator",
"HasOperatorNoDefault",
Expand Down
27 changes: 27 additions & 0 deletions tests/cpp/rpytest/ft/include/gilsafe_container.h
Original file line number Diff line number Diff line change
@@ -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();
}
}

};
9 changes: 9 additions & 0 deletions tests/test_ft_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down

0 comments on commit 585217c

Please sign in to comment.