Skip to content

Commit 6b3c9b5

Browse files
committed
fixup! solved multiple access issue
- Unregister instance after moving, thus avoiding reusing an invalidated instance pointer later on. Note: this implementation is rather hacky, because in hopes that the function clear_instance() inlined in detail/class.h will be available in cast.h as well. A clean solution should move the corresponding code into a shared header. Not sure also, I should clear_instance() or only deregister_instance()? - (Partially) reverts 8b45197 Moving the same variable twice into a function will error in C++ as well. We don't need to catch that in Python, do we?
1 parent 8b45197 commit 6b3c9b5

File tree

1 file changed

+13
-10
lines changed

1 file changed

+13
-10
lines changed

include/pybind11/cast.h

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@
3939
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
4040
PYBIND11_NAMESPACE_BEGIN(detail)
4141

42+
/// HACK: Declare inline function defined in class.h to be called in move_only_holder_caster's destructor
43+
/// Correct solution would be to move that function into a common header
44+
void clear_instance(PyObject *self);
45+
4246
/// A life support system for temporary objects created by `type_caster::load()`.
4347
/// Adding a patient will keep it alive up until the enclosing function returns.
4448
class loader_life_support {
@@ -1564,6 +1568,14 @@ struct move_only_holder_caster : public type_caster_base<type> {
15641568
using base::typeinfo;
15651569
using base::value;
15661570

1571+
~move_only_holder_caster() {
1572+
if (!holder_helper<holder_type>::get(*holder_ptr)) {
1573+
// if held object was actually moved, unregister it
1574+
clear_instance(reinterpret_cast<PyObject*>(v_h.inst));
1575+
v_h.value_ptr() = nullptr; // mark value as moved
1576+
}
1577+
}
1578+
15671579
bool load(handle& src, bool convert) {
15681580
bool success = base::template load_impl<move_only_holder_caster<type, holder_type>>(src, convert);
15691581
if (success) // On success, remember src pointer to withdraw later
@@ -1578,16 +1590,7 @@ struct move_only_holder_caster : public type_caster_base<type> {
15781590
#if !defined(__ICC) && !defined(__INTEL_COMPILER)
15791591
explicit
15801592
#endif
1581-
operator holder_type&&() {
1582-
// In load_value() value_ptr was still valid. If it's invalid now, another argument consumed the same value before.
1583-
if (!v_h.value_ptr())
1584-
throw cast_error("Multiple access to moved argument");
1585-
v_h.value_ptr() = nullptr;
1586-
// TODO: release object instance in python
1587-
// clear_instance(src_handle->ptr()); ???
1588-
1589-
return std::move(*holder_ptr);
1590-
}
1593+
operator holder_type&&() { return std::move(*holder_ptr); }
15911594

15921595
static handle cast(const holder_type &src, return_value_policy, handle) {
15931596
const auto *ptr = holder_helper<holder_type>::get(src);

0 commit comments

Comments
 (0)