Bug fix: trampoline_self_life_support CpCtor, MvCtor. (#2947)

include/pybind11/trampoline_self_life_support.h

@@ -22,6 +22,8 @@ PYBIND11_NAMESPACE_END(detail)
 struct trampoline_self_life_support {
     detail::value_and_holder v_h;
 
+    trampoline_self_life_support() = default;
+
     void activate_life_support(const detail::value_and_holder &v_h_) {
         Py_INCREF((PyObject *) v_h_.inst);
         v_h = v_h_;
@@ -46,12 +48,14 @@ struct trampoline_self_life_support {
         }
     }
 
-    // Some compilers complain about implicitly defined versions of some of the following:
+    // For the next two, the default implementations generate undefined behavior (ASAN failures
-    trampoline_self_life_support()                                     = default;
+    // manually verified). The reason is that v_h needs to be kept default-initialized.
-    trampoline_self_life_support(const trampoline_self_life_support &) = default;
+    trampoline_self_life_support(const trampoline_self_life_support &) {}
-    trampoline_self_life_support(trampoline_self_life_support &&)      = default;
+    trampoline_self_life_support(trampoline_self_life_support &&) {}
-    trampoline_self_life_support &operator=(const trampoline_self_life_support &) = default;
+
-    trampoline_self_life_support &operator=(trampoline_self_life_support &&) = default;
+    // These should never be needed (please provide test cases if you think they are).
+    trampoline_self_life_support &operator=(const trampoline_self_life_support &) = delete;
+    trampoline_self_life_support &operator=(trampoline_self_life_support &&) = delete;
 };
 
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

tests/CMakeLists.txt

@@ -107,6 +107,7 @@ set(PYBIND11_TEST_FILES
     test_class_sh_factory_constructors.cpp
     test_class_sh_inheritance.cpp
     test_class_sh_trampoline_basic.cpp
+    test_class_sh_trampoline_self_life_support.cpp
     test_class_sh_trampoline_shared_ptr_cpp_arg.cpp
     test_class_sh_trampoline_unique_ptr.cpp
     test_class_sh_unique_ptr_member.cpp

tests/test_class_sh_trampoline_self_life_support.cpp

@@ -0,0 +1,84 @@
+// Copyright (c) 2021 The Pybind Development Team.
+// All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#include "pybind11/smart_holder.h"
+#include "pybind11/trampoline_self_life_support.h"
+#include "pybind11_tests.h"
+
+#include <memory>
+#include <string>
+
+namespace {
+
+struct Big5 { // Also known as "rule of five".
+    std::string history;
+
+    explicit Big5(std::string history_start) : history{history_start} {}
+
+    Big5(const Big5 &other) { history = other.history + "_CpCtor"; }
+
+    Big5(Big5 &&other) { history = other.history + "_MvCtor"; }
+
+    Big5 &operator=(const Big5 &other) {
+        history = other.history + "_OpEqLv";
+        return *this;
+    }
+
+    Big5 &operator=(Big5 &&other) {
+        history = other.history + "_OpEqRv";
+        return *this;
+    }
+
+    virtual ~Big5() = default;
+
+protected:
+    Big5() : history{"DefaultConstructor"} {}
+};
+
+struct Big5Trampoline : Big5, py::trampoline_self_life_support {
+    using Big5::Big5;
+};
+
+} // namespace
+
+PYBIND11_SMART_HOLDER_TYPE_CASTERS(Big5)
+
+TEST_SUBMODULE(class_sh_trampoline_self_life_support, m) {
+    py::classh<Big5, Big5Trampoline>(m, "Big5")
+        .def(py::init<std::string>())
+        .def_readonly("history", &Big5::history);
+
+    m.def("action", [](std::unique_ptr<Big5> obj, int action_id) {
+        py::object o2 = py::none();
+        // This is very unusual, but needed to directly exercise the trampoline_self_life_support
+        // CpCtor, MvCtor, operator= lvalue, operator= rvalue.
+        auto obj_trampoline = dynamic_cast<Big5Trampoline *>(obj.get());
+        if (obj_trampoline != nullptr) {
+            switch (action_id) {
+                case 0: { // CpCtor
+                    std::unique_ptr<Big5> cp(new Big5Trampoline(*obj_trampoline));
+                    o2 = py::cast(std::move(cp));
+                } break;
+                case 1: { // MvCtor
+                    std::unique_ptr<Big5> mv(new Big5Trampoline(std::move(*obj_trampoline)));
+                    o2 = py::cast(std::move(mv));
+                } break;
+                case 2: { // operator= lvalue
+                    std::unique_ptr<Big5> lv(new Big5Trampoline);
+                    *lv = *obj_trampoline;
+                    o2  = py::cast(std::move(lv));
+                } break;
+                case 3: { // operator= rvalue
+                    std::unique_ptr<Big5> rv(new Big5Trampoline);
+                    *rv = std::move(*obj_trampoline);
+                    o2  = py::cast(std::move(rv));
+                } break;
+                default:
+                    break;
+            }
+        }
+        py::object o1 = py::cast(std::move(obj));
+        return py::make_tuple(o1, o2);
+    });
+}

tests/test_class_sh_trampoline_self_life_support.py

@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+import pytest
+
+import pybind11_tests.class_sh_trampoline_self_life_support as m
+
+
+class PyBig5(m.Big5):
+    pass
+
+
+def test_m_big5():
+    obj = m.Big5("Seed")
+    assert obj.history == "Seed"
+    o1, o2 = m.action(obj, 0)
+    assert o1 is not obj
+    assert o1.history == "Seed"
+    with pytest.raises(ValueError) as excinfo:
+        obj.history
+    assert "Python instance was disowned" in str(excinfo.value)
+    assert o2 is None
+
+
+@pytest.mark.parametrize(
+    "action_id, expected_history",
+    [
+        (0, "Seed_CpCtor"),
+        (1, "Seed_MvCtor"),
+        (2, "Seed_OpEqLv"),
+        (3, "Seed_OpEqRv"),
+    ],
+)
+def test_py_big5(action_id, expected_history):
+    obj = PyBig5("Seed")
+    assert obj.history == "Seed"
+    o1, o2 = m.action(obj, action_id)
+    assert o1 is obj
+    assert o2.history == expected_history