From 6b7b3d27e6c0c4438ed2ca4c7a3484d75cd96682 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Tue, 17 Mar 2026 00:49:55 -0400 Subject: [PATCH 1/9] Add tests that cause crash in def_readwrite - Occurs with non-smart-holder property of smart-holder class --- tests/test_class_sh_property.cpp | 31 +++++++++++++++++++++++++++++++ tests/test_class_sh_property.py | 17 +++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/tests/test_class_sh_property.cpp b/tests/test_class_sh_property.cpp index 8863ad7d7b..93981ff8d0 100644 --- a/tests/test_class_sh_property.cpp +++ b/tests/test_class_sh_property.cpp @@ -43,6 +43,23 @@ struct WithConstCharPtrMember { const char *const_char_ptr_member = "ConstChar*"; }; +enum class TinyLevel { + A = 0, + B = 1, +}; + +struct HolderWithEnum { + TinyLevel level = TinyLevel::A; +}; + +struct LegacyThing { + int value = 7; +}; + +struct HolderWithLegacyMember { + LegacyThing legacy; +}; + } // namespace test_class_sh_property TEST_SUBMODULE(class_sh_property, m) { @@ -91,4 +108,18 @@ TEST_SUBMODULE(class_sh_property, m) { py::classh(m, "WithConstCharPtrMember") .def(py::init<>()) .def_readonly("const_char_ptr_member", &WithConstCharPtrMember::const_char_ptr_member); + + py::enum_(m, "TinyLevel").value("A", TinyLevel::A).value("B", TinyLevel::B); + + py::classh(m, "HolderWithEnum") + .def(py::init<>()) + .def_readwrite("level", &HolderWithEnum::level); + + py::class_(m, "LegacyThing") + .def(py::init<>()) + .def_readwrite("value", &LegacyThing::value); + + py::classh(m, "HolderWithLegacyMember") + .def(py::init<>()) + .def_readwrite("legacy", &HolderWithLegacyMember::legacy); } diff --git a/tests/test_class_sh_property.py b/tests/test_class_sh_property.py index 0250a7f78e..2644de0a56 100644 --- a/tests/test_class_sh_property.py +++ b/tests/test_class_sh_property.py @@ -164,3 +164,20 @@ def test_readonly_char6_member(): def test_readonly_const_char_ptr_member(): obj = m.WithConstCharPtrMember() assert obj.const_char_ptr_member == "ConstChar*" + + +def test_enum_member_with_smart_holder_def_readwrite(): + obj = m.HolderWithEnum() + assert obj.level == m.TinyLevel.A + for _ in range(100): + v = obj.level + assert v == m.TinyLevel.A + del v + + +def test_non_smart_holder_member_type_with_smart_holder_owner(): + obj = m.HolderWithLegacyMember() + for _ in range(1000): + v = obj.legacy + assert v.value == 7 + del v From 02f2b5f14e47b6521772f4704bd17fcbfae0778a Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Tue, 17 Mar 2026 01:04:44 -0400 Subject: [PATCH 2/9] Fix crash in def_readwrite for non-smart-holder properties of smart-holder classes --- include/pybind11/cast.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 1c4973b748..05aef0e993 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1021,7 +1021,19 @@ struct copyable_holder_caster< return smart_holder_type_caster_support::smart_holder_from_shared_ptr( src, policy, parent, srcs.result); } - return type_caster_base::cast_holder(srcs, &src); + + auto *tinfo = srcs.result.tinfo; + if (tinfo != nullptr && tinfo->holder_enum_v == holder_enum_t::std_shared_ptr) { + return type_caster_base::cast_holder(srcs, &src); + } + + if (parent) { + return type_caster_base::cast( + srcs, return_value_policy::reference_internal, parent); + } + + throw cast_error("Unable to convert std::shared_ptr to Python when the bound type " + "does not use std::shared_ptr or py::smart_holder as its holder type"); } // This function will succeed even if the `responsible_parent` does not own the From b299f321044ff946701a0020a4549f3eb0525b83 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Sun, 22 Mar 2026 16:22:33 -0400 Subject: [PATCH 3/9] Use default policy --- include/pybind11/cast.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 05aef0e993..802fb845a4 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1028,8 +1028,7 @@ struct copyable_holder_caster< } if (parent) { - return type_caster_base::cast( - srcs, return_value_policy::reference_internal, parent); + return type_caster_base::cast(srcs, policy, parent); } throw cast_error("Unable to convert std::shared_ptr to Python when the bound type " From e1231bfe99956f14998a64fd62ac641d61587576 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Sun, 22 Mar 2026 16:31:23 -0400 Subject: [PATCH 4/9] Address PR comments --- tests/test_class_sh_property.cpp | 28 +++++++++++++++------------- tests/test_class_sh_property.py | 10 ++++++---- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/tests/test_class_sh_property.cpp b/tests/test_class_sh_property.cpp index 93981ff8d0..e330542626 100644 --- a/tests/test_class_sh_property.cpp +++ b/tests/test_class_sh_property.cpp @@ -43,21 +43,22 @@ struct WithConstCharPtrMember { const char *const_char_ptr_member = "ConstChar*"; }; -enum class TinyLevel { +// See PR #6008 +enum class EnumAB { A = 0, B = 1, }; -struct HolderWithEnum { - TinyLevel level = TinyLevel::A; +struct ShWithEnumABMember { + EnumAB level = EnumAB::A; }; -struct LegacyThing { +struct SimpleStruct { int value = 7; }; -struct HolderWithLegacyMember { - LegacyThing legacy; +struct ShWithSimpleStructMember { + SimpleStruct legacy; }; } // namespace test_class_sh_property @@ -109,17 +110,18 @@ TEST_SUBMODULE(class_sh_property, m) { .def(py::init<>()) .def_readonly("const_char_ptr_member", &WithConstCharPtrMember::const_char_ptr_member); - py::enum_(m, "TinyLevel").value("A", TinyLevel::A).value("B", TinyLevel::B); + // See PR #6008 + py::enum_(m, "EnumAB").value("A", EnumAB::A).value("B", EnumAB::B); - py::classh(m, "HolderWithEnum") + py::classh(m, "ShWithEnumABMember") .def(py::init<>()) - .def_readwrite("level", &HolderWithEnum::level); + .def_readwrite("level", &ShWithEnumABMember::level); - py::class_(m, "LegacyThing") + py::class_(m, "SimpleStruct") .def(py::init<>()) - .def_readwrite("value", &LegacyThing::value); + .def_readwrite("value", &SimpleStruct::value); - py::classh(m, "HolderWithLegacyMember") + py::classh(m, "ShWithSimpleStructMember") .def(py::init<>()) - .def_readwrite("legacy", &HolderWithLegacyMember::legacy); + .def_readwrite("legacy", &ShWithSimpleStructMember::legacy); } diff --git a/tests/test_class_sh_property.py b/tests/test_class_sh_property.py index 2644de0a56..02e84ae0bb 100644 --- a/tests/test_class_sh_property.py +++ b/tests/test_class_sh_property.py @@ -166,17 +166,19 @@ def test_readonly_const_char_ptr_member(): assert obj.const_char_ptr_member == "ConstChar*" +# See PR #6008 def test_enum_member_with_smart_holder_def_readwrite(): - obj = m.HolderWithEnum() - assert obj.level == m.TinyLevel.A + obj = m.ShWithEnumABMember() + assert obj.level == m.EnumAB.A for _ in range(100): v = obj.level - assert v == m.TinyLevel.A + assert v == m.EnumAB.A del v +# See PR #6008 def test_non_smart_holder_member_type_with_smart_holder_owner(): - obj = m.HolderWithLegacyMember() + obj = m.ShWithSimpleStructMember() for _ in range(1000): v = obj.legacy assert v.value == 7 From 64a8cd8c82a531d23b8ce7a0b41b9a361f369b1e Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Sun, 22 Mar 2026 17:01:43 -0400 Subject: [PATCH 5/9] Add test for cast error path --- tests/test_class_sh_property.cpp | 4 ++++ tests/test_class_sh_property.py | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/tests/test_class_sh_property.cpp b/tests/test_class_sh_property.cpp index e330542626..b830ddab89 100644 --- a/tests/test_class_sh_property.cpp +++ b/tests/test_class_sh_property.cpp @@ -124,4 +124,8 @@ TEST_SUBMODULE(class_sh_property, m) { py::classh(m, "ShWithSimpleStructMember") .def(py::init<>()) .def_readwrite("legacy", &ShWithSimpleStructMember::legacy); + + m.def("getSimpleStructAsShared", []() { + return std::make_shared(); + }); } diff --git a/tests/test_class_sh_property.py b/tests/test_class_sh_property.py index 02e84ae0bb..9754ae6355 100644 --- a/tests/test_class_sh_property.py +++ b/tests/test_class_sh_property.py @@ -183,3 +183,9 @@ def test_non_smart_holder_member_type_with_smart_holder_owner(): v = obj.legacy assert v.value == 7 del v + + +# See PR #6008, previously this was UB +def test_shared_ptr_return_for_unique_ptr_holder(): + with pytest.raises(RuntimeError, match="Unable to convert std::shared_ptr to Python when the bound type does not use std::shared_ptr or py::smart_holder as its holder type"): + m.getSimpleStructAsShared() From 141ac16b826af520c72d9cbf4ca84ba99551d415 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 22 Mar 2026 21:02:37 +0000 Subject: [PATCH 6/9] style: pre-commit fixes --- tests/test_class_sh_property.cpp | 4 +--- tests/test_class_sh_property.py | 5 ++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_class_sh_property.cpp b/tests/test_class_sh_property.cpp index b830ddab89..8777bdc932 100644 --- a/tests/test_class_sh_property.cpp +++ b/tests/test_class_sh_property.cpp @@ -125,7 +125,5 @@ TEST_SUBMODULE(class_sh_property, m) { .def(py::init<>()) .def_readwrite("legacy", &ShWithSimpleStructMember::legacy); - m.def("getSimpleStructAsShared", []() { - return std::make_shared(); - }); + m.def("getSimpleStructAsShared", []() { return std::make_shared(); }); } diff --git a/tests/test_class_sh_property.py b/tests/test_class_sh_property.py index 9754ae6355..cd49426f19 100644 --- a/tests/test_class_sh_property.py +++ b/tests/test_class_sh_property.py @@ -187,5 +187,8 @@ def test_non_smart_holder_member_type_with_smart_holder_owner(): # See PR #6008, previously this was UB def test_shared_ptr_return_for_unique_ptr_holder(): - with pytest.raises(RuntimeError, match="Unable to convert std::shared_ptr to Python when the bound type does not use std::shared_ptr or py::smart_holder as its holder type"): + with pytest.raises( + RuntimeError, + match="Unable to convert std::shared_ptr to Python when the bound type does not use std::shared_ptr or py::smart_holder as its holder type", + ): m.getSimpleStructAsShared() From 2286af61efe28498f2958975579477f7a2452b03 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 23 Mar 2026 21:30:47 -0700 Subject: [PATCH 7/9] Revert "Use default policy" This reverts commit b299f321044ff946701a0020a4549f3eb0525b83. --- include/pybind11/cast.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index d54a2ab41e..bef0f2cb8f 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1027,7 +1027,8 @@ struct copyable_holder_caster< } if (parent) { - return type_caster_base::cast(srcs, policy, parent); + return type_caster_base::cast( + srcs, return_value_policy::reference_internal, parent); } throw cast_error("Unable to convert std::shared_ptr to Python when the bound type " From 2d8b02711870cbab7e54cd8ac21b3d5133c41524 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Tue, 24 Mar 2026 01:01:15 -0400 Subject: [PATCH 8/9] Disable test_shared_ptr_return_for_unique_ptr_holder when PYBIND11_TEST_SMART_HOLDER=ON --- tests/pybind11_tests.cpp | 6 ++++++ tests/test_class_sh_property.py | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/tests/pybind11_tests.cpp b/tests/pybind11_tests.cpp index 4317925737..47d9d9a8cb 100644 --- a/tests/pybind11_tests.cpp +++ b/tests/pybind11_tests.cpp @@ -96,6 +96,12 @@ PYBIND11_MODULE(pybind11_tests, m, py::mod_gil_not_used()) { #else false; #endif + m.attr("PYBIND11_TEST_SMART_HOLDER") = +#if defined(PYBIND11_RUN_TESTING_WITH_SMART_HOLDER_AS_DEFAULT_BUT_NEVER_USE_IN_PRODUCTION_PLEASE) + true; +#else + false; +#endif bind_ConstructorStats(m); diff --git a/tests/test_class_sh_property.py b/tests/test_class_sh_property.py index cd49426f19..72e69bdca3 100644 --- a/tests/test_class_sh_property.py +++ b/tests/test_class_sh_property.py @@ -5,6 +5,7 @@ import pytest import env # noqa: F401 +import pybind11_tests from pybind11_tests import class_sh_property as m @@ -186,6 +187,10 @@ def test_non_smart_holder_member_type_with_smart_holder_owner(): # See PR #6008, previously this was UB +@pytest.mark.skipif( + pybind11_tests.PYBIND11_TEST_SMART_HOLDER, + reason="PYBIND11_TEST_SMART_HOLDER changes the default holder", +) def test_shared_ptr_return_for_unique_ptr_holder(): with pytest.raises( RuntimeError, From 6dbf1fc2837473507a0951f17cd0273098ba5da2 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Tue, 24 Mar 2026 01:02:23 -0400 Subject: [PATCH 9/9] Add counterexample --- tests/test_class_sh_property.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_class_sh_property.py b/tests/test_class_sh_property.py index 72e69bdca3..4a7b77c69a 100644 --- a/tests/test_class_sh_property.py +++ b/tests/test_class_sh_property.py @@ -197,3 +197,10 @@ def test_shared_ptr_return_for_unique_ptr_holder(): match="Unable to convert std::shared_ptr to Python when the bound type does not use std::shared_ptr or py::smart_holder as its holder type", ): m.getSimpleStructAsShared() + + +def test_non_smart_holder_member_type_with_smart_holder_owner_aliases_member(): + obj = m.ShWithSimpleStructMember() + legacy = obj.legacy + legacy.value = 13 + assert obj.legacy.value == 13