Merge pull request #30157 from rwgk/pybind11clif_merge_sh

git merge smart_holder

Author: rwgk

CMakeLists.txt

@@ -170,7 +170,8 @@ set(PYBIND11_HEADERS
     include/pybind11/stl/filesystem.h
     include/pybind11/trampoline_self_life_support.h
     include/pybind11/type_caster_pyobject_ptr.h
-    include/pybind11/typing.h)
+    include/pybind11/typing.h
+    include/pybind11/warnings.h)
 
 # Compare with grep and warn if mismatched
 if(PYBIND11_MASTER_PROJECT)

docs/faq.rst

@@ -247,6 +247,50 @@ been received, you must either explicitly interrupt execution by throwing
         });
     }
 
+What is a highly conclusive and simple way to find memory leaks (e.g. in pybind11 bindings)?
+============================================================================================
+
+Use ``while True`` & ``top`` (Linux, macOS).
+
+For example, locally change tests/test_type_caster_pyobject_ptr.py like this:
+
+.. code-block:: diff
+
+     def test_return_list_pyobject_ptr_reference():
+    +  while True:
+         vec_obj = m.return_list_pyobject_ptr_reference(ValueHolder)
+         assert [e.value for e in vec_obj] == [93, 186]
+         # Commenting out the next `assert` will leak the Python references.
+         # An easy way to see evidence of the leaks:
+         # Insert `while True:` as the first line of this function and monitor the
+         # process RES (Resident Memory Size) with the Unix top command.
+    -    assert m.dec_ref_each_pyobject_ptr(vec_obj) == 2
+    +    # assert m.dec_ref_each_pyobject_ptr(vec_obj) == 2
+
+Then run the test as you would normally do, which will go into the infinite loop.
+
+**In another shell, but on the same machine** run:
+
+.. code-block:: bash
+
+    top
+
+This will show:
+
+.. code-block::
+
+        PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
+    1266095 rwgk      20   0 5207496 611372  45696 R 100.0   0.3   0:08.01 test_type_caste
+
+Look for the number under ``RES`` there. You'll see it going up very quickly.
+
+**Don't forget to Ctrl-C the test command** before your machine becomes
+unresponsive due to swapping.
+
+This method only takes a couple minutes of effort and is very conclusive.
+What you want to see is that the ``RES`` number is stable after a couple
+seconds.
+
 CMake doesn't detect the right Python version
 =============================================
 

include/pybind11/detail/struct_smart_holder.h

@@ -215,7 +215,7 @@ struct smart_holder {
         // race conditions, but in the context of Python it is a bug (elsewhere)
         // if the Global Interpreter Lock (GIL) is not being held when this code
         // is reached.
-        // SMART_HOLDER_WIP: IMPROVABLE: assert(GIL is held).
+        // PYBIND11:REMINDER: This may need to be protected by a mutex in free-threaded Python.
         if (vptr.use_count() != 1) {
             throw std::invalid_argument(std::string("Cannot disown use_count != 1 (") + context
                                         + ").");
@@ -277,29 +277,32 @@ struct smart_holder {
         return hld;
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void disown() {
         reset_vptr_deleter_armed_flag(false);
         is_disowned = true;
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void reclaim_disowned() {
         reset_vptr_deleter_armed_flag(true);
         is_disowned = false;
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void release_disowned() { vptr.reset(); }
 
-    // SMART_HOLDER_WIP: review this function.
     void ensure_can_release_ownership(const char *context = "ensure_can_release_ownership") const {
         ensure_is_not_disowned(context);
         ensure_vptr_is_using_builtin_delete(context);
         ensure_use_count_1(context);
     }
 
-    // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details).
+    // Caller is responsible for ensuring the complex preconditions
+    // (see `smart_holder_type_caster_support::load_helper`).
     void release_ownership() {
         reset_vptr_deleter_armed_flag(false);
         release_disowned();

include/pybind11/detail/type_caster_base.h

@@ -476,7 +476,7 @@ inline PyObject *make_new_instance(PyTypeObject *type);
 
 #ifdef PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
 
-// SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code.
+// PYBIND11:REMINDER: Needs refactoring of existing pybind11 code.
 inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);
 
 PYBIND11_NAMESPACE_BEGIN(smart_holder_type_caster_support)
@@ -569,8 +569,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr<T, D> &&src,
 
     if (static_cast<void *>(src.get()) == src_raw_void_ptr) {
         // This is a multiple-inheritance situation that is incompatible with the current
-        // shared_from_this handling (see PR #3023).
-        // SMART_HOLDER_WIP: IMPROVABLE: Is there a better solution?
+        // shared_from_this handling (see PR #3023). Is there a better solution?
         src_raw_void_ptr = nullptr;
     }
     auto smhldr = smart_holder::from_unique_ptr(std::move(src), src_raw_void_ptr);
@@ -626,8 +625,8 @@ handle smart_holder_from_shared_ptr(const std::shared_ptr<T> &src,
     void *src_raw_void_ptr = static_cast<void *>(src_raw_ptr);
     const detail::type_info *tinfo = st.second;
     if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) {
-        // SMART_HOLDER_WIP: MISSING: Enforcement of consistency with existing smart_holder.
-        // SMART_HOLDER_WIP: MISSING: keep_alive.
+        // PYBIND11:REMINDER: MISSING: Enforcement of consistency with existing smart_holder.
+        // PYBIND11:REMINDER: MISSING: keep_alive.
         return existing_inst;
     }
 

include/pybind11/trampoline_self_life_support.h

@@ -15,14 +15,13 @@
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
 PYBIND11_NAMESPACE_BEGIN(detail)
-// SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code.
+// PYBIND11:REMINDER: Needs refactoring of existing pybind11 code.
 inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);
 PYBIND11_NAMESPACE_END(detail)
 
 // The original core idea for this struct goes back to PyCLIF:
 // https://github.com/google/clif/blob/07f95d7e69dca2fcf7022978a55ef3acff506c19/clif/python/runtime.cc#L37
-// URL provided here mainly to give proper credit. To fully explain the `HoldPyObj` feature, more
-// context is needed (SMART_HOLDER_WIP).
+// URL provided here mainly to give proper credit.
 struct trampoline_self_life_support {
     detail::value_and_holder v_h;
 

include/pybind11/typing.h

@@ -100,9 +100,7 @@ class Never : public none {
     using none::none;
 };
 
-#if defined(__cpp_nontype_template_parameter_class)                                               \
-    && (/* See #5201 */ !defined(__GNUC__)                                                        \
-        || (__GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 3)))
+#if defined(__cpp_nontype_template_args) && __cpp_nontype_template_args >= 201911L
 #    define PYBIND11_TYPING_H_HAS_STRING_LITERAL
 template <size_t N>
 struct StringLiteral {

include/pybind11/warnings.h

@@ -0,0 +1,75 @@
+/*
+    pybind11/warnings.h: Python warnings wrappers.
+
+    Copyright (c) 2024 Jan Iwaszkiewicz <[email protected]>
+
+    All rights reserved. Use of this source code is governed by a
+    BSD-style license that can be found in the LICENSE file.
+*/
+
+#pragma once
+
+#include "pybind11.h"
+#include "detail/common.h"
+
+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+
+PYBIND11_NAMESPACE_BEGIN(detail)
+
+inline bool PyWarning_Check(PyObject *obj) {
+    int result = PyObject_IsSubclass(obj, PyExc_Warning);
+    if (result == 1) {
+        return true;
+    }
+    if (result == -1) {
+        raise_from(PyExc_SystemError,
+                   "pybind11::detail::PyWarning_Check(): PyObject_IsSubclass() call failed.");
+        throw error_already_set();
+    }
+    return false;
+}
+
+PYBIND11_NAMESPACE_END(detail)
+
+PYBIND11_NAMESPACE_BEGIN(warnings)
+
+inline object
+new_warning_type(handle scope, const char *name, handle base = PyExc_RuntimeWarning) {
+    if (!detail::PyWarning_Check(base.ptr())) {
+        pybind11_fail("pybind11::warnings::new_warning_type(): cannot create custom warning, base "
+                      "must be a subclass of "
+                      "PyExc_Warning!");
+    }
+    if (hasattr(scope, name)) {
+        pybind11_fail("pybind11::warnings::new_warning_type(): an attribute with name \""
+                      + std::string(name) + "\" exists already.");
+    }
+    std::string full_name = scope.attr("__name__").cast<std::string>() + std::string(".") + name;
+    handle h(PyErr_NewException(full_name.c_str(), base.ptr(), nullptr));
+    if (!h) {
+        raise_from(PyExc_SystemError,
+                   "pybind11::warnings::new_warning_type(): PyErr_NewException() call failed.");
+        throw error_already_set();
+    }
+    auto obj = reinterpret_steal<object>(h);
+    scope.attr(name) = obj;
+    return obj;
+}
+
+// Similar to Python `warnings.warn()`
+inline void
+warn(const char *message, handle category = PyExc_RuntimeWarning, int stack_level = 2) {
+    if (!detail::PyWarning_Check(category.ptr())) {
+        pybind11_fail(
+            "pybind11::warnings::warn(): cannot raise warning, category must be a subclass of "
+            "PyExc_Warning!");
+    }
+
+    if (PyErr_WarnEx(category.ptr(), message, stack_level) == -1) {
+        throw error_already_set();
+    }
+}
+
+PYBIND11_NAMESPACE_END(warnings)
+
+PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

tests/CMakeLists.txt

@@ -135,7 +135,6 @@ set(PYBIND11_TEST_FILES
     test_class_sh_unique_ptr_member
     test_class_sh_virtual_py_cpp_mix
     test_class_sh_void_ptr_capsule
-    test_classh_mock
     test_const_name
     test_constants_and_functions
     test_copy_move
@@ -182,7 +181,8 @@ set(PYBIND11_TEST_FILES
     test_unnamed_namespace_a
     test_unnamed_namespace_b
     test_vector_unique_ptr_member
-    test_virtual_functions)
+    test_virtual_functions
+    test_warnings)
 
 # Invoking cmake with something like:
 #     cmake -DPYBIND11_TEST_OVERRIDE="test_callbacks.cpp;test_pickling.cpp" ..

tests/extra_python_package/test_files.py

@@ -50,6 +50,7 @@
     "include/pybind11/trampoline_self_life_support.h",
     "include/pybind11/type_caster_pyobject_ptr.h",
     "include/pybind11/typing.h",
+    "include/pybind11/warnings.h",
 }
 
 detail_headers = {

tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py

@@ -62,7 +62,7 @@ def test_shared_ptr_arg_identity():
     del obj
     pytest.gc_collect()
 
-    # SMART_HOLDER_WIP: the behavior below is DIFFERENT from PR #2839
+    # NOTE: the behavior below is DIFFERENT from PR #2839
     # python reference is gone because it is not an Alias instance
     assert objref() is None
     assert tester.has_python_instance() is False

tests/test_classh_mock.cpp

@@ -1026,7 +1026,7 @@ def test_optional_object_annotations(doc):
 
 @pytest.mark.skipif(
     not m.defined_PYBIND11_TYPING_H_HAS_STRING_LITERAL,
-    reason="C++20 feature not available.",
+    reason="C++20 non-type template args feature not available.",
 )
 def test_literal(doc):
     assert (
@@ -1037,7 +1037,7 @@ def test_literal(doc):
 
 @pytest.mark.skipif(
     not m.defined_PYBIND11_TYPING_H_HAS_STRING_LITERAL,
-    reason="C++20 feature not available.",
+    reason="C++20 non-type template args feature not available.",
 )
 def test_typevar(doc):
     assert (