Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gilsafe_object to simplify storing python objects in containers #220

Merged
merged 1 commit into from
Nov 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 141 additions & 0 deletions robotpy_build/include/gilsafe_object.h
Original file line number Diff line number Diff line change
@@ -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)
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