Add error_scope to detail::get_internals() (#3981)

* Add `error_scope` to `detail::get_internals()`

* Adjust test to tolerate macOS PyPy behavior.

Author: rwgk

include/pybind11/detail/internals.h

@@ -415,6 +415,7 @@ PYBIND11_NOINLINE internals &get_internals() {
         ~gil_scoped_acquire_local() { PyGILState_Release(state); }
         const PyGILState_STATE state;
     } gil;
+    error_scope err_scope;
 
     PYBIND11_STR_TYPE id(PYBIND11_INTERNALS_ID);
     auto builtins = handle(PyEval_GetBuiltins());

tests/CMakeLists.txt

@@ -215,6 +215,7 @@ tests_extra_targets("test_exceptions.py;test_local_bindings.py;test_stl.py;test_
                     "pybind11_cross_module_tests")
 
 # And add additional targets for other tests.
+tests_extra_targets("test_exceptions.py" "cross_module_interleaved_error_already_set")
 tests_extra_targets("test_gil_scoped.py" "cross_module_gil_utils")
 
 set(PYBIND11_EIGEN_REPO

tests/cross_module_interleaved_error_already_set.cpp

@@ -0,0 +1,51 @@
+/*
+    Copyright (c) 2022 Google LLC
+
+    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/pybind11.h>
+
+// This file mimics a DSO that makes pybind11 calls but does not define a PYBIND11_MODULE,
+// so that the first call of cross_module_error_already_set() triggers the first call of
+// pybind11::detail::get_internals().
+
+namespace {
+
+namespace py = pybind11;
+
+void interleaved_error_already_set() {
+    PyErr_SetString(PyExc_RuntimeError, "1st error.");
+    try {
+        throw py::error_already_set();
+    } catch (const py::error_already_set &) {
+        // The 2nd error could be conditional in a real application.
+        PyErr_SetString(PyExc_RuntimeError, "2nd error.");
+    } // Here the 1st error is destroyed before the 2nd error is fetched.
+    // The error_already_set dtor triggers a pybind11::detail::get_internals()
+    // call via pybind11::gil_scoped_acquire.
+    if (PyErr_Occurred()) {
+        throw py::error_already_set();
+    }
+}
+
+constexpr char kModuleName[] = "cross_module_interleaved_error_already_set";
+
+struct PyModuleDef moduledef = {
+    PyModuleDef_HEAD_INIT, kModuleName, nullptr, 0, nullptr, nullptr, nullptr, nullptr, nullptr};
+
+} // namespace
+
+extern "C" PYBIND11_EXPORT PyObject *PyInit_cross_module_interleaved_error_already_set() {
+    PyObject *m = PyModule_Create(&moduledef);
+    if (m != nullptr) {
+        static_assert(sizeof(&interleaved_error_already_set) == sizeof(void *),
+                      "Function pointer must have the same size as void *");
+        PyModule_AddObject(
+            m,
+            "funcaddr",
+            PyLong_FromVoidPtr(reinterpret_cast<void *>(&interleaved_error_already_set)));
+    }
+    return m;
+}

tests/test_exceptions.cpp

@@ -307,4 +307,11 @@ TEST_SUBMODULE(exceptions, m) {
         PyErr_Clear();
         return py::make_tuple(std::move(what), py_err_set_after_what);
     });
+
+    m.def("test_cross_module_interleaved_error_already_set", []() {
+        auto cm = py::module_::import("cross_module_interleaved_error_already_set");
+        auto interleaved_error_already_set
+            = reinterpret_cast<void (*)()>(PyLong_AsVoidPtr(cm.attr("funcaddr").ptr()));
+        interleaved_error_already_set();
+    });
 }

tests/test_exceptions.py

@@ -320,3 +320,12 @@ def test_flaky_exception_failure_point_str():
     with pytest.raises(ValueError) as excinfo:
         m.error_already_set_what(FlakyException, ("failure_point_str",))
     assert str(excinfo.value) == "triggered_failure_point_str"
+
+
+def test_cross_module_interleaved_error_already_set():
+    with pytest.raises(RuntimeError) as excinfo:
+        m.test_cross_module_interleaved_error_already_set()
+    assert str(excinfo.value) in (
+        "2nd error.",  # Almost all platforms.
+        "RuntimeError: 2nd error.",  # Some PyPy builds (seen under macOS).
+    )