From 2cbc975a2f202c3fd21b667dce66c7104a363285 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Mon, 12 May 2025 11:40:42 +0800
Subject: [PATCH 01/11] Add tests for accessing uninitialized holders

---
 tests/CMakeLists.txt                 |  1 +
 tests/pybind11_tests.cpp             | 14 +++++
 tests/test_invalid_holder_access.cpp | 93 ++++++++++++++++++++++++++++
 tests/test_invalid_holder_access.py  | 71 +++++++++++++++++++++
 4 files changed, 179 insertions(+)
 create mode 100644 tests/test_invalid_holder_access.cpp
 create mode 100644 tests/test_invalid_holder_access.py

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 69976f745a..9ce5099281 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -148,6 +148,7 @@ set(PYBIND11_TEST_FILES
     test_exceptions
     test_factory_constructors
     test_gil_scoped
+    test_invalid_holder_access
     test_iostream
     test_kwargs_and_defaults
     test_local_bindings
diff --git a/tests/pybind11_tests.cpp b/tests/pybind11_tests.cpp
index 0b92d15871..70f54de341 100644
--- a/tests/pybind11_tests.cpp
+++ b/tests/pybind11_tests.cpp
@@ -75,6 +75,19 @@ const char *cpp_std() {
 #endif
 }
 
+int cpp_std_num() {
+    return
+#if defined(PYBIND11_CPP20)
+        20;
+#elif defined(PYBIND11_CPP17)
+        17;
+#elif defined(PYBIND11_CPP14)
+        14;
+#else
+        11;
+#endif
+}
+
 PYBIND11_MODULE(pybind11_tests, m, py::mod_gil_not_used()) {
     m.doc() = "pybind11 test module";
 
@@ -88,6 +101,7 @@ PYBIND11_MODULE(pybind11_tests, m, py::mod_gil_not_used()) {
     m.attr("compiler_info") = py::none();
 #endif
     m.attr("cpp_std") = cpp_std();
+    m.attr("cpp_std_num") = cpp_std_num();
     m.attr("PYBIND11_INTERNALS_ID") = PYBIND11_INTERNALS_ID;
     // Free threaded Python uses UINT32_MAX for immortal objects.
     m.attr("PYBIND11_REFCNT_IMMORTAL") = UINT32_MAX;
diff --git a/tests/test_invalid_holder_access.cpp b/tests/test_invalid_holder_access.cpp
new file mode 100644
index 0000000000..da85a8f3c8
--- /dev/null
+++ b/tests/test_invalid_holder_access.cpp
@@ -0,0 +1,93 @@
+#include "pybind11_tests.h"
+
+#include <Python.h>
+#include <memory>
+#include <vector>
+
+class VectorOwns4PythonObjects {
+public:
+    void append(const py::object &obj) {
+        if (size() >= 4) {
+            throw std::out_of_range("Index out of range");
+        }
+        vec.emplace_back(obj);
+    }
+
+    void set_item(py::ssize_t i, const py::object &obj) {
+        if (!(i >= 0 && i < size())) {
+            throw std::out_of_range("Index out of range");
+        }
+        vec[py::size_t(i)] = obj;
+    }
+
+    py::object get_item(py::ssize_t i) const {
+        if (!(i >= 0 && i < size())) {
+            throw std::out_of_range("Index out of range");
+        }
+        return vec[py::size_t(i)];
+    }
+
+    py::ssize_t size() const { return py::ssize_t_cast(vec.size()); }
+
+    bool is_empty() const { return vec.empty(); }
+
+    void sanity_check() const {
+        auto current_size = size();
+        if (current_size < 0 || current_size > 4) {
+            throw std::out_of_range("Invalid size");
+        }
+    }
+
+    static int tp_traverse(PyObject *self_base, visitproc visit, void *arg) {
+#if PY_VERSION_HEX >= 0x03090000 // Python 3.9
+        Py_VISIT(Py_TYPE(self_base));
+#endif
+        auto *const instance = reinterpret_cast<py::detail::instance *>(self_base);
+        if (!instance->get_value_and_holder().holder_constructed()) {
+            // The holder has not been constructed yet. Skip the traversal to avoid segmentation
+            // faults.
+            return 0;
+        }
+        auto &self = py::cast<VectorOwns4PythonObjects &>(py::handle{self_base});
+        for (const auto &obj : self.vec) {
+            Py_VISIT(obj.ptr());
+        }
+        return 0;
+    }
+
+private:
+    std::vector<py::object> vec{};
+};
+
+TEST_SUBMODULE(invalid_holder_access, m) {
+    m.doc() = "Test invalid holder access";
+
+#if defined(PYBIND11_CPP14)
+    m.def("create_vector", []() -> std::unique_ptr<VectorOwns4PythonObjects> {
+        auto vec = std::make_unique<VectorOwns4PythonObjects>();
+        vec->append(py::none());
+        vec->append(py::int_(1));
+        vec->append(py::str("test"));
+        vec->append(py::tuple());
+        return vec;
+    });
+#endif
+
+    py::class_<VectorOwns4PythonObjects>(
+        m,
+        "VectorOwns4PythonObjects",
+        py::custom_type_setup([](PyHeapTypeObject *heap_type) -> void {
+            auto *const type = &heap_type->ht_type;
+            type->tp_flags |= Py_TPFLAGS_HAVE_GC;
+            type->tp_traverse = &VectorOwns4PythonObjects::tp_traverse;
+        }))
+        .def("append", &VectorOwns4PythonObjects::append, py::arg("obj"))
+        .def("set_item", &VectorOwns4PythonObjects::set_item, py::arg("i"), py::arg("obj"))
+        .def("get_item", &VectorOwns4PythonObjects::get_item, py::arg("i"))
+        .def("size", &VectorOwns4PythonObjects::size)
+        .def("is_empty", &VectorOwns4PythonObjects::is_empty)
+        .def("__setitem__", &VectorOwns4PythonObjects::set_item, py::arg("i"), py::arg("obj"))
+        .def("__getitem__", &VectorOwns4PythonObjects::get_item, py::arg("i"))
+        .def("__len__", &VectorOwns4PythonObjects::size)
+        .def("sanity_check", &VectorOwns4PythonObjects::sanity_check);
+}
diff --git a/tests/test_invalid_holder_access.py b/tests/test_invalid_holder_access.py
new file mode 100644
index 0000000000..aec1860c8d
--- /dev/null
+++ b/tests/test_invalid_holder_access.py
@@ -0,0 +1,71 @@
+from __future__ import annotations
+
+import gc
+import weakref
+
+import pytest
+
+import pybind11_tests
+from pybind11_tests import invalid_holder_access as m
+
+XFAIL_REASON = "Known issues: https://github.com/pybind/pybind11/pull/5654"
+
+
+@pytest.mark.skipif(
+    pybind11_tests.cpp_std_num < 14,
+    reason="std::{unique_ptr,make_unique} not available in C++11",
+)
+def test_create_vector():
+    vec = m.create_vector()
+    assert vec.size() == 4
+    assert not vec.is_empty()
+    assert vec[0] is None
+    assert vec[1] == 1
+    assert vec[2] == "test"
+    assert vec[3] == ()
+
+
+def test_no_init():
+    with pytest.raises(TypeError, match=r"No constructor defined"):
+        m.VectorOwns4PythonObjects()
+    vec = m.VectorOwns4PythonObjects.__new__(m.VectorOwns4PythonObjects)
+    with pytest.raises(TypeError, match=r"No constructor defined"):
+        vec.__init__()
+
+
+# The test might succeed on some platforms with some compilers, but
+# it is not guaranteed to work everywhere. It is marked as xfail.
+@pytest.mark.xfail(reason=XFAIL_REASON, raises=SystemError, strict=False)
+def test_manual_new():
+    # Repeatedly trigger allocation without initialization (raw malloc'ed) to
+    # detect uninitialized memory bugs.
+    for _ in range(32):
+        # The holder is a pointer variable while the C++ ctor is not called.
+        vec = m.VectorOwns4PythonObjects.__new__(m.VectorOwns4PythonObjects)
+        if vec.is_empty():
+            # The C++ compiler initializes container correctly.
+            assert vec.size() == 0
+        else:
+            raise SystemError(
+                "Segmentation Fault: The C++ compiler initializes container incorrectly."
+            )
+        vec.append(1)
+        vec.append(2)
+        vec.append(3)
+        vec.append(4)
+
+
+@pytest.mark.skipif(
+    pybind11_tests.cpp_std_num < 14,
+    reason="std::{unique_ptr,make_unique} not available in C++11",
+)
+def test_gc_traverse():
+    vec = m.create_vector()
+    vec[3] = (vec, vec)
+
+    wr = weakref.ref(vec)
+    assert wr() is vec
+    del vec
+    for _ in range(10):
+        gc.collect()
+    assert wr() is None

From edcb5ac0c652dc9e78f4b0d7569309529e98b813 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Mon, 12 May 2025 14:29:10 +0800
Subject: [PATCH 02/11] Enable `faulthandler` for pytest

---
 tests/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 9ce5099281..5acf775a34 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -557,7 +557,7 @@ set(PYBIND11_PYTEST_ARGS
 # A single command to compile and run the tests
 add_custom_target(
   pytest
-  COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -m pytest
+  COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -X faulthandler -m pytest
           ${PYBIND11_ABS_PYTEST_FILES} ${PYBIND11_PYTEST_ARGS}
   DEPENDS ${test_targets}
   WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"

From 4bc7a11542ee4bed9b4190d3aa2941526c67ec00 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Mon, 12 May 2025 14:43:34 +0800
Subject: [PATCH 03/11] Disable GC test for non-CPython

---
 tests/test_invalid_holder_access.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/test_invalid_holder_access.py b/tests/test_invalid_holder_access.py
index aec1860c8d..11b3124db3 100644
--- a/tests/test_invalid_holder_access.py
+++ b/tests/test_invalid_holder_access.py
@@ -5,6 +5,7 @@
 
 import pytest
 
+import env  # noqa: F401
 import pybind11_tests
 from pybind11_tests import invalid_holder_access as m
 
@@ -55,6 +56,7 @@ def test_manual_new():
         vec.append(4)
 
 
+@pytest.mark.skipif("env.PYPY or env.GRAALPY", reason="Cannot reliably trigger GC")
 @pytest.mark.skipif(
     pybind11_tests.cpp_std_num < 14,
     reason="std::{unique_ptr,make_unique} not available in C++11",

From 20d23e20e7713609054d7a185deda688cd25fce1 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Mon, 12 May 2025 15:11:40 +0800
Subject: [PATCH 04/11] Move segfault test to subprocess

---
 tests/test_invalid_holder_access.py | 31 ++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/tests/test_invalid_holder_access.py b/tests/test_invalid_holder_access.py
index 11b3124db3..6e026ff950 100644
--- a/tests/test_invalid_holder_access.py
+++ b/tests/test_invalid_holder_access.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import gc
+import multiprocessing
 import weakref
 
 import pytest
@@ -12,6 +13,12 @@
 XFAIL_REASON = "Known issues: https://github.com/pybind/pybind11/pull/5654"
 
 
+try:
+    spawn_context = multiprocessing.get_context("spawn")
+except ValueError:
+    spawn_context = None
+
+
 @pytest.mark.skipif(
     pybind11_tests.cpp_std_num < 14,
     reason="std::{unique_ptr,make_unique} not available in C++11",
@@ -34,16 +41,13 @@ def test_no_init():
         vec.__init__()
 
 
-# The test might succeed on some platforms with some compilers, but
-# it is not guaranteed to work everywhere. It is marked as xfail.
-@pytest.mark.xfail(reason=XFAIL_REASON, raises=SystemError, strict=False)
-def test_manual_new():
+def manual_new_target():
     # Repeatedly trigger allocation without initialization (raw malloc'ed) to
     # detect uninitialized memory bugs.
     for _ in range(32):
         # The holder is a pointer variable while the C++ ctor is not called.
         vec = m.VectorOwns4PythonObjects.__new__(m.VectorOwns4PythonObjects)
-        if vec.is_empty():
+        if vec.is_empty():  # <= this line could cause a segfault
             # The C++ compiler initializes container correctly.
             assert vec.size() == 0
         else:
@@ -56,6 +60,23 @@ def test_manual_new():
         vec.append(4)
 
 
+# This test might succeed on some platforms with some compilers, but it is not
+# guaranteed to work everywhere. It is marked as non-strict xfail.
+@pytest.mark.xfail(reason=XFAIL_REASON, raises=SystemError, strict=False)
+@pytest.mark.skipif(spawn_context is None, reason="spawn context not available")
+def test_manual_new():
+    process = spawn_context.Process(
+        target=manual_new_target,
+        name="manual_new_target",
+    )
+    process.start()
+    process.join()
+    if process.exitcode != 0:
+        raise SystemError(
+            "Segmentation Fault: The C++ compiler initializes container incorrectly."
+        )
+
+
 @pytest.mark.skipif("env.PYPY or env.GRAALPY", reason="Cannot reliably trigger GC")
 @pytest.mark.skipif(
     pybind11_tests.cpp_std_num < 14,

From 2b0062129e936daf11eb4967cd09d227c1083e61 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Mon, 12 May 2025 15:56:50 +0800
Subject: [PATCH 05/11] Refine tests

---
 tests/test_invalid_holder_access.py | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tests/test_invalid_holder_access.py b/tests/test_invalid_holder_access.py
index 6e026ff950..e268f6d613 100644
--- a/tests/test_invalid_holder_access.py
+++ b/tests/test_invalid_holder_access.py
@@ -2,6 +2,8 @@
 
 import gc
 import multiprocessing
+import signal
+import sys
 import weakref
 
 import pytest
@@ -14,8 +16,10 @@
 
 
 try:
+    import multiprocessing
+
     spawn_context = multiprocessing.get_context("spawn")
-except ValueError:
+except (ImportError, ValueError):
     spawn_context = None
 
 
@@ -51,9 +55,9 @@ def manual_new_target():
             # The C++ compiler initializes container correctly.
             assert vec.size() == 0
         else:
-            raise SystemError(
-                "Segmentation Fault: The C++ compiler initializes container incorrectly."
-            )
+            # The program is not supposed to reach here. It will abort with
+            # SIGSEGV on `vec.is_empty()`.
+            sys.exit(signal.SIGSEGV)  # manually trigger SIGSEGV if not raised
         vec.append(1)
         vec.append(2)
         vec.append(3)
@@ -64,6 +68,10 @@ def manual_new_target():
 # guaranteed to work everywhere. It is marked as non-strict xfail.
 @pytest.mark.xfail(reason=XFAIL_REASON, raises=SystemError, strict=False)
 @pytest.mark.skipif(spawn_context is None, reason="spawn context not available")
+@pytest.mark.skipif(
+    sys.platform.startswith("emscripten"),
+    reason="Requires multiprocessing",
+)
 def test_manual_new():
     process = spawn_context.Process(
         target=manual_new_target,
@@ -71,6 +79,7 @@ def test_manual_new():
     )
     process.start()
     process.join()
+    assert abs(process.exitcode) in (0, signal.SIGSEGV, signal.SIGABRT)
     if process.exitcode != 0:
         raise SystemError(
             "Segmentation Fault: The C++ compiler initializes container incorrectly."

From 6c6db0e277bdeabfe5deabd789723d1dd679df6e Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Mon, 12 May 2025 17:10:04 +0800
Subject: [PATCH 06/11] Add `STATUS_ACCESS_VIOLATION` for Windows exitcode

---
 tests/test_invalid_holder_access.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/test_invalid_holder_access.py b/tests/test_invalid_holder_access.py
index e268f6d613..c4292b5263 100644
--- a/tests/test_invalid_holder_access.py
+++ b/tests/test_invalid_holder_access.py
@@ -79,8 +79,16 @@ def test_manual_new():
     )
     process.start()
     process.join()
-    assert abs(process.exitcode) in (0, signal.SIGSEGV, signal.SIGABRT)
-    if process.exitcode != 0:
+    rc = abs(process.exitcode)
+    if 128 < rc < 256:
+        rc -= 128
+    assert rc in (
+        0,
+        signal.SIGSEGV,
+        signal.SIGABRT,
+        0xC0000005,  # STATUS_ACCESS_VIOLATION on Windows
+    )
+    if rc != 0:
         raise SystemError(
             "Segmentation Fault: The C++ compiler initializes container incorrectly."
         )

From 2c73d2bb67a4ff7268a3045b4e8bfa44a47244b1 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 14 May 2025 01:39:14 +0800
Subject: [PATCH 07/11] Update tests

---
 tests/test_invalid_holder_access.cpp | 132 +++++++++++++++-------
 tests/test_invalid_holder_access.py  | 160 ++++++++++++++++++++-------
 2 files changed, 210 insertions(+), 82 deletions(-)

diff --git a/tests/test_invalid_holder_access.cpp b/tests/test_invalid_holder_access.cpp
index da85a8f3c8..ed83163a1d 100644
--- a/tests/test_invalid_holder_access.cpp
+++ b/tests/test_invalid_holder_access.cpp
@@ -4,14 +4,9 @@
 #include <memory>
 #include <vector>
 
-class VectorOwns4PythonObjects {
+class VecOwnsObjs {
 public:
-    void append(const py::object &obj) {
-        if (size() >= 4) {
-            throw std::out_of_range("Index out of range");
-        }
-        vec.emplace_back(obj);
-    }
+    void append(const py::object &obj) { vec.emplace_back(obj); }
 
     void set_item(py::ssize_t i, const py::object &obj) {
         if (!(i >= 0 && i < size())) {
@@ -31,63 +26,120 @@ class VectorOwns4PythonObjects {
 
     bool is_empty() const { return vec.empty(); }
 
-    void sanity_check() const {
-        auto current_size = size();
-        if (current_size < 0 || current_size > 4) {
-            throw std::out_of_range("Invalid size");
-        }
-    }
-
     static int tp_traverse(PyObject *self_base, visitproc visit, void *arg) {
 #if PY_VERSION_HEX >= 0x03090000 // Python 3.9
         Py_VISIT(Py_TYPE(self_base));
 #endif
-        auto *const instance = reinterpret_cast<py::detail::instance *>(self_base);
-        if (!instance->get_value_and_holder().holder_constructed()) {
-            // The holder has not been constructed yet. Skip the traversal to avoid segmentation
-            // faults.
-            return 0;
+        if (should_check_holder_initialization) {
+            auto *const instance = reinterpret_cast<py::detail::instance *>(self_base);
+            if (!instance->get_value_and_holder().holder_constructed()) {
+                // The holder has not been constructed yet. Skip the traversal to avoid
+                // segmentation faults.
+                return 0;
+            }
         }
-        auto &self = py::cast<VectorOwns4PythonObjects &>(py::handle{self_base});
+        auto &self = py::cast<VecOwnsObjs &>(py::handle{self_base});
         for (const auto &obj : self.vec) {
             Py_VISIT(obj.ptr());
         }
         return 0;
     }
 
+    static int tp_clear(PyObject *self_base) {
+        if (should_check_holder_initialization) {
+            auto *const instance = reinterpret_cast<py::detail::instance *>(self_base);
+            if (!instance->get_value_and_holder().holder_constructed()) {
+                // The holder has not been constructed yet. Skip the traversal to avoid
+                // segmentation faults.
+                return 0;
+            }
+        }
+        auto &self = py::cast<VecOwnsObjs &>(py::handle{self_base});
+        for (auto &obj : self.vec) {
+            Py_CLEAR(obj.ptr());
+        }
+        self.vec.clear();
+        return 0;
+    }
+
+    py::object get_state() const {
+        py::list state{};
+        for (const auto &item : vec) {
+            state.append(item);
+        }
+        return py::tuple(state);
+    }
+
+    static bool get_should_check_holder_initialization() {
+        return should_check_holder_initialization;
+    }
+
+    static void set_should_check_holder_initialization(bool value) {
+        should_check_holder_initialization = value;
+    }
+
+    static bool get_should_raise_error_on_set_state() { return should_raise_error_on_set_state; }
+
+    static void set_should_raise_error_on_set_state(bool value) {
+        should_raise_error_on_set_state = value;
+    }
+
+    static bool should_check_holder_initialization;
+    static bool should_raise_error_on_set_state;
+
 private:
     std::vector<py::object> vec{};
 };
 
+bool VecOwnsObjs::should_check_holder_initialization = false;
+bool VecOwnsObjs::should_raise_error_on_set_state = false;
+
 TEST_SUBMODULE(invalid_holder_access, m) {
     m.doc() = "Test invalid holder access";
 
 #if defined(PYBIND11_CPP14)
-    m.def("create_vector", []() -> std::unique_ptr<VectorOwns4PythonObjects> {
-        auto vec = std::make_unique<VectorOwns4PythonObjects>();
-        vec->append(py::none());
-        vec->append(py::int_(1));
-        vec->append(py::str("test"));
-        vec->append(py::tuple());
+    m.def("create_vector", [](const py::iterable &iterable) -> std::unique_ptr<VecOwnsObjs> {
+        auto vec = std::make_unique<VecOwnsObjs>();
+        for (const auto &item : iterable) {
+            vec->append(py::reinterpret_borrow<py::object>(item));
+        }
         return vec;
     });
 #endif
 
-    py::class_<VectorOwns4PythonObjects>(
-        m,
-        "VectorOwns4PythonObjects",
-        py::custom_type_setup([](PyHeapTypeObject *heap_type) -> void {
+    py::class_<VecOwnsObjs>(
+        m, "VecOwnsObjs", py::custom_type_setup([](PyHeapTypeObject *heap_type) -> void {
             auto *const type = &heap_type->ht_type;
             type->tp_flags |= Py_TPFLAGS_HAVE_GC;
-            type->tp_traverse = &VectorOwns4PythonObjects::tp_traverse;
+            type->tp_traverse = &VecOwnsObjs::tp_traverse;
+            type->tp_clear = &VecOwnsObjs::tp_clear;
         }))
-        .def("append", &VectorOwns4PythonObjects::append, py::arg("obj"))
-        .def("set_item", &VectorOwns4PythonObjects::set_item, py::arg("i"), py::arg("obj"))
-        .def("get_item", &VectorOwns4PythonObjects::get_item, py::arg("i"))
-        .def("size", &VectorOwns4PythonObjects::size)
-        .def("is_empty", &VectorOwns4PythonObjects::is_empty)
-        .def("__setitem__", &VectorOwns4PythonObjects::set_item, py::arg("i"), py::arg("obj"))
-        .def("__getitem__", &VectorOwns4PythonObjects::get_item, py::arg("i"))
-        .def("__len__", &VectorOwns4PythonObjects::size)
-        .def("sanity_check", &VectorOwns4PythonObjects::sanity_check);
+        .def_static("set_should_check_holder_initialization",
+                    &VecOwnsObjs::set_should_check_holder_initialization,
+                    py::arg("value"))
+        .def_static("set_should_raise_error_on_set_state",
+                    &VecOwnsObjs::set_should_raise_error_on_set_state,
+                    py::arg("value"))
+#if defined(PYBIND11_CPP14)
+        .def(py::pickle([](const VecOwnsObjs &self) -> py::object { return self.get_state(); },
+                        [](const py::object &state) -> std::unique_ptr<VecOwnsObjs> {
+                            if (!py::isinstance<py::tuple>(state)) {
+                                throw std::runtime_error("Invalid state");
+                            }
+                            auto vec = std::make_unique<VecOwnsObjs>();
+                            if (VecOwnsObjs::get_should_raise_error_on_set_state()) {
+                                throw std::runtime_error("raise error on set_state for testing");
+                            }
+                            for (const auto &item : state) {
+                                vec->append(py::reinterpret_borrow<py::object>(item));
+                            }
+                            return vec;
+                        }),
+             py::arg("state"))
+#endif
+        .def("append", &VecOwnsObjs::append, py::arg("obj"))
+        .def("is_empty", &VecOwnsObjs::is_empty)
+        .def("__setitem__", &VecOwnsObjs::set_item, py::arg("i"), py::arg("obj"))
+        .def("__getitem__", &VecOwnsObjs::get_item, py::arg("i"))
+        .def("__len__", &VecOwnsObjs::size);
 }
diff --git a/tests/test_invalid_holder_access.py b/tests/test_invalid_holder_access.py
index c4292b5263..d92c5efdb9 100644
--- a/tests/test_invalid_holder_access.py
+++ b/tests/test_invalid_holder_access.py
@@ -2,6 +2,7 @@
 
 import gc
 import multiprocessing
+import pickle
 import signal
 import sys
 import weakref
@@ -23,24 +24,34 @@
     spawn_context = None
 
 
-@pytest.mark.skipif(
-    pybind11_tests.cpp_std_num < 14,
-    reason="std::{unique_ptr,make_unique} not available in C++11",
-)
-def test_create_vector():
-    vec = m.create_vector()
-    assert vec.size() == 4
-    assert not vec.is_empty()
-    assert vec[0] is None
-    assert vec[1] == 1
-    assert vec[2] == "test"
-    assert vec[3] == ()
+def check_segfault(target):
+    """Check if the target function causes a segmentation fault.
+
+    The check should be done in a separate process to avoid crashing the main
+    process with the segfault.
+    """
+    process = spawn_context.Process(target=target)
+    process.start()
+    process.join()
+    rc = abs(process.exitcode)
+    if 128 < rc < 256:
+        rc -= 128
+    assert rc in (
+        0,
+        signal.SIGSEGV,
+        signal.SIGABRT,
+        0xC0000005,  # STATUS_ACCESS_VIOLATION on Windows
+    )
+    if rc != 0:
+        raise SystemError(
+            "Segmentation Fault: The C++ compiler initializes container incorrectly."
+        )
 
 
 def test_no_init():
     with pytest.raises(TypeError, match=r"No constructor defined"):
-        m.VectorOwns4PythonObjects()
-    vec = m.VectorOwns4PythonObjects.__new__(m.VectorOwns4PythonObjects)
+        m.VecOwnsObjs()
+    vec = m.VecOwnsObjs.__new__(m.VecOwnsObjs)
     with pytest.raises(TypeError, match=r"No constructor defined"):
         vec.__init__()
 
@@ -50,18 +61,14 @@ def manual_new_target():
     # detect uninitialized memory bugs.
     for _ in range(32):
         # The holder is a pointer variable while the C++ ctor is not called.
-        vec = m.VectorOwns4PythonObjects.__new__(m.VectorOwns4PythonObjects)
+        vec = m.VecOwnsObjs.__new__(m.VecOwnsObjs)
         if vec.is_empty():  # <= this line could cause a segfault
             # The C++ compiler initializes container correctly.
-            assert vec.size() == 0
+            assert len(vec) == 0
         else:
             # The program is not supposed to reach here. It will abort with
             # SIGSEGV on `vec.is_empty()`.
             sys.exit(signal.SIGSEGV)  # manually trigger SIGSEGV if not raised
-        vec.append(1)
-        vec.append(2)
-        vec.append(3)
-        vec.append(4)
 
 
 # This test might succeed on some platforms with some compilers, but it is not
@@ -73,25 +80,94 @@ def manual_new_target():
     reason="Requires multiprocessing",
 )
 def test_manual_new():
-    process = spawn_context.Process(
-        target=manual_new_target,
-        name="manual_new_target",
-    )
-    process.start()
-    process.join()
-    rc = abs(process.exitcode)
-    if 128 < rc < 256:
-        rc -= 128
-    assert rc in (
-        0,
-        signal.SIGSEGV,
-        signal.SIGABRT,
-        0xC0000005,  # STATUS_ACCESS_VIOLATION on Windows
-    )
-    if rc != 0:
-        raise SystemError(
-            "Segmentation Fault: The C++ compiler initializes container incorrectly."
-        )
+    """
+    Manually calling the constructor (__new__) of a class that has C++ STL
+    container will allocate memory without initializing it can cause a
+    segmentation fault.
+    """
+    check_segfault(manual_new_target)
+
+
+@pytest.mark.skipif(
+    pybind11_tests.cpp_std_num < 14,
+    reason="std::{unique_ptr,make_unique} not available in C++11",
+)
+def test_set_state_with_error_no_segfault_if_gc_checks_holder_has_initialized():
+    """
+    The ``tp_traverse`` and ``tp_clear`` functions should check if the holder
+    has been initialized before traversing or clearing the C++ STL container.
+    """
+    m.VecOwnsObjs.set_should_check_holder_initialization(True)
+
+    vec = m.create_vector((1, 2, 3, 4))
+
+    m.VecOwnsObjs.set_should_raise_error_on_set_state(False)
+    pickle.loads(pickle.dumps(vec))
+
+    # During the unpickling process, Python firstly allocates the object with
+    # the `__new__` method and then calls the `__setstate__`` method to set the
+    # state of the object.
+    #
+    #     obj = cls.__new__(cls)
+    #     obj.__setstate__(state)
+    #
+    # The `__init__` method is not called during unpickling.
+    # So, if the `__setstate__` method raises an error, the object will be in
+    # an uninitialized state.
+    m.VecOwnsObjs.set_should_raise_error_on_set_state(True)
+    serialized = pickle.dumps(vec)
+    with pytest.raises(
+        RuntimeError,
+        match="raise error on set_state for testing",
+    ):
+        pickle.loads(serialized)
+
+
+def unpicklable_with_error_target():
+    m.VecOwnsObjs.set_should_check_holder_initialization(False)
+    m.VecOwnsObjs.set_should_raise_error_on_set_state(True)
+
+    vec = m.create_vector((1, 2, 3, 4))
+    serialized = pickle.dumps(vec)
+
+    # Repeatedly trigger allocation without initialization (raw malloc'ed) to
+    # detect uninitialized memory bugs.
+    for _ in range(32):
+        # During the unpickling process, Python firstly allocates the object with
+        # the `__new__` method and then calls the `__setstate__`` method to set the
+        # state of the object.
+        #
+        #     obj = cls.__new__(cls)
+        #     obj.__setstate__(state)
+        #
+        # The `__init__` method is not called during unpickling.
+        # So, if the `__setstate__` method raises an error, the object will be in
+        # an uninitialized state. The GC will traverse the uninitialized C++ STL
+        # container and cause a segmentation fault.
+        try:  # noqa: SIM105
+            pickle.loads(serialized)
+        except RuntimeError:
+            pass
+
+
+# This test might succeed on some platforms with some compilers, but it is not
+# guaranteed to work everywhere. It is marked as non-strict xfail.
+@pytest.mark.xfail(reason=XFAIL_REASON, raises=SystemError, strict=False)
+@pytest.mark.skipif(spawn_context is None, reason="spawn context not available")
+@pytest.mark.skipif(
+    pybind11_tests.cpp_std_num < 14,
+    reason="std::{unique_ptr,make_unique} not available in C++11",
+)
+def test_set_state_with_error_will_segfault_if_gc_does_not_check_holder_has_initialized():
+    m.VecOwnsObjs.set_should_check_holder_initialization(False)
+
+    vec = m.create_vector((1, 2, 3, 4))
+
+    m.VecOwnsObjs.set_should_raise_error_on_set_state(False)
+    pickle.loads(pickle.dumps(vec))
+
+    # See comments above.
+    check_segfault(unpicklable_with_error_target)
 
 
 @pytest.mark.skipif("env.PYPY or env.GRAALPY", reason="Cannot reliably trigger GC")
@@ -99,9 +175,9 @@ def test_manual_new():
     pybind11_tests.cpp_std_num < 14,
     reason="std::{unique_ptr,make_unique} not available in C++11",
 )
-def test_gc_traverse():
-    vec = m.create_vector()
-    vec[3] = (vec, vec)
+def test_gc():
+    vec = m.create_vector(())
+    vec.append((vec, vec))
 
     wr = weakref.ref(vec)
     assert wr() is vec

From d103e2603f0b8813b8e40fd05cb8dfba1f922fb8 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 14 May 2025 01:50:54 +0800
Subject: [PATCH 08/11] Update docs

---
 docs/advanced/classes.rst            | 21 +++++++++++++++++++++
 tests/test_invalid_holder_access.cpp |  6 ++++++
 tests/test_invalid_holder_access.py  |  2 ++
 3 files changed, 29 insertions(+)

diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst
index 7c0ec10e79..8e8b15b9fc 100644
--- a/docs/advanced/classes.rst
+++ b/docs/advanced/classes.rst
@@ -1339,11 +1339,32 @@ You can do that using ``py::custom_type_setup``:
            auto *type = &heap_type->ht_type;
            type->tp_flags |= Py_TPFLAGS_HAVE_GC;
            type->tp_traverse = [](PyObject *self_base, visitproc visit, void *arg) {
+               // https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_traverse
+   #if PY_VERSION_HEX >= 0x03090000  // Python 3.9
+               Py_VISIT(Py_TYPE(self_base));
+   #endif
+
+               auto* const instance = reinterpret_cast<py::detail::instance*>(self_base);
+               if (!instance->get_value_and_holder().holder_constructed()) [[unlikely]] {
+                   // The holder has not been constructed yet.
+                   // Skip the traversal to avoid segmentation faults.
+                   return 0;
+               }
+
+               // The actual logic of the tp_clear function goes here.
                auto &self = py::cast<OwnsPythonObjects&>(py::handle(self_base));
                Py_VISIT(self.value.ptr());
                return 0;
            };
            type->tp_clear = [](PyObject *self_base) {
+               auto* const instance = reinterpret_cast<py::detail::instance*>(self_base);
+               if (!instance->get_value_and_holder().holder_constructed()) [[unlikely]] {
+                   // The holder has not been constructed yet.
+                   // Skip the traversal to avoid segmentation faults.
+                   return 0;
+               }
+
+               // The actual logic of the tp_clear function goes here.
                auto &self = py::cast<OwnsPythonObjects&>(py::handle(self_base));
                self.value = py::none();
                return 0;
diff --git a/tests/test_invalid_holder_access.cpp b/tests/test_invalid_holder_access.cpp
index ed83163a1d..a79f8af98b 100644
--- a/tests/test_invalid_holder_access.cpp
+++ b/tests/test_invalid_holder_access.cpp
@@ -27,9 +27,11 @@ class VecOwnsObjs {
     bool is_empty() const { return vec.empty(); }
 
     static int tp_traverse(PyObject *self_base, visitproc visit, void *arg) {
+        // https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_traverse
 #if PY_VERSION_HEX >= 0x03090000 // Python 3.9
         Py_VISIT(Py_TYPE(self_base));
 #endif
+
         if (should_check_holder_initialization) {
             auto *const instance = reinterpret_cast<py::detail::instance *>(self_base);
             if (!instance->get_value_and_holder().holder_constructed()) {
@@ -38,6 +40,8 @@ class VecOwnsObjs {
                 return 0;
             }
         }
+
+        // The actual logic of the tp_traverse function goes here.
         auto &self = py::cast<VecOwnsObjs &>(py::handle{self_base});
         for (const auto &obj : self.vec) {
             Py_VISIT(obj.ptr());
@@ -54,6 +58,8 @@ class VecOwnsObjs {
                 return 0;
             }
         }
+
+        // The actual logic of the tp_clear function goes here.
         auto &self = py::cast<VecOwnsObjs &>(py::handle{self_base});
         for (auto &obj : self.vec) {
             Py_CLEAR(obj.ptr());
diff --git a/tests/test_invalid_holder_access.py b/tests/test_invalid_holder_access.py
index d92c5efdb9..593435f855 100644
--- a/tests/test_invalid_holder_access.py
+++ b/tests/test_invalid_holder_access.py
@@ -114,6 +114,8 @@ def test_set_state_with_error_no_segfault_if_gc_checks_holder_has_initialized():
     # The `__init__` method is not called during unpickling.
     # So, if the `__setstate__` method raises an error, the object will be in
     # an uninitialized state.
+    # If `tp_traverse` and `tp_clear` functions check if the holder has been
+    # initialized, the functions can exit early and do not cause segfault on GC.
     m.VecOwnsObjs.set_should_raise_error_on_set_state(True)
     serialized = pickle.dumps(vec)
     with pytest.raises(

From 7db0a9f409dc54dc7b9e584236eb04bf4eec11e0 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Wed, 14 May 2025 04:22:07 +0800
Subject: [PATCH 09/11] Update docs

---
 docs/advanced/classes.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst
index 8e8b15b9fc..bdd5df5cdc 100644
--- a/docs/advanced/classes.rst
+++ b/docs/advanced/classes.rst
@@ -1351,7 +1351,7 @@ You can do that using ``py::custom_type_setup``:
                    return 0;
                }
 
-               // The actual logic of the tp_clear function goes here.
+               // The actual logic of the tp_traverse function goes here.
                auto &self = py::cast<OwnsPythonObjects&>(py::handle(self_base));
                Py_VISIT(self.value.ptr());
                return 0;

From 752d8838551d091f6928b7782a685276b0fe0335 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Sun, 18 May 2025 16:53:41 +0800
Subject: [PATCH 10/11] Add semi-public API:
 `pybind11::detail::is_holder_constructed`

---
 docs/advanced/classes.rst                  |  7 ++-----
 include/pybind11/detail/value_and_holder.h | 12 ++++++++++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst
index aeee0de81d..0fd42446d6 100644
--- a/docs/advanced/classes.rst
+++ b/docs/advanced/classes.rst
@@ -1388,9 +1388,7 @@ You can do that using ``py::custom_type_setup``:
    #if PY_VERSION_HEX >= 0x03090000  // Python 3.9
                Py_VISIT(Py_TYPE(self_base));
    #endif
-
-               auto* const instance = reinterpret_cast<py::detail::instance*>(self_base);
-               if (!instance->get_value_and_holder().holder_constructed()) [[unlikely]] {
+               if (!py::detail::is_holder_constructed(self_base)) [[unlikely]] {
                    // The holder has not been constructed yet.
                    // Skip the traversal to avoid segmentation faults.
                    return 0;
@@ -1402,8 +1400,7 @@ You can do that using ``py::custom_type_setup``:
                return 0;
            };
            type->tp_clear = [](PyObject *self_base) {
-               auto* const instance = reinterpret_cast<py::detail::instance*>(self_base);
-               if (!instance->get_value_and_holder().holder_constructed()) [[unlikely]] {
+               if (!py::detail::is_holder_constructed(self_base)) [[unlikely]] {
                    // The holder has not been constructed yet.
                    // Skip the traversal to avoid segmentation faults.
                    return 0;
diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h
index 64c55cc595..78036979e4 100644
--- a/include/pybind11/detail/value_and_holder.h
+++ b/include/pybind11/detail/value_and_holder.h
@@ -74,5 +74,17 @@ struct value_and_holder {
     }
 };
 
+// This is a semi-public API to check if the corresponding instance has been constructed with a
+// holder. That is, if the instance has been constructed with a holder, the `__init__` method is
+// called and the C++ object is valid. Otherwise, the C++ object might only be allocated, but not
+// initialized. This will lead to **SEGMENTATION FAULTS** if the C++ object is used in any way.
+// Example usage: https://pybind11.readthedocs.io/en/stable/advanced/classes.html#custom-type-setup
+//                for `tp_traverse` and `tp_clear` implementations.
+// WARNING: The caller is responsible for ensuring that the `reinterpret_cast` is valid.
+extern "C" [[nodiscard]] PYBIND11_NOINLINE bool is_holder_constructed(PyObject *obj) {
+    auto *const instance = reinterpret_cast<pybind11::detail::instance *>(obj);
+    return instance->get_value_and_holder().holder_constructed();
+}
+
 PYBIND11_NAMESPACE_END(detail)
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

From 110d024e83ce345246cdf20544e8502271550399 Mon Sep 17 00:00:00 2001
From: Xuehai Pan <XuehaiPan@pku.edu.cn>
Date: Mon, 19 May 2025 01:03:48 +0800
Subject: [PATCH 11/11] Apply suggestions from code review

---
 docs/advanced/classes.rst                  | 22 ++++++----------------
 include/pybind11/detail/value_and_holder.h |  2 +-
 tests/test_invalid_holder_access.cpp       | 22 ++++++++--------------
 3 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst
index 0fd42446d6..d494a8a616 100644
--- a/docs/advanced/classes.rst
+++ b/docs/advanced/classes.rst
@@ -1388,27 +1388,17 @@ You can do that using ``py::custom_type_setup``:
    #if PY_VERSION_HEX >= 0x03090000  // Python 3.9
                Py_VISIT(Py_TYPE(self_base));
    #endif
-               if (!py::detail::is_holder_constructed(self_base)) [[unlikely]] {
-                   // The holder has not been constructed yet.
-                   // Skip the traversal to avoid segmentation faults.
-                   return 0;
+               if (py::detail::is_holder_constructed(self_base)) {
+                   auto &self = py::cast<OwnsPythonObjects&>(py::handle(self_base));
+                   Py_VISIT(self.value.ptr());
                }
-
-               // The actual logic of the tp_traverse function goes here.
-               auto &self = py::cast<OwnsPythonObjects&>(py::handle(self_base));
-               Py_VISIT(self.value.ptr());
                return 0;
            };
            type->tp_clear = [](PyObject *self_base) {
-               if (!py::detail::is_holder_constructed(self_base)) [[unlikely]] {
-                   // The holder has not been constructed yet.
-                   // Skip the traversal to avoid segmentation faults.
-                   return 0;
+               if (py::detail::is_holder_constructed(self_base)) {
+                   auto &self = py::cast<OwnsPythonObjects&>(py::handle(self_base));
+                   self.value = py::none();
                }
-
-               // The actual logic of the tp_clear function goes here.
-               auto &self = py::cast<OwnsPythonObjects&>(py::handle(self_base));
-               self.value = py::none();
                return 0;
            };
        }));
diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h
index 78036979e4..87c92f8e49 100644
--- a/include/pybind11/detail/value_and_holder.h
+++ b/include/pybind11/detail/value_and_holder.h
@@ -81,7 +81,7 @@ struct value_and_holder {
 // Example usage: https://pybind11.readthedocs.io/en/stable/advanced/classes.html#custom-type-setup
 //                for `tp_traverse` and `tp_clear` implementations.
 // WARNING: The caller is responsible for ensuring that the `reinterpret_cast` is valid.
-extern "C" [[nodiscard]] PYBIND11_NOINLINE bool is_holder_constructed(PyObject *obj) {
+inline bool is_holder_constructed(PyObject *obj) {
     auto *const instance = reinterpret_cast<pybind11::detail::instance *>(obj);
     return instance->get_value_and_holder().holder_constructed();
 }
diff --git a/tests/test_invalid_holder_access.cpp b/tests/test_invalid_holder_access.cpp
index a79f8af98b..eb2c4b1f07 100644
--- a/tests/test_invalid_holder_access.cpp
+++ b/tests/test_invalid_holder_access.cpp
@@ -32,13 +32,10 @@ class VecOwnsObjs {
         Py_VISIT(Py_TYPE(self_base));
 #endif
 
-        if (should_check_holder_initialization) {
-            auto *const instance = reinterpret_cast<py::detail::instance *>(self_base);
-            if (!instance->get_value_and_holder().holder_constructed()) {
-                // The holder has not been constructed yet. Skip the traversal to avoid
-                // segmentation faults.
-                return 0;
-            }
+        if (should_check_holder_initialization && !py::detail::is_holder_constructed(self_base)) {
+            // The holder has not been constructed yet. Skip the traversal to avoid
+            // segmentation faults.
+            return 0;
         }
 
         // The actual logic of the tp_traverse function goes here.
@@ -50,13 +47,10 @@ class VecOwnsObjs {
     }
 
     static int tp_clear(PyObject *self_base) {
-        if (should_check_holder_initialization) {
-            auto *const instance = reinterpret_cast<py::detail::instance *>(self_base);
-            if (!instance->get_value_and_holder().holder_constructed()) {
-                // The holder has not been constructed yet. Skip the traversal to avoid
-                // segmentation faults.
-                return 0;
-            }
+        if (should_check_holder_initialization && !py::detail::is_holder_constructed(self_base)) {
+            // The holder has not been constructed yet. Skip the traversal to avoid
+            // segmentation faults.
+            return 0;
         }
 
         // The actual logic of the tp_clear function goes here.