From 3ab9655094796914f6dfd4726b4e4f21f014f445 Mon Sep 17 00:00:00 2001
From: Robert Haschke <rhaschke@techfak.uni-bielefeld.de>
Date: Thu, 10 Dec 2020 17:34:36 +0100
Subject: [PATCH] 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 8b451972e0576d83908ce0922848d84f6b96db43
  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?
---
 include/pybind11/cast.h | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index 0f81bb6ba35..20d176796e9 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -39,6 +39,10 @@
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 PYBIND11_NAMESPACE_BEGIN(detail)
 
+/// HACK: Declare inline function defined in class.h to be called in move_only_holder_caster's destructor
+/// Correct solution would be to move that function into a common header
+void clear_instance(PyObject *self);
+
 /// A life support system for temporary objects created by `type_caster::load()`.
 /// Adding a patient will keep it alive up until the enclosing function returns.
 class loader_life_support {
@@ -588,9 +592,23 @@ class type_caster_generic {
 
     // Base methods for generic caster; they are overridden in copyable_holder_caster
     void load_value(value_and_holder &&v_h) {
-        value = v_h.value_ptr();
-        if (value == nullptr)
-            throw cast_error("Invalid object instance");
+        auto *&vptr = v_h.value_ptr();
+        // Lazy allocation for unallocated values:
+        if (vptr == nullptr) {
+            auto *type = v_h.type ? v_h.type : typeinfo;
+            if (type->operator_new) {
+                vptr = type->operator_new(type->type_size);
+            } else {
+                #if defined(__cpp_aligned_new) && (!defined(_MSC_VER) || _MSC_VER >= 1912)
+                    if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
+                        vptr = ::operator new(type->type_size,
+                                              std::align_val_t(type->type_align));
+                    else
+                #endif
+                vptr = ::operator new(type->type_size);
+            }
+        }
+        value = vptr;
     }
     bool try_implicit_casts(handle src, bool convert) {
         for (auto &cast : typeinfo->implicit_casts) {
@@ -1564,6 +1582,12 @@ struct move_only_holder_caster : public type_caster_base<type> {
     using base::typeinfo;
     using base::value;
 
+    ~move_only_holder_caster() {
+        // if held object was actually moved, unregister it
+        if (!holder_helper<holder_type>::get(*holder_ptr))
+            clear_instance(reinterpret_cast<PyObject*>(v_h.inst));
+    }
+
     bool load(handle& src, bool convert) {
         bool success = base::template load_impl<move_only_holder_caster<type, holder_type>>(src, convert);
         if (success) // On success, remember src pointer to withdraw later
@@ -1578,16 +1602,7 @@ struct move_only_holder_caster : public type_caster_base<type> {
     #if !defined(__ICC) && !defined(__INTEL_COMPILER)
     explicit
     #endif
-    operator holder_type&&() {
-        // In load_value() value_ptr was still valid. If it's invalid now, another argument consumed the same value before.
-        if (!v_h.value_ptr())
-            throw cast_error("Multiple access to moved argument");
-        v_h.value_ptr() = nullptr;
-        // TODO: release object instance in python
-        // clear_instance(src_handle->ptr()); ???
-
-        return std::move(*holder_ptr);
-    }
+    operator holder_type&&() { return std::move(*holder_ptr); }
 
     static handle cast(const holder_type &src, return_value_policy, handle) {
         const auto *ptr = holder_helper<holder_type>::get(src);