Merge pull request #30069 from rwgk/pywrapcc_merge_sh

git merge smart_holder

.github/workflows/ci.yml

@@ -1053,7 +1053,7 @@ jobs:
         uses: jwlawson/actions-setup-cmake@v1.14
 
       - name: Install ninja-build tool
-        uses: seanmiddleditch/gha-setup-ninja@v3
+        uses: seanmiddleditch/gha-setup-ninja@v4
 
       - name: Run pip installs
         run: |

.github/workflows/ci_sh_def.yml

@@ -1080,7 +1080,7 @@ jobs:
         uses: jwlawson/actions-setup-cmake@v1.14
 
       - name: Install ninja-build tool
-        uses: seanmiddleditch/gha-setup-ninja@v3
+        uses: seanmiddleditch/gha-setup-ninja@v4
 
       - name: Run pip installs
         run: |

.github/workflows/ci_sh_def.yml.patch

@@ -1,5 +1,5 @@
---- ci.yml	2023-09-10 10:49:38.655616237 -0700
-+++ ci_sh_def.yml	2023-09-10 10:50:17.855597833 -0700
+--- ci.yml	2023-10-11 21:07:36.619964634 -0700
++++ ci_sh_def.yml	2023-10-11 21:10:12.551698440 -0700
 @@ -1,4 +1,16 @@
 -name: CI
 +# PLEASE KEEP THIS GROUP OF FILES IN SYNC AT ALL TIMES:

CMakeLists.txt

@@ -151,6 +151,7 @@ set(PYBIND11_HEADERS
     include/pybind11/embed.h
     include/pybind11/eval.h
     include/pybind11/gil.h
+    include/pybind11/gil_safe_call_once.h
     include/pybind11/iostream.h
     include/pybind11/functional.h
     include/pybind11/native_enum.h

docs/advanced/exceptions.rst

@@ -141,15 +141,14 @@ standard python RuntimeError:
 
 .. code-block:: cpp
 
-    // This is a static object, so we must leak the Python reference:
-    // It is undefined when the destructor will run, possibly only after the
-    // Python interpreter is finalized already.
-    static py::handle exc = py::exception<MyCustomException>(m, "MyCustomError").release();
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> exc_storage;
+    exc_storage.call_once_and_store_result(
+        [&]() { return py::exception<MyCustomException>(m, "MyCustomError"); });
     py::register_exception_translator([](std::exception_ptr p) {
         try {
             if (p) std::rethrow_exception(p);
         } catch (const MyCustomException &e) {
-            py::set_error(exc, e.what());
+            py::set_error(exc_storage.get_stored(), e.what());
         } catch (const OtherException &e) {
             py::set_error(PyExc_RuntimeError, e.what());
         }

include/pybind11/detail/common.h

@@ -118,6 +118,14 @@
 #    endif
 #endif
 
+#if defined(PYBIND11_CPP20)
+#    define PYBIND11_CONSTINIT constinit
+#    define PYBIND11_DTOR_CONSTEXPR constexpr
+#else
+#    define PYBIND11_CONSTINIT
+#    define PYBIND11_DTOR_CONSTEXPR
+#endif
+
 // Compiler version assertions
 #if defined(__INTEL_COMPILER)
 #    if __INTEL_COMPILER < 1800

include/pybind11/gil.h

@@ -11,6 +11,8 @@
 
 #include "detail/common.h"
 
+#include <cassert>
+
 #if defined(WITH_THREAD) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
 #    include "detail/internals.h"
 #endif
@@ -137,7 +139,9 @@ class gil_scoped_acquire {
 
 class gil_scoped_release {
 public:
+    // PRECONDITION: The GIL must be held when this constructor is called.
     explicit gil_scoped_release(bool disassoc = false) : disassoc(disassoc) {
+        assert(PyGILState_Check());
         // `get_internals()` must be called here unconditionally in order to initialize
         // `internals.tstate` for subsequent `gil_scoped_acquire` calls. Otherwise, an
         // initialization race could occur as multiple threads try `gil_scoped_acquire`.
@@ -201,7 +205,11 @@ class gil_scoped_release {
     PyThreadState *state;
 
 public:
-    gil_scoped_release() : state{PyEval_SaveThread()} {}
+    // PRECONDITION: The GIL must be held when this constructor is called.
+    gil_scoped_release() {
+        assert(PyGILState_Check());
+        state = PyEval_SaveThread();
+    }
     gil_scoped_release(const gil_scoped_release &) = delete;
     gil_scoped_release &operator=(const gil_scoped_release &) = delete;
     ~gil_scoped_release() { PyEval_RestoreThread(state); }

include/pybind11/gil_safe_call_once.h

@@ -0,0 +1,91 @@
+// Copyright (c) 2023 The pybind Community.
+
+#pragma once
+
+#include "detail/common.h"
+#include "gil.h"
+
+#include <cassert>
+#include <mutex>
+
+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+
+// Use the `gil_safe_call_once_and_store` class below instead of the naive
+//
+//   static auto imported_obj = py::module_::import("module_name"); // BAD, DO NOT USE!
+//
+// which has two serious issues:
+//
+//     1. Py_DECREF() calls potentially after the Python interpreter was finalized already, and
+//     2. deadlocks in multi-threaded processes (because of missing lock ordering).
+//
+// The following alternative avoids both problems:
+//
+//   PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> storage;
+//   auto &imported_obj = storage // Do NOT make this `static`!
+//       .call_once_and_store_result([]() {
+//           return py::module_::import("module_name");
+//       })
+//       .get_stored();
+//
+// The parameter of `call_once_and_store_result()` must be callable. It can make
+// CPython API calls, and in particular, it can temporarily release the GIL.
+//
+// `T` can be any C++ type, it does not have to involve CPython API types.
+//
+// The behavior with regard to signals, e.g. `SIGINT` (`KeyboardInterrupt`),
+// is not ideal. If the main thread is the one to actually run the `Callable`,
+// then a `KeyboardInterrupt` will interrupt it if it is running normal Python
+// code. The situation is different if a non-main thread runs the
+// `Callable`, and then the main thread starts waiting for it to complete:
+// a `KeyboardInterrupt` will not interrupt the non-main thread, but it will
+// get processed only when it is the main thread's turn again and it is running
+// normal Python code. However, this will be unnoticeable for quick call-once
+// functions, which is usually the case.
+template <typename T>
+class gil_safe_call_once_and_store {
+public:
+    // PRECONDITION: The GIL must be held when `call_once_and_store_result()` is called.
+    template <typename Callable>
+    gil_safe_call_once_and_store &call_once_and_store_result(Callable &&fn) {
+        if (!is_initialized_) { // This read is guarded by the GIL.
+            // Multiple threads may enter here, because the GIL is released in the next line and
+            // CPython API calls in the `fn()` call below may release and reacquire the GIL.
+            gil_scoped_release gil_rel; // Needed to establish lock ordering.
+            std::call_once(once_flag_, [&] {
+                // Only one thread will ever enter here.
+                gil_scoped_acquire gil_acq;
+                ::new (storage_) T(fn()); // fn may release, but will reacquire, the GIL.
+                is_initialized_ = true;   // This write is guarded by the GIL.
+            });
+            // All threads will observe `is_initialized_` as true here.
+        }
+        // Intentionally not returning `T &` to ensure the calling code is self-documenting.
+        return *this;
+    }
+
+    // This must only be called after `call_once_and_store_result()` was called.
+    T &get_stored() {
+        assert(is_initialized_);
+        PYBIND11_WARNING_PUSH
+#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5
+        // Needed for gcc 4.8.5
+        PYBIND11_WARNING_DISABLE_GCC("-Wstrict-aliasing")
+#endif
+        return *reinterpret_cast<T *>(storage_);
+        PYBIND11_WARNING_POP
+    }
+
+    constexpr gil_safe_call_once_and_store() = default;
+    PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() = default;
+
+private:
+    alignas(T) char storage_[sizeof(T)] = {};
+    std::once_flag once_flag_ = {};
+    bool is_initialized_ = false;
+    // The `is_initialized_`-`storage_` pair is very similar to `std::optional`,
+    // but the latter does not have the triviality properties of former,
+    // therefore `std::optional` is not a viable alternative here.
+};
+
+PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

include/pybind11/numpy.h

@@ -10,7 +10,10 @@
 #pragma once
 
 #include "pybind11.h"
+#include "detail/common.h"
 #include "complex.h"
+#include "gil_safe_call_once.h"
+#include "pytypes.h"
 
 #include <algorithm>
 #include <array>
@@ -206,8 +209,8 @@ struct npy_api {
     };
 
     static npy_api &get() {
-        static npy_api api = lookup();
-        return api;
+        PYBIND11_CONSTINIT static gil_safe_call_once_and_store<npy_api> storage;
+        return storage.call_once_and_store_result(lookup).get_stored();
     }
 
     bool PyArray_Check_(PyObject *obj) const {
@@ -643,10 +646,14 @@ class dtype : public object {
     char flags() const { return detail::array_descriptor_proxy(m_ptr)->flags; }
 
 private:
-    static object _dtype_from_pep3118() {
-        module_ m = detail::import_numpy_core_submodule("_internal");
-        static PyObject *obj = m.attr("_dtype_from_pep3118").cast<object>().release().ptr();
-        return reinterpret_borrow<object>(obj);
+    static object &_dtype_from_pep3118() {
+        PYBIND11_CONSTINIT static gil_safe_call_once_and_store<object> storage;
+        return storage
+            .call_once_and_store_result([]() {
+                return detail::import_numpy_core_submodule("_internal")
+                    .attr("_dtype_from_pep3118");
+            })
+            .get_stored();
     }
 
     dtype strip_padding(ssize_t itemsize) {

tests/extra_python_package/test_files.py

@@ -35,6 +35,7 @@
     "include/pybind11/eval.h",
     "include/pybind11/functional.h",
     "include/pybind11/gil.h",
+    "include/pybind11/gil_safe_call_once.h",
     "include/pybind11/iostream.h",
     "include/pybind11/native_enum.h",
     "include/pybind11/numpy.h",

tests/test_exceptions.cpp

@@ -6,6 +6,8 @@
     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/gil_safe_call_once.h>
+
 #include "test_exceptions.h"
 
 #include "local_bindings.h"
@@ -114,15 +116,17 @@ TEST_SUBMODULE(exceptions, m) {
           []() { throw std::runtime_error("This exception was intentionally thrown."); });
 
     // PLEASE KEEP IN SYNC with docs/advanced/exceptions.rst
-    static py::handle ex = py::exception<MyException>(m, "MyException").release();
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> ex_storage;
+    ex_storage.call_once_and_store_result(
+        [&]() { return py::exception<MyException>(m, "MyException"); });
     py::register_exception_translator([](std::exception_ptr p) {
         try {
             if (p) {
                 std::rethrow_exception(p);
             }
         } catch (const MyException &e) {
             // Set MyException as the active python error
-            py::set_error(ex, e.what());
+            py::set_error(ex_storage.get_stored(), e.what());
         }
     });