From de4ba92c9fe1d5d225f39cbc251e7f2616f3b58d Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Tue, 31 May 2022 11:51:13 -0700
Subject: [PATCH] Add `error_scope` to `detail::get_internals()` (#3981)

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

* Adjust test to tolerate macOS PyPy behavior.
---
 include/pybind11/detail/internals.h           |  1 +
 tests/CMakeLists.txt                          |  1 +
 ...s_module_interleaved_error_already_set.cpp | 51 +++++++++++++++++++
 tests/test_exceptions.cpp                     |  7 +++
 tests/test_exceptions.py                      |  9 ++++
 5 files changed, 69 insertions(+)
 create mode 100644 tests/cross_module_interleaved_error_already_set.cpp

diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h
index 3f83113bd..6ca5e1482 100644
--- a/include/pybind11/detail/internals.h
+++ b/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());
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index a14b520a5..7296cd1b8 100644
--- a/tests/CMakeLists.txt
+++ b/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
diff --git a/tests/cross_module_interleaved_error_already_set.cpp b/tests/cross_module_interleaved_error_already_set.cpp
new file mode 100644
index 000000000..fdd9939e4
--- /dev/null
+++ b/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;
+}
diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp
index d7b31cf74..8c74d7029 100644
--- a/tests/test_exceptions.cpp
+++ b/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();
+    });
 }
diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py
index dc7594113..4320ad181 100644
--- a/tests/test_exceptions.py
+++ b/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).
+    )