From 229b3bce971696438c908f4be3d313ab7a740963 Mon Sep 17 00:00:00 2001 From: Yuanyuan Chen Date: Tue, 9 Dec 2025 08:43:36 +0800 Subject: [PATCH 1/5] Fix PyObject_HasAttrString return value Signed-off-by: cyy --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index b28692fd74..6b81eff7f7 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -547,7 +547,7 @@ struct error_fetch_and_normalize { // The presence of __notes__ is likely due to exception normalization // errors, although that is not necessarily true, therefore insert a // hint only: - if (PyObject_HasAttrString(m_value.ptr(), "__notes__")) { + if (PyObject_HasAttrString(m_value.ptr(), "__notes__") == 1) { m_lazy_error_string += "[WITH __notes__]"; } #else From 2d1a3a68b4e1d1b2d4edf042b9adae3fc034b2cd Mon Sep 17 00:00:00 2001 From: Yuanyuan Chen Date: Tue, 9 Dec 2025 10:04:26 +0800 Subject: [PATCH 2/5] Tidy unchecked files Signed-off-by: cyy --- include/pybind11/detail/internals.h | 5 ++--- include/pybind11/subinterpreter.h | 33 +++++++++++++++-------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 4d6c147db3..858de67525 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -606,9 +606,8 @@ class internals_pp_manager { // this could be called without an active interpreter, just use what was cached if (!tstate || tstate->interp == last_istate_tls()) { auto tpp = internals_p_tls(); - if (tpp) { - delete tpp; - } + + delete tpp; } unref(); return; diff --git a/include/pybind11/subinterpreter.h b/include/pybind11/subinterpreter.h index 5d2f0a8393..aaf5204570 100644 --- a/include/pybind11/subinterpreter.h +++ b/include/pybind11/subinterpreter.h @@ -22,11 +22,11 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) inline PyInterpreterState *get_interpreter_state_unchecked() { - auto cur_tstate = get_thread_state_unchecked(); - if (cur_tstate) + auto *cur_tstate = get_thread_state_unchecked(); + if (cur_tstate) { return cur_tstate->interp; - else - return nullptr; + } + return nullptr; } PYBIND11_NAMESPACE_END(detail) @@ -76,7 +76,7 @@ class subinterpreter { /// Create a new subinterpreter with the specified configuration /// @note This function acquires (and then releases) the main interpreter GIL, but the main /// interpreter and its GIL are not required to be held prior to calling this function. - static inline subinterpreter create(PyInterpreterConfig const &cfg) { + static subinterpreter create(PyInterpreterConfig const &cfg) { error_scope err_scope; subinterpreter result; @@ -84,7 +84,7 @@ class subinterpreter { // we must hold the main GIL in order to create a subinterpreter subinterpreter_scoped_activate main_guard(main()); - auto prev_tstate = PyThreadState_Get(); + auto *prev_tstate = PyThreadState_Get(); PyStatus status; @@ -103,7 +103,7 @@ class subinterpreter { } // this doesn't raise a normal Python exception, it provides an exit() status code. - if (PyStatus_Exception(status)) { + if (PyStatus_Exception(status) != 0) { pybind11_fail("failed to create new sub-interpreter"); } @@ -128,7 +128,7 @@ class subinterpreter { /// Calls create() with a default configuration of an isolated interpreter that disallows fork, /// exec, and Python threads. - static inline subinterpreter create() { + static subinterpreter create() { // same as the default config in the python docs PyInterpreterConfig cfg; std::memset(&cfg, 0, sizeof(cfg)); @@ -144,8 +144,8 @@ class subinterpreter { return; } - PyThreadState *destroy_tstate; - PyThreadState *old_tstate; + PyThreadState *destroy_tstate = nullptr; + PyThreadState *old_tstate = nullptr; // Python 3.12 requires us to keep the original PyThreadState alive until we are ready to // destroy the interpreter. We prefer to use that to destroy the interpreter. @@ -173,7 +173,7 @@ class subinterpreter { old_tstate = PyThreadState_Swap(destroy_tstate); #endif - bool switch_back = old_tstate && old_tstate->interp != istate_; + bool switch_back = (old_tstate != nullptr) && old_tstate->interp != istate_; // Internals always exists in the subinterpreter, this class enforces it when it creates // the subinterpreter. Even if it didn't, this only creates the pointer-to-pointer, not the @@ -190,8 +190,9 @@ class subinterpreter { detail::get_local_internals_pp_manager().destroy(); // switch back to the old tstate and old GIL (if there was one) - if (switch_back) + if (switch_back) { PyThreadState_Swap(old_tstate); + } } /// Get a handle to the main interpreter that can be used with subinterpreter_scoped_activate @@ -214,11 +215,11 @@ class subinterpreter { /// Get the numerical identifier for the sub-interpreter int64_t id() const { - if (istate_ != nullptr) + if (istate_ != nullptr) { return PyInterpreterState_GetID(istate_); - else - return -1; // CPython uses one-up numbers from 0, so negative should be safe to return - // here. + } + return -1; // CPython uses one-up numbers from 0, so negative should be safe to return + // here. } /// Get the interpreter's state dict. This interpreter's GIL must be held before calling! From 1d1f3434051e80e91af255b6f5f6d16b657d1680 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 12 Dec 2025 22:11:35 -0800 Subject: [PATCH 3/5] [skip ci] Handle PyObject_HasAttrString error when probing __notes__ PyObject_HasAttrString may return -1 to signal an error and set a Python exception. The previous logic only checked for "!= 0", which meant that the error path was treated the same as "attribute exists", causing two problems: misreporting the presence of __notes__ and leaving a spurious exception pending. The earlier PR tightened the condition to "== 1" so that only a successful lookup marks the error string as [WITH __notes__], but it still left the -1 case unhandled. In the context of error_fetch_and_normalize, we are already dealing with an active exception and only want to best-effort detect whether normalization attached any __notes__. If the attribute probe itself fails, we do not want that secondary failure to affect later C-API calls or the error we ultimately report. This change stores the PyObject_HasAttrString return value, treats "== 1" as "has __notes__", and explicitly calls PyErr_Clear() when it returns -1. That way, we avoid leaking a secondary error while still preserving the original exception information and hinting [WITH __notes__] only when we can determine it reliably. --- include/pybind11/pytypes.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a389f21b69..0ab0b73e1f 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -547,8 +547,13 @@ struct error_fetch_and_normalize { // The presence of __notes__ is likely due to exception normalization // errors, although that is not necessarily true, therefore insert a // hint only: - if (PyObject_HasAttrString(m_value.ptr(), "__notes__") == 1) { + const int has_notes = PyObject_HasAttrString(m_value.ptr(), "__notes__"); + if (has_notes == 1) { m_lazy_error_string += "[WITH __notes__]"; + } else if (has_notes == -1) { + // Ignore secondary errors when probing for __notes__ to avoid leaking a + // spurious exception while still reporting the original error. + PyErr_Clear(); } #else // PyErr_NormalizeException() may change the exception type if there are cascading From bb6e751de4a96d491fcbf5a989cf38aab31694d3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 13 Dec 2025 00:30:03 -0800 Subject: [PATCH 4/5] Run clang-tidy with -DPYBIND11_HAS_SUBINTERPRETER_SUPPORT --- .github/workflows/format.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index 6bf77324a9..897806cb0a 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -46,7 +46,7 @@ jobs: run: apt-get update && apt-get install -y git python3-dev python3-pytest ninja-build - name: Configure - run: cmake --preset tidy + run: cmake --preset tidy -DCMAKE_CXX_FLAGS="-DPYBIND11_HAS_SUBINTERPRETER_SUPPORT=1" - name: Build run: cmake --build --preset tidy From bbb47ed2e9cae185fddd03b741292cc50e0608ae Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 13 Dec 2025 02:15:36 -0800 Subject: [PATCH 5/5] [skip ci] Revert "Run clang-tidy with -DPYBIND11_HAS_SUBINTERPRETER_SUPPORT" This reverts commit bb6e751de4a96d491fcbf5a989cf38aab31694d3. --- .github/workflows/format.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index 897806cb0a..6bf77324a9 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -46,7 +46,7 @@ jobs: run: apt-get update && apt-get install -y git python3-dev python3-pytest ninja-build - name: Configure - run: cmake --preset tidy -DCMAKE_CXX_FLAGS="-DPYBIND11_HAS_SUBINTERPRETER_SUPPORT=1" + run: cmake --preset tidy - name: Build run: cmake --build --preset tidy