Experimental work on top of PR #6096

docs/advanced/cast/stl.rst

@@ -17,6 +17,10 @@ converted (i.e. copied) on every Python->C++ and C++->Python transition, which
 can have implications on the program semantics and performance. Please read the
 next sections for more details and alternative approaches that avoid this.
 
+Copying the container does not make non-owning element types own their data.
+In particular, containers of C++ string views have additional
+:ref:`string_view_lifetime` requirements.
+
 .. note::
 
     Arbitrary nesting of any of these types is possible.

docs/advanced/cast/strings.rst

@@ -280,6 +280,8 @@ expressed as a single Unicode code point
 no way to capture them in a C++ character type.
 
 
+.. _string_view_lifetime:
+
 C++17 string views
 ==================
 
@@ -289,6 +291,30 @@ string type (for example, a ``std::u16string_view`` argument will be passed
 UTF-16-encoded data, and a returned ``std::string_view`` will be decoded as
 UTF-8).
 
+A string view does not own its character data. When a view is loaded as an
+argument to a pybind11-bound function, pybind11 keeps the Python object that
+provides the data alive until the function returns. This also applies to views
+nested in automatically converted STL containers. The C++ function must not
+retain any such view after it returns unless it separately guarantees that the
+backing storage remains alive and valid.
+
+Lifetime support keeps the Python object alive, but does not prevent its storage
+from being invalidated. For example, if C++ releases the GIL or calls back into
+Python, resizing a backing ``bytearray`` while the view is in use can invalidate
+the view.
+
+A direct Python-to-C++ :func:`py::cast` made when no bound-function call is
+active has no such lifetime support. When a cast to a non-owning view succeeds,
+the caller must keep the backing Python object alive, with its storage
+unchanged, for as long as the view is used. For a container of views, this
+requirement applies to every element: retain the elements directly or through
+an unmodified owning container, and do not cast an iterable that creates
+temporary elements.
+
+Some view conversions require temporary backing storage, for example to encode
+text. Outside a bound-function call, such conversions raise
+:class:`cast_error` instead of returning a dangling view.
+
 References
 ==========
 

docs/advanced/pycpp/object.rst

@@ -76,6 +76,10 @@ The reverse direction uses the following syntax:
 
 When conversion fails, both directions throw the exception :class:`cast_error`.
 
+When casting to a non-owning type such as ``std::string_view``, the Python
+source may need to remain alive after a successful cast. See
+:ref:`string_view_lifetime` for details.
+
 .. _python_libs:
 
 Accessing Python libraries from C++

include/pybind11/cast.h

@@ -525,6 +525,11 @@ struct string_caster {
                 return false;
             }
             value = StringType(buffer, static_cast<size_t>(size));
+            if (IsView) {
+                // `src` owns the buffer; keep it alive if inside a bound function,
+                // otherwise the caller is responsible for its lifetime.
+                loader_life_support::try_add_patient(src);
+            }
             return true;
         }
 
@@ -602,6 +607,9 @@ struct string_caster {
                 pybind11_fail("Unexpected PYBIND11_BYTES_AS_STRING() failure.");
             }
             value = StringType(bytes, (size_t) PYBIND11_BYTES_SIZE(src.ptr()));
+            if (IsView) {
+                loader_life_support::try_add_patient(src);
+            }
             return true;
         }
         if (PyByteArray_Check(src.ptr())) {
@@ -612,6 +620,9 @@ struct string_caster {
                 pybind11_fail("Unexpected PyByteArray_AsString() failure.");
             }
             value = StringType(bytearray, (size_t) PyByteArray_Size(src.ptr()));
+            if (IsView) {
+                loader_life_support::try_add_patient(src);
+            }
             return true;
         }
 

include/pybind11/detail/type_caster_base.h

@@ -81,11 +81,26 @@ class loader_life_support {
         }
     }
 
+    /// Keep `h` alive until the current patient frame is destroyed, if there is one.
+    /// Returns false when called outside a bound function (no frame). Use this, rather
+    /// than `add_patient`, when failing to register is acceptable because the caller
+    /// owns the source's lifetime outside the call framework (e.g. a view that points
+    /// into an existing Python object, as opposed to a freshly created temporary).
+    PYBIND11_NOINLINE static bool try_add_patient(handle h) {
+        loader_life_support *frame = tls_current_frame();
+        if (!frame) {
+            return false;
+        }
+        if (frame->keep_alive.insert(h.ptr()).second) {
+            Py_INCREF(h.ptr());
+        }
+        return true;
+    }
+
     /// This can only be used inside a pybind11-bound function, either by `argument_loader`
     /// at argument preparation time or by `py::cast()` at execution time.
     PYBIND11_NOINLINE static void add_patient(handle h) {
-        loader_life_support *frame = tls_current_frame();
-        if (!frame) {
+        if (!try_add_patient(h)) {
             // NOTE: It would be nice to include the stack frames here, as this indicates
             // use of pybind11::cast<> outside the normal call framework, finding such
             // a location is challenging. Developers could consider printing out
@@ -94,10 +109,6 @@ class loader_life_support {
                              "do Python -> C++ conversions which require the creation "
                              "of temporary values");
         }
-
-        if (frame->keep_alive.insert(h.ptr()).second) {
-            Py_INCREF(h.ptr());
-        }
     }
 };
 

tests/test_stl.cpp

@@ -582,6 +582,24 @@ TEST_SUBMODULE(stl, m) {
           [](const std::list<std::string> &) { return 2; });
     m.def("func_with_string_or_vector_string_arg_overload", [](const std::string &) { return 3; });
 
+#ifdef PYBIND11_HAS_STRING_VIEW
+    m.def("func_with_string_views", [](const std::vector<std::string_view> &svs) {
+        py::list l;
+        for (std::string_view sv : svs) {
+            l.append(sv);
+        }
+        return l;
+    });
+    m.def("string_view_life_support_check",
+          [](const std::vector<std::string_view> &, int, const py::list &destroyed) {
+              return destroyed.size();
+          });
+    m.def("nested_string_view_life_support_check",
+          [](const std::vector<std::vector<std::string_view>> &, int, const py::list &destroyed) {
+              return destroyed.size();
+          });
+#endif
+
     class Placeholder {
     public:
         Placeholder() { print_created(this); }

tests/test_stl.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import weakref
+
 import pytest
 
 import env  # noqa: F401
@@ -28,6 +30,116 @@ def test_vector(doc):
     # Test regression caused by 936: pointers to stl containers weren't castable
     assert m.cast_ptr_vector() == ["lvalue", "lvalue"]
 
+    if hasattr(m, "func_with_string_views"):
+
+        def gen():
+            return ("a" + str(x) for x in range(10000, 10010))
+
+        expected = list(gen())
+        assert m.func_with_string_views(gen()) == expected
+        assert m.func_with_string_views(x.encode() for x in gen()) == expected
+        assert (
+            m.func_with_string_views(bytearray(x.encode()) for x in gen()) == expected
+        )
+
+
+@pytest.mark.skipif(
+    not hasattr(m, "string_view_life_support_check"), reason="no <string_view>"
+)
+@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC")
+def test_string_view_life_support_during_argument_conversion():
+    destroyed = []
+
+    class TrackedString(str):
+        pass
+
+    source = [TrackedString("first"), TrackedString("second")]
+    weakrefs = [weakref.ref(item, lambda _: destroyed.append(None)) for item in source]
+
+    # Clear the only Python owners after the views have loaded, but before the
+    # bound function is called.
+    class ClearSourceOnIndex:
+        def __index__(self):
+            source.clear()
+            pytest.gc_collect()
+            return 0
+
+    assert (
+        m.string_view_life_support_check(source, ClearSourceOnIndex(), destroyed) == 0
+    )
+    assert source == []
+    pytest.gc_collect()
+    assert len(destroyed) == 2
+    assert all(ref() is None for ref in weakrefs)
+
+
+@pytest.mark.skipif(
+    not hasattr(m, "string_view_life_support_check"), reason="no <string_view>"
+)
+@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC")
+@pytest.mark.parametrize(
+    ("element_type", "values"),
+    [
+        pytest.param(str, ("first", "second"), id="str"),
+        pytest.param(bytes, (b"first", b"second"), id="bytes"),
+        pytest.param(bytearray, (b"first", b"second"), id="bytearray"),
+    ],
+)
+def test_string_view_life_support_for_generator(element_type, values):
+    destroyed = []
+
+    class Tracked(element_type):
+        def __del__(self):
+            destroyed.append(None)
+
+    def source():
+        for value in values:
+            yield Tracked(value)
+
+    class CollectGarbageOnIndex:
+        def __index__(self):
+            pytest.gc_collect()
+            return 0
+
+    assert (
+        m.string_view_life_support_check(source(), CollectGarbageOnIndex(), destroyed)
+        == 0
+    )
+    pytest.gc_collect()
+    assert len(destroyed) == len(values)
+
+
+@pytest.mark.skipif(
+    not hasattr(m, "nested_string_view_life_support_check"),
+    reason="no <string_view>",
+)
+@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC")
+def test_string_view_life_support_for_nested_containers():
+    destroyed = []
+
+    class TrackedString(str):
+        def __del__(self):
+            destroyed.append(None)
+
+    source = [
+        [TrackedString("first"), TrackedString("second")],
+        [TrackedString("third"), TrackedString("fourth")],
+    ]
+
+    class ClearSourceOnIndex:
+        def __index__(self):
+            source.clear()
+            pytest.gc_collect()
+            return 0
+
+    assert (
+        m.nested_string_view_life_support_check(source, ClearSourceOnIndex(), destroyed)
+        == 0
+    )
+    assert source == []
+    pytest.gc_collect()
+    assert len(destroyed) == 4
+
 
 def test_deque():
     """std::deque <-> list"""

tests/test_with_catch/test_interpreter.cpp

@@ -509,3 +509,32 @@ TEST_CASE("make_iterator can be called before then after finalizing an interpret
 
     py::initialize_interpreter();
 }
+
+#ifdef PYBIND11_HAS_STRING_VIEW
+TEST_CASE("Casting to a string_view outside a bound function") {
+    // Regression for PR #6092: view casters add the source to loader_life_support, but
+    // outside a bound function there is no frame. The caller owns the source's lifetime
+    // here, so the cast must succeed rather than throw.
+    py::str unicode("hello");
+    py::bytes bytes_obj("world", 5);
+    auto bytearray_obj
+        = py::reinterpret_steal<py::object>(PyByteArray_FromStringAndSize("bytes", 5));
+
+    REQUIRE(py::cast<std::string_view>(unicode) == "hello");
+    REQUIRE(py::cast<std::string_view>(bytes_obj) == "world");
+    REQUIRE(py::cast<std::string_view>(bytearray_obj) == "bytes");
+
+    // Wide string views require an encoded temporary. With no loader life-support
+    // frame, returning a view into that temporary must fail.
+    REQUIRE_THROWS_AS(py::cast<std::u16string_view>(unicode), py::cast_error);
+    REQUIRE_THROWS_AS(py::cast<std::u32string_view>(unicode), py::cast_error);
+
+    // Bound-function dispatch provides a frame that keeps both temporaries alive.
+    auto accepts_wide_views
+        = py::cpp_function([](std::u16string_view value16, std::u32string_view value32) {
+              return value16 == std::u16string_view(u"hello")
+                     && value32 == std::u32string_view(U"hello");
+          });
+    REQUIRE(accepts_wide_views(unicode, unicode).cast<bool>());
+}
+#endif