mirror of
https://github.com/pybind/pybind11.git
synced 2026-04-26 17:59:11 +00:00
feat: add py::potentially_slicing_weak_ptr(handle) function (#5624)
* Added weak_ptr test (currently failing)
* Cleanup
* [skip ci] Simplify test case.
* Add test_class_sp_trampoline_weak_ptr.cpp,py (using std::shared_ptr as holder). Tweak test_class_sh_trampoline_weak_ptr.py to pass, with `# THIS NEEDS FIXING` comment.
* Resolve clang-tidy errors
```
/__w/pybind11/pybind11/tests/test_class_sh_trampoline_weak_ptr.cpp:23:43: error: the parameter 'sp' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors]
23 | void set_wp(std::shared_ptr<VirtBase> sp) { wp = sp; }
| ^
| const &
4443 warnings generated.
```
```
/__w/pybind11/pybind11/tests/test_class_sp_trampoline_weak_ptr.cpp:23:43: error: the parameter 'sp' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors]
23 | void set_wp(std::shared_ptr<VirtBase> sp) { wp = sp; }
| ^
| const &
4430 warnings generated.
```
* PYPY, GRAALPY: skip last part of test_weak_ptr_base
* Rename test_weak_ptr_base → test_weak_ptr_owner
* Add SpOwner, test_with_sp_owner, test_with_sp_and_wp_owners
* Modify py::trampoline_self_life_support semantics: if trampoline class does not inherit from this class, preserve established Inheritance Slicing behavior.
rwgk reached this point with the help of ChatGPT:
* https://chatgpt.com/share/68056498-7d94-8008-8ff0-232e2aba451c
The only production code change in this commit is:
```
diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h
index d4f9a41e..f3d45301 100644
--- a/include/pybind11/detail/type_caster_base.h
+++ b/include/pybind11/detail/type_caster_base.h
@@ -776,6 +776,14 @@ struct load_helper : value_and_holder_helper {
if (released_ptr) {
return std::shared_ptr<T>(released_ptr, type_raw_ptr);
}
+ auto *self_life_support
+ = dynamic_raw_ptr_cast_if_possible<trampoline_self_life_support>(type_raw_ptr);
+ if (self_life_support == nullptr) {
+ std::shared_ptr<void> void_shd_ptr = hld.template as_shared_ptr<void>();
+ std::shared_ptr<T> to_be_released(void_shd_ptr, type_raw_ptr);
+ vptr_gd_ptr->released_ptr = to_be_released;
+ return to_be_released;
+ }
std::shared_ptr<T> to_be_released(
type_raw_ptr, shared_ptr_trampoline_self_life_support(loaded_v_h.inst));
vptr_gd_ptr->released_ptr = to_be_released;
```
* Remove debug printf in include/pybind11/pybind11.h
* Resolve MSVC error
```
11>D:\a\pybind11\pybind11\tests\test_class_sp_trampoline_weak_ptr.cpp(44,50): error C2220: the following warning is treated as an error [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
11>D:\a\pybind11\pybind11\tests\test_class_sp_trampoline_weak_ptr.cpp(44,50): warning C4458: declaration of 'sp' hides class member [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
D:\a\pybind11\pybind11\tests\test_class_sp_trampoline_weak_ptr.cpp(54,31):
see declaration of 'pybind11_tests::class_sp_trampoline_weak_ptr::SpOwner::sp'
```
* [skip ci] Undo the production code change under 4638e017b6
Also undo the corresponding test change in test_class_sh_trampoline_weak_ptr.py
But keep all extra debugging code for now.
* [skip ci] Introduce lambda in `WpOwner::set_wp` bindings, but simply cast to `std::shared_ptr<VirtBase>` for now.
* Add `py::potentially_slicing_shared_ptr()`
* Add `type_id<T>()` to `py::potentially_slicing_shared_ptr()` error message and add test.
* test_potentially_slicing_shared_ptr.cpp,py (for smart_holder only)
* Generalize test_potentially_slicing_shared_ptr.cpp,py for testing with smart_holder and std::shared_ptr as holder.
* Add back test_potentially_slicing_shared_ptr_not_convertible_error(), it got lost accidentally in commit 56d23dc478
* Add simple trampoline state assertions.
* Resolve clang-tidy errors.
```
/__w/pybind11/pybind11/tests/test_potentially_slicing_shared_ptr.cpp:30:9: error: 'magic_token' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer,-warnings-as-errors]
29 | trampoline_is_alive_simple(const trampoline_is_alive_simple &other) {
| : magic_token(other.magic_token)
30 | magic_token = other.magic_token;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/__w/pybind11/pybind11/tests/test_potentially_slicing_shared_ptr.cpp:33:9: error: 'magic_token' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer,-warnings-as-errors]
32 | trampoline_is_alive_simple(trampoline_is_alive_simple &&other) noexcept {
| : magic_token(other.magic_token)
33 | magic_token = other.magic_token;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
* Add a (long) C++ comment for py::potentially_slicing_shared_ptr<>()
* [skip ci] Add new "Avoiding Inheritance Slicing and ``std::weak_ptr`` surprises" section in advanced/classes.rst
* [skip ci] Add introductory comment to test_potentially_slicing_shared_ptr.py
* Minimal (!) changes to have py::potentially_slicing_weak_ptr<T>(handle) as the public API. For CI testing, before changing the names around more widely, and the documentation.
* Rename test_potentially_slicing_shared_ptr.cpp,py → test_potentially_slicing_weak_ptr.cpp,py
* Update docs/advanced/classes.rst and C++ comments → potentially_slicing_weak_ptr
* Write "shared_ptr" instead of just "pointer" in a couple places in docs/advanced/classes.rst
* Add comment for force_potentially_slicing_shared_ptr in type_caster_base.h, to make a direct connection py::potentially_slicing_weak_ptr
---------
Co-authored-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
This commit is contained in:
@@ -161,6 +161,7 @@ set(PYBIND11_TEST_FILES
|
||||
test_opaque_types
|
||||
test_operator_overloading
|
||||
test_pickling
|
||||
test_potentially_slicing_weak_ptr
|
||||
test_python_multiple_inheritance
|
||||
test_pytypes
|
||||
test_sequences_and_iterators
|
||||
|
||||
168
tests/test_potentially_slicing_weak_ptr.cpp
Normal file
168
tests/test_potentially_slicing_weak_ptr.cpp
Normal file
@@ -0,0 +1,168 @@
|
||||
// Copyright (c) 2025 The pybind Community.
|
||||
// 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_tests.h"
|
||||
|
||||
#include <memory>
|
||||
|
||||
namespace pybind11_tests {
|
||||
namespace potentially_slicing_weak_ptr {
|
||||
|
||||
template <int> // Using int as a trick to easily generate multiple types.
|
||||
struct VirtBase {
|
||||
virtual ~VirtBase() = default;
|
||||
virtual int get_code() { return 100; }
|
||||
};
|
||||
|
||||
using VirtBaseSH = VirtBase<0>; // for testing with py::smart_holder
|
||||
using VirtBaseSP = VirtBase<1>; // for testing with std::shared_ptr as holder
|
||||
|
||||
// Similar to trampoline_self_life_support
|
||||
struct trampoline_is_alive_simple {
|
||||
std::uint64_t magic_token = 197001010000u;
|
||||
|
||||
trampoline_is_alive_simple() = default;
|
||||
|
||||
~trampoline_is_alive_simple() { magic_token = 20380118191407u; }
|
||||
|
||||
trampoline_is_alive_simple(const trampoline_is_alive_simple &other) = default;
|
||||
trampoline_is_alive_simple(trampoline_is_alive_simple &&other) noexcept
|
||||
: magic_token(other.magic_token) {
|
||||
other.magic_token = 20380118191407u;
|
||||
}
|
||||
|
||||
trampoline_is_alive_simple &operator=(const trampoline_is_alive_simple &) = delete;
|
||||
trampoline_is_alive_simple &operator=(trampoline_is_alive_simple &&) = delete;
|
||||
};
|
||||
|
||||
template <typename VB>
|
||||
const char *determine_trampoline_state(const std::shared_ptr<VB> &sp) {
|
||||
if (!sp) {
|
||||
return "sp nullptr";
|
||||
}
|
||||
auto *tias = dynamic_cast<trampoline_is_alive_simple *>(sp.get());
|
||||
if (!tias) {
|
||||
return "dynamic_cast failed";
|
||||
}
|
||||
if (tias->magic_token == 197001010000u) {
|
||||
return "trampoline alive";
|
||||
}
|
||||
if (tias->magic_token == 20380118191407u) {
|
||||
return "trampoline DEAD";
|
||||
}
|
||||
return "UNDEFINED BEHAVIOR";
|
||||
}
|
||||
|
||||
struct PyVirtBaseSH : VirtBaseSH, py::trampoline_self_life_support, trampoline_is_alive_simple {
|
||||
using VirtBaseSH::VirtBaseSH;
|
||||
int get_code() override { PYBIND11_OVERRIDE(int, VirtBaseSH, get_code); }
|
||||
};
|
||||
|
||||
struct PyVirtBaseSP : VirtBaseSP, trampoline_is_alive_simple { // self-life-support not available
|
||||
using VirtBaseSP::VirtBaseSP;
|
||||
int get_code() override { PYBIND11_OVERRIDE(int, VirtBaseSP, get_code); }
|
||||
};
|
||||
|
||||
template <typename VB>
|
||||
std::shared_ptr<VB> rtrn_obj_cast_shared_ptr(py::handle obj) {
|
||||
return obj.cast<std::shared_ptr<VB>>();
|
||||
}
|
||||
|
||||
// There is no type_caster<std::weak_ptr<VB>>, and to minimize code complexity
|
||||
// we do not want to add one, therefore we have to return a shared_ptr here.
|
||||
template <typename VB>
|
||||
std::shared_ptr<VB> rtrn_potentially_slicing_shared_ptr(py::handle obj) {
|
||||
return py::potentially_slicing_weak_ptr<VB>(obj).lock();
|
||||
}
|
||||
|
||||
template <typename VB>
|
||||
struct SpOwner {
|
||||
void set_sp(const std::shared_ptr<VB> &sp_) { sp = sp_; }
|
||||
|
||||
int get_code() const {
|
||||
if (!sp) {
|
||||
return -888;
|
||||
}
|
||||
return sp->get_code();
|
||||
}
|
||||
|
||||
const char *get_trampoline_state() const { return determine_trampoline_state(sp); }
|
||||
|
||||
private:
|
||||
std::shared_ptr<VB> sp;
|
||||
};
|
||||
|
||||
template <typename VB>
|
||||
struct WpOwner {
|
||||
void set_wp(const std::weak_ptr<VB> &wp_) { wp = wp_; }
|
||||
|
||||
int get_code() const {
|
||||
auto sp = wp.lock();
|
||||
if (!sp) {
|
||||
return -999;
|
||||
}
|
||||
return sp->get_code();
|
||||
}
|
||||
|
||||
const char *get_trampoline_state() const { return determine_trampoline_state(wp.lock()); }
|
||||
|
||||
private:
|
||||
std::weak_ptr<VB> wp;
|
||||
};
|
||||
|
||||
template <typename VB>
|
||||
void wrap(py::module_ &m,
|
||||
const char *roc_pyname,
|
||||
const char *rps_pyname,
|
||||
const char *spo_pyname,
|
||||
const char *wpo_pyname) {
|
||||
m.def(roc_pyname, rtrn_obj_cast_shared_ptr<VB>);
|
||||
m.def(rps_pyname, rtrn_potentially_slicing_shared_ptr<VB>);
|
||||
|
||||
py::classh<SpOwner<VB>>(m, spo_pyname)
|
||||
.def(py::init<>())
|
||||
.def("set_sp", &SpOwner<VB>::set_sp)
|
||||
.def("get_code", &SpOwner<VB>::get_code)
|
||||
.def("get_trampoline_state", &SpOwner<VB>::get_trampoline_state);
|
||||
|
||||
py::classh<WpOwner<VB>>(m, wpo_pyname)
|
||||
.def(py::init<>())
|
||||
.def("set_wp",
|
||||
[](WpOwner<VB> &self, py::handle obj) {
|
||||
self.set_wp(obj.cast<std::shared_ptr<VB>>());
|
||||
})
|
||||
.def("set_wp_potentially_slicing",
|
||||
[](WpOwner<VB> &self, py::handle obj) {
|
||||
self.set_wp(py::potentially_slicing_weak_ptr<VB>(obj));
|
||||
})
|
||||
.def("get_code", &WpOwner<VB>::get_code)
|
||||
.def("get_trampoline_state", &WpOwner<VB>::get_trampoline_state);
|
||||
}
|
||||
|
||||
} // namespace potentially_slicing_weak_ptr
|
||||
} // namespace pybind11_tests
|
||||
|
||||
using namespace pybind11_tests::potentially_slicing_weak_ptr;
|
||||
|
||||
TEST_SUBMODULE(potentially_slicing_weak_ptr, m) {
|
||||
py::classh<VirtBaseSH, PyVirtBaseSH>(m, "VirtBaseSH")
|
||||
.def(py::init<>())
|
||||
.def("get_code", &VirtBaseSH::get_code);
|
||||
|
||||
py::class_<VirtBaseSP, std::shared_ptr<VirtBaseSP>, PyVirtBaseSP>(m, "VirtBaseSP")
|
||||
.def(py::init<>())
|
||||
.def("get_code", &VirtBaseSP::get_code);
|
||||
|
||||
wrap<VirtBaseSH>(m,
|
||||
"SH_rtrn_obj_cast_shared_ptr",
|
||||
"SH_rtrn_potentially_slicing_shared_ptr",
|
||||
"SH_SpOwner",
|
||||
"SH_WpOwner");
|
||||
|
||||
wrap<VirtBaseSP>(m,
|
||||
"SP_rtrn_obj_cast_shared_ptr",
|
||||
"SP_rtrn_potentially_slicing_shared_ptr",
|
||||
"SP_SpOwner",
|
||||
"SP_WpOwner");
|
||||
}
|
||||
174
tests/test_potentially_slicing_weak_ptr.py
Normal file
174
tests/test_potentially_slicing_weak_ptr.py
Normal file
@@ -0,0 +1,174 @@
|
||||
# Copyright (c) 2025 The pybind Community.
|
||||
# All rights reserved. Use of this source code is governed by a
|
||||
# BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
# This module tests the interaction of pybind11's shared_ptr and smart_holder
|
||||
# mechanisms with trampoline object lifetime management and inheritance slicing.
|
||||
#
|
||||
# The following combinations are covered:
|
||||
#
|
||||
# - Holder type: std::shared_ptr (class_ holder) vs.
|
||||
# py::smart_holder
|
||||
# - Conversion function: obj.cast<std::shared_ptr<T>>() vs.
|
||||
# py::potentially_slicing_weak_ptr<T>(obj)
|
||||
# - Python object type: C++ base class vs.
|
||||
# Python-derived trampoline class
|
||||
#
|
||||
# The tests verify
|
||||
#
|
||||
# - that casting or passing Python objects into functions returns usable
|
||||
# std::shared_ptr<T> instances.
|
||||
# - that inheritance slicing occurs as expected in controlled cases
|
||||
# (issue #1333).
|
||||
# - that surprising weak_ptr behavior (issue #5623) can be reproduced when
|
||||
# smart_holder is used.
|
||||
# - that the trampoline object remains alive in all situations
|
||||
# (no use-after-free) as long as the C++ shared_ptr exists.
|
||||
#
|
||||
# Where applicable, trampoline state is introspected to confirm whether the
|
||||
# C++ object retains knowledge of the Python override or has fallen back to
|
||||
# the base implementation.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import gc
|
||||
import weakref
|
||||
|
||||
import pytest
|
||||
|
||||
import env
|
||||
import pybind11_tests.potentially_slicing_weak_ptr as m
|
||||
|
||||
|
||||
class PyDrvdSH(m.VirtBaseSH):
|
||||
def get_code(self):
|
||||
return 200
|
||||
|
||||
|
||||
class PyDrvdSP(m.VirtBaseSP):
|
||||
def get_code(self):
|
||||
return 200
|
||||
|
||||
|
||||
VIRT_BASE_TYPES = {
|
||||
"SH": {100: m.VirtBaseSH, 200: PyDrvdSH},
|
||||
"SP": {100: m.VirtBaseSP, 200: PyDrvdSP},
|
||||
}
|
||||
|
||||
RTRN_FUNCS = {
|
||||
"SH": {
|
||||
"oc": m.SH_rtrn_obj_cast_shared_ptr,
|
||||
"ps": m.SH_rtrn_potentially_slicing_shared_ptr,
|
||||
},
|
||||
"SP": {
|
||||
"oc": m.SP_rtrn_obj_cast_shared_ptr,
|
||||
"ps": m.SP_rtrn_potentially_slicing_shared_ptr,
|
||||
},
|
||||
}
|
||||
|
||||
SP_OWNER_TYPES = {
|
||||
"SH": m.SH_SpOwner,
|
||||
"SP": m.SP_SpOwner,
|
||||
}
|
||||
|
||||
WP_OWNER_TYPES = {
|
||||
"SH": m.SH_WpOwner,
|
||||
"SP": m.SP_WpOwner,
|
||||
}
|
||||
|
||||
GC_IS_RELIABLE = not (env.PYPY or env.GRAALPY)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("expected_code", [100, 200])
|
||||
@pytest.mark.parametrize("rtrn_kind", ["oc", "ps"])
|
||||
@pytest.mark.parametrize("holder_kind", ["SH", "SP"])
|
||||
def test_rtrn_obj_cast_shared_ptr(holder_kind, rtrn_kind, expected_code):
|
||||
obj = VIRT_BASE_TYPES[holder_kind][expected_code]()
|
||||
ptr = RTRN_FUNCS[holder_kind][rtrn_kind](obj)
|
||||
assert ptr.get_code() == expected_code
|
||||
objref = weakref.ref(obj)
|
||||
del obj
|
||||
gc.collect()
|
||||
assert ptr.get_code() == expected_code # the ptr Python object keeps obj alive
|
||||
assert objref() is not None
|
||||
del ptr
|
||||
gc.collect()
|
||||
if GC_IS_RELIABLE:
|
||||
assert objref() is None
|
||||
|
||||
|
||||
@pytest.mark.parametrize("expected_code", [100, 200])
|
||||
@pytest.mark.parametrize("holder_kind", ["SH", "SP"])
|
||||
def test_with_sp_owner(holder_kind, expected_code):
|
||||
spo = SP_OWNER_TYPES[holder_kind]()
|
||||
assert spo.get_code() == -888
|
||||
assert spo.get_trampoline_state() == "sp nullptr"
|
||||
|
||||
obj = VIRT_BASE_TYPES[holder_kind][expected_code]()
|
||||
assert obj.get_code() == expected_code
|
||||
|
||||
spo.set_sp(obj)
|
||||
assert spo.get_code() == expected_code
|
||||
expected_trampoline_state = (
|
||||
"dynamic_cast failed" if expected_code == 100 else "trampoline alive"
|
||||
)
|
||||
assert spo.get_trampoline_state() == expected_trampoline_state
|
||||
|
||||
del obj
|
||||
gc.collect()
|
||||
if holder_kind == "SH":
|
||||
assert spo.get_code() == expected_code
|
||||
elif GC_IS_RELIABLE:
|
||||
assert (
|
||||
spo.get_code() == 100
|
||||
) # see issue #1333 (inheritance slicing) and PR #5624
|
||||
assert spo.get_trampoline_state() == expected_trampoline_state
|
||||
|
||||
|
||||
@pytest.mark.parametrize("expected_code", [100, 200])
|
||||
@pytest.mark.parametrize("set_meth", ["set_wp", "set_wp_potentially_slicing"])
|
||||
@pytest.mark.parametrize("holder_kind", ["SH", "SP"])
|
||||
def test_with_wp_owner(holder_kind, set_meth, expected_code):
|
||||
wpo = WP_OWNER_TYPES[holder_kind]()
|
||||
assert wpo.get_code() == -999
|
||||
assert wpo.get_trampoline_state() == "sp nullptr"
|
||||
|
||||
obj = VIRT_BASE_TYPES[holder_kind][expected_code]()
|
||||
assert obj.get_code() == expected_code
|
||||
|
||||
getattr(wpo, set_meth)(obj)
|
||||
if (
|
||||
holder_kind == "SP"
|
||||
or expected_code == 100
|
||||
or set_meth == "set_wp_potentially_slicing"
|
||||
):
|
||||
assert wpo.get_code() == expected_code
|
||||
else:
|
||||
assert wpo.get_code() == -999 # see issue #5623 (weak_ptr expired) and PR #5624
|
||||
if expected_code == 100:
|
||||
expected_trampoline_state = "dynamic_cast failed"
|
||||
elif holder_kind == "SH" and set_meth == "set_wp":
|
||||
expected_trampoline_state = "sp nullptr"
|
||||
else:
|
||||
expected_trampoline_state = "trampoline alive"
|
||||
assert wpo.get_trampoline_state() == expected_trampoline_state
|
||||
|
||||
del obj
|
||||
gc.collect()
|
||||
if GC_IS_RELIABLE:
|
||||
assert wpo.get_code() == -999
|
||||
|
||||
|
||||
def test_potentially_slicing_weak_ptr_not_convertible_error():
|
||||
with pytest.raises(Exception) as excinfo:
|
||||
m.SH_rtrn_potentially_slicing_shared_ptr("")
|
||||
assert str(excinfo.value) == (
|
||||
'"str" object is not convertible to std::weak_ptr<T>'
|
||||
" (with T = pybind11_tests::potentially_slicing_weak_ptr::VirtBase<0>)"
|
||||
)
|
||||
with pytest.raises(Exception) as excinfo:
|
||||
m.SP_rtrn_potentially_slicing_shared_ptr([])
|
||||
assert str(excinfo.value) == (
|
||||
'"list" object is not convertible to std::weak_ptr<T>'
|
||||
" (with T = pybind11_tests::potentially_slicing_weak_ptr::VirtBase<1>)"
|
||||
)
|
||||
Reference in New Issue
Block a user