Skip to content

Commit c31659c

Browse files
committed
fix: only add string_view life support for transient sources
Refine the previous commit. Best-effort registration (try_add_patient) silently produces a dangling view when a container of views is built from a generator outside a bound function: there the materialized temporary is released before the view is used, and with no frame nothing keeps it alive. Such a cast cannot be made safe, so it should fail loudly, while a view into a durable, caller-owned object needs no life support at all. The view caster cannot tell a durable source from a pybind11-managed transient one; that provenance lives in the container caster. Introduce an ambient transient_source_guard that the list, set, map, and array casters set around their generator/materialized paths, and have the string caster keep the source alive only when loading from a transient source (via the throwing add_patient, so try_add_patient is no longer needed). This means: - views into durable sources (direct arguments, sequences, manual casts) add no life support and no longer throw outside a bound function, and - a generator used outside a frame throws, rather than silently dangling. The guard restores (rather than clears) the previous value, so a durable container nested in a transient one is correctly treated as transient. Verified with AddressSanitizer: the in-frame generator case is clean, the out-of-frame durable cases succeed, and the out-of-frame generator case throws. Assisted-by: ClaudeCode:claude-opus-4.8
1 parent 91feed5 commit c31659c

4 files changed

Lines changed: 78 additions & 29 deletions

File tree

include/pybind11/cast.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,10 @@ struct string_caster {
525525
return false;
526526
}
527527
value = StringType(buffer, static_cast<size_t>(size));
528-
if (IsView) {
529-
// `src` owns the buffer; keep it alive if inside a bound function,
530-
// otherwise the caller is responsible for its lifetime.
531-
loader_life_support::try_add_patient(src);
528+
if (IsView && loading_from_transient_source()) {
529+
// The view points into `src`, which is owned by a transient source that
530+
// will be released before the view is used, so keep `src` alive.
531+
loader_life_support::add_patient(src);
532532
}
533533
return true;
534534
}
@@ -607,8 +607,8 @@ struct string_caster {
607607
pybind11_fail("Unexpected PYBIND11_BYTES_AS_STRING() failure.");
608608
}
609609
value = StringType(bytes, (size_t) PYBIND11_BYTES_SIZE(src.ptr()));
610-
if (IsView) {
611-
loader_life_support::try_add_patient(src);
610+
if (IsView && loading_from_transient_source()) {
611+
loader_life_support::add_patient(src);
612612
}
613613
return true;
614614
}
@@ -620,8 +620,8 @@ struct string_caster {
620620
pybind11_fail("Unexpected PyByteArray_AsString() failure.");
621621
}
622622
value = StringType(bytearray, (size_t) PyByteArray_Size(src.ptr()));
623-
if (IsView) {
624-
loader_life_support::try_add_patient(src);
623+
if (IsView && loading_from_transient_source()) {
624+
loader_life_support::add_patient(src);
625625
}
626626
return true;
627627
}

include/pybind11/detail/type_caster_base.h

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,26 +81,11 @@ class loader_life_support {
8181
}
8282
}
8383

84-
/// Keep `h` alive until the current patient frame is destroyed, if there is one.
85-
/// Returns false when called outside a bound function (no frame). Use this, rather
86-
/// than `add_patient`, when failing to register is acceptable because the caller
87-
/// owns the source's lifetime outside the call framework (e.g. a view that points
88-
/// into an existing Python object, as opposed to a freshly created temporary).
89-
PYBIND11_NOINLINE static bool try_add_patient(handle h) {
90-
loader_life_support *frame = tls_current_frame();
91-
if (!frame) {
92-
return false;
93-
}
94-
if (frame->keep_alive.insert(h.ptr()).second) {
95-
Py_INCREF(h.ptr());
96-
}
97-
return true;
98-
}
99-
10084
/// This can only be used inside a pybind11-bound function, either by `argument_loader`
10185
/// at argument preparation time or by `py::cast()` at execution time.
10286
PYBIND11_NOINLINE static void add_patient(handle h) {
103-
if (!try_add_patient(h)) {
87+
loader_life_support *frame = tls_current_frame();
88+
if (!frame) {
10489
// NOTE: It would be nice to include the stack frames here, as this indicates
10590
// use of pybind11::cast<> outside the normal call framework, finding such
10691
// a location is challenging. Developers could consider printing out
@@ -109,7 +94,39 @@ class loader_life_support {
10994
"do Python -> C++ conversions which require the creation "
11095
"of temporary values");
11196
}
97+
98+
if (frame->keep_alive.insert(h.ptr()).second) {
99+
Py_INCREF(h.ptr());
100+
}
101+
}
102+
};
103+
104+
// While set, a type caster is converting elements borrowed from a *transient* source --
105+
// a generator/iterator, or a temporary container materialized from one -- whose items are
106+
// released before the converted C++ value is used. A view caster (e.g. for std::string_view)
107+
// consults this to decide whether the viewed Python object needs life support: a view into a
108+
// durable, caller-owned object does not, whereas a view into a transient source does. Outside
109+
// a bound function (no life support frame) the latter cannot be made safe, so `add_patient`
110+
// rightly throws rather than producing a dangling view.
111+
inline bool &loading_from_transient_source() {
112+
static thread_local bool flag = false;
113+
return flag;
114+
}
115+
116+
// RAII guard marking the dynamic scope in which elements are loaded from a transient source.
117+
// Restores (rather than clears) the previous value so that nesting is transitive: a durable
118+
// container nested inside a transient one is itself transient.
119+
class transient_source_guard {
120+
public:
121+
transient_source_guard() : prev(loading_from_transient_source()) {
122+
loading_from_transient_source() = true;
112123
}
124+
~transient_source_guard() { loading_from_transient_source() = prev; }
125+
transient_source_guard(const transient_source_guard &) = delete;
126+
transient_source_guard &operator=(const transient_source_guard &) = delete;
127+
128+
private:
129+
bool prev;
113130
};
114131

115132
// Gets the cache entry for the given type, creating it if necessary. The return value is the pair

include/pybind11/stl.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,9 @@ struct set_caster {
198198
}
199199
assert(isinstance<iterable>(src));
200200
value.clear();
201+
// Elements are borrowed from the iterator and released as it advances, so views
202+
// into them need life support.
203+
transient_source_guard guard;
201204
return convert_iterable(reinterpret_borrow<iterable>(src), convert);
202205
}
203206

@@ -264,6 +267,9 @@ struct map_caster {
264267
throw error_already_set();
265268
}
266269
assert(isinstance<iterable>(items));
270+
// The materialized dict is transient (released when load() returns), so views into
271+
// its keys/values need life support.
272+
transient_source_guard guard;
267273
return convert_elements(dict(reinterpret_borrow<iterable>(items)), convert);
268274
}
269275

@@ -314,6 +320,9 @@ struct list_caster {
314320
// the generator is not left in an unpredictable (to the caller) partially-consumed
315321
// state.
316322
assert(isinstance<iterable>(src));
323+
// The materialized tuple is transient (released when load() returns), so views into
324+
// its elements need life support.
325+
transient_source_guard guard;
317326
return convert_elements(tuple(reinterpret_borrow<iterable>(src)), convert);
318327
}
319328

@@ -449,6 +458,9 @@ struct array_caster {
449458
// the generator is not left in an unpredictable (to the caller) partially-consumed
450459
// state.
451460
assert(isinstance<iterable>(src));
461+
// The materialized tuple is transient (released when load() returns), so views into
462+
// its elements need life support.
463+
transient_source_guard guard;
452464
return convert_elements(tuple(reinterpret_borrow<iterable>(src)), convert);
453465
}
454466

tests/test_with_catch/test_interpreter.cpp

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include <pybind11/critical_section.h>
22
#include <pybind11/embed.h>
3+
#include <pybind11/stl.h>
34

45
// Silence MSVC C++17 deprecation warning from Catch regarding std::uncaught_exceptions (up to
56
// catch 2.0.1; this should be fixed in the next catch release after 2.0.1).
@@ -11,8 +12,10 @@ PYBIND11_WARNING_DISABLE_MSVC(4996)
1112
#include <cstdlib>
1213
#include <fstream>
1314
#include <functional>
15+
#include <string_view>
1416
#include <thread>
1517
#include <utility>
18+
#include <vector>
1619

1720
namespace py = pybind11;
1821
using namespace py::literals;
@@ -511,10 +514,10 @@ TEST_CASE("make_iterator can be called before then after finalizing an interpret
511514
}
512515

513516
#ifdef PYBIND11_HAS_STRING_VIEW
514-
TEST_CASE("Casting to a string_view outside a bound function") {
515-
// Regression for PR #6092: view casters add the source to loader_life_support, but
516-
// outside a bound function there is no frame. The caller owns the source's lifetime
517-
// here, so the cast must succeed rather than throw.
517+
TEST_CASE("Casting to a string_view from a durable source outside a bound function") {
518+
// Outside a bound function there is no loader_life_support frame. When the source is a
519+
// durable, caller-owned object the view does not need life support, so the cast must
520+
// succeed rather than throw (regression from PR #6092).
518521
py::str unicode("hello");
519522
py::bytes bytes_obj("world", 5);
520523
auto bytearray_obj
@@ -523,5 +526,22 @@ TEST_CASE("Casting to a string_view outside a bound function") {
523526
REQUIRE(py::cast<std::string_view>(unicode) == "hello");
524527
REQUIRE(py::cast<std::string_view>(bytes_obj) == "world");
525528
REQUIRE(py::cast<std::string_view>(bytearray_obj) == "bytes");
529+
530+
// A list is durable too: the views point into elements the list keeps alive.
531+
py::list durable;
532+
durable.append("a");
533+
durable.append("b");
534+
auto from_list = py::cast<std::vector<std::string_view>>(durable);
535+
REQUIRE(from_list.size() == 2);
536+
REQUIRE(from_list[0] == "a");
537+
REQUIRE(from_list[1] == "b");
538+
}
539+
540+
TEST_CASE("Casting to a string_view from a transient source outside a bound function") {
541+
// A generator is a transient source: the materialized temporary backing the views is
542+
// released when the cast returns, and there is no frame to keep it alive. This cannot
543+
// be made safe, so it must throw rather than produce dangling views.
544+
auto gen = py::eval("('transient_' + str(x) for x in range(5))");
545+
REQUIRE_THROWS_AS(py::cast<std::vector<std::string_view>>(gen), py::cast_error);
526546
}
527547
#endif

0 commit comments

Comments
 (0)