Merge pull request #11 from EricCousineau-TRI/issue/drake_8160

unique_ptr: Fix ownership issues for failed overloads (RobotLocomotion/drake#8160)

include/pybind11/cast.h

@@ -1630,6 +1630,22 @@ struct move_only_holder_caster : type_caster_base<type> {
     using base::typeinfo;
     using base::value;
 
+    // We must explicitly define the default constructor(s) since we define a
+    // destructor; otherwise, the compiler will incorrectly use the copy
+    // constructor.
+    move_only_holder_caster() = default;
+    move_only_holder_caster(move_only_holder_caster&&) = default;
+    move_only_holder_caster(const move_only_holder_caster&) = delete;
+    ~move_only_holder_caster() {
+        if (holder) {
+            // If the argument was loaded into C++, but not transferred out,
+            // then this was most likely part of a failed overload in
+            // `argument_loader`. Transfer ownership back to Python.
+            move_only_holder_caster::cast(
+                std::move(holder), return_value_policy{}, handle{});
+        }
+    }
+
     static_assert(std::is_base_of<type_caster_base<type>, type_caster<type>>::value,
             "Holder classes are only supported for custom types");
 

tests/test_smart_ptr.cpp

@@ -335,6 +335,28 @@ TEST_SUBMODULE(smart_ptr, m) {
             return py::cast(std::move(obj));
         });
 
+    class FirstT {};
+    py::class_<FirstT>(m, "FirstT")
+        .def(py::init());
+    class SecondT {};
+    py::class_<SecondT>(m, "SecondT")
+        .def(py::init());
+
+    m.def("unique_ptr_overload",
+        [](std::unique_ptr<UniquePtrHeld> obj, FirstT) {
+            py::dict out;
+            out["obj"] = py::cast(std::move(obj));
+            out["overload"] = 1;
+            return out;
+        });
+    m.def("unique_ptr_overload",
+        [](std::unique_ptr<UniquePtrHeld> obj, SecondT) {
+            py::dict out;
+            out["obj"] = py::cast(std::move(obj));
+            out["overload"] = 2;
+            return out;
+        });
+
     // Ensure class is non-empty, so it's easier to detect double-free
     // corruption. (If empty, this may be harder to see easily.)
     struct SharedPtrHeld { int value = 10; };

tests/test_smart_ptr.py

@@ -258,3 +258,14 @@ def test_unique_ptr_arg():
 def test_unique_ptr_to_shared_ptr():
     obj = m.shared_ptr_held_in_unique_ptr()
     assert m.shared_ptr_held_func(obj)
+
+
+def test_unique_ptr_overload_fail():
+    obj = m.UniquePtrHeld(1)
+    # These overloads pass ownership back to Python.
+    out = m.unique_ptr_overload(obj, m.FirstT())
+    assert out["obj"] is obj
+    assert out["overload"] == 1
+    out = m.unique_ptr_overload(obj, m.SecondT())
+    assert out["obj"] is obj
+    assert out["overload"] == 2