From 2de6e398e1c0453de51886e18366ac58aebd8b92 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Fri, 30 Dec 2022 10:44:55 -0800 Subject: [PATCH 1/7] [v2.10] Revert the addition of the GIL check feature (#4432) * Revert the GIL check * Update include/pybind11/detail/common.h Co-authored-by: Henry Schreiner --- include/pybind11/detail/common.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 068ae0137..62bf521fb 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -329,7 +329,8 @@ PYBIND11_WARNING_POP && defined(_MSC_VER)) /* PyPy Windows: pytest hangs indefinitely at the end of the \ process (see PR #4268) */ \ && !defined(PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF) -# define PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF +// The following define will be enabled by default in the 2.11 release +// define PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF #endif // #define PYBIND11_STR_LEGACY_PERMISSIVE From e414c4bdd64cdc220eabb5eb69420fd49dbb6d55 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Fri, 30 Dec 2022 10:46:55 -0800 Subject: [PATCH 2/7] fix: improve the error reporting for inc_ref GIL failures (#4427) * First * Fixs * Improve * Additional assertions comment * Improve docs --- docs/advanced/misc.rst | 28 ++++++++++++++++++++++++++++ include/pybind11/pytypes.h | 30 +++++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 35a6ebcd6..805ec838f 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -118,6 +118,34 @@ The ``call_go`` wrapper can also be simplified using the ``call_guard`` policy m.def("call_go", &call_go, py::call_guard()); +Common Sources Of Global Interpreter Lock Errors +================================================================== + +Failing to properly hold the Global Interpreter Lock (GIL) is one of the +more common sources of bugs within code that uses pybind11. If you are +running into GIL related errors, we highly recommend you consult the +following checklist. + +- Do you have any global variables that are pybind11 objects or invoke + pybind11 functions in either their constructor or destructor? You are generally + not allowed to invoke any Python function in a global static context. We recommend + using lazy initialization and then intentionally leaking at the end of the program. + +- Do you have any pybind11 objects that are members of other C++ structures? One + commonly overlooked requirement is that pybind11 objects have to increase their reference count + whenever their copy constructor is called. Thus, you need to be holding the GIL to invoke + the copy constructor of any C++ class that has a pybind11 member. This can sometimes be very + tricky to track for complicated programs Think carefully when you make a pybind11 object + a member in another struct. + +- C++ destructors that invoke Python functions can be particularly troublesome as + destructors can sometimes get invoked in weird and unexpected circumstances as a result + of exceptions. + +- You should try running your code in a debug build. That will enable additional assertions + within pybind11 that will throw exceptions on certain GIL handling errors + (reference counting operations). + Binding sequence data types, iterators, the slicing protocol, etc. ================================================================== diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 56e0423ac..3660c180d 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -250,9 +250,9 @@ public: #ifdef PYBIND11_HANDLE_REF_DEBUG inc_ref_counter(1); #endif -#if defined(PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF) +#ifdef PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF if (m_ptr != nullptr && !PyGILState_Check()) { - throw std::runtime_error("pybind11::handle::inc_ref() PyGILState_Check() failure."); + throw_gilstate_error("pybind11::handle::inc_ref()"); } #endif Py_XINCREF(m_ptr); @@ -265,9 +265,9 @@ public: this function automatically. Returns a reference to itself. \endrst */ const handle &dec_ref() const & { -#if defined(PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF) +#ifdef PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF if (m_ptr != nullptr && !PyGILState_Check()) { - throw std::runtime_error("pybind11::handle::dec_ref() PyGILState_Check() failure."); + throw_gilstate_error("pybind11::handle::dec_ref()"); } #endif Py_XDECREF(m_ptr); @@ -296,8 +296,28 @@ public: protected: PyObject *m_ptr = nullptr; -#ifdef PYBIND11_HANDLE_REF_DEBUG private: +#ifdef PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF + void throw_gilstate_error(const std::string &function_name) const { + fprintf( + stderr, + "%s is being called while the GIL is either not held or invalid. Please see " + "https://pybind11.readthedocs.io/en/stable/advanced/" + "misc.html#common-sources-of-global-interpreter-lock-errors for debugging advice.\n", + function_name.c_str()); + fflush(stderr); + if (Py_TYPE(m_ptr)->tp_name != nullptr) { + fprintf(stderr, + "The failing %s call was triggered on a %s object.\n", + function_name.c_str(), + Py_TYPE(m_ptr)->tp_name); + fflush(stderr); + } + throw std::runtime_error(function_name + " PyGILState_Check() failure."); + } +#endif + +#ifdef PYBIND11_HANDLE_REF_DEBUG static std::size_t inc_ref_counter(std::size_t add) { thread_local std::size_t counter = 0; counter += add; From 0abe64c5f6a96a4e7f795e4cd2b2dd0cae27f8d4 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 2 Jan 2023 03:46:17 -0800 Subject: [PATCH 3/7] Fix `detail::obj_class_name()` to work correctly for meta classes. (#4436) * Fix `detail::obj_class_name()` to work correctly for meta classes. * Adjust expected name for PyPy --- include/pybind11/pytypes.h | 2 +- tests/test_class.cpp | 2 ++ tests/test_class.py | 11 ++++++++++- tests/test_pytypes.cpp | 2 ++ tests/test_pytypes.py | 6 ++++++ 5 files changed, 21 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 3660c180d..a2824a0e0 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -463,7 +463,7 @@ PYBIND11_NAMESPACE_BEGIN(detail) // Equivalent to obj.__class__.__name__ (or obj.__name__ if obj is a class). inline const char *obj_class_name(PyObject *obj) { - if (Py_TYPE(obj) == &PyType_Type) { + if (PyType_Check(obj)) { return reinterpret_cast(obj)->tp_name; } return Py_TYPE(obj)->tp_name; diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 9ea10fae1..ca925917e 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -55,6 +55,8 @@ void bind_empty0(py::module_ &m) { } // namespace test_class TEST_SUBMODULE(class_, m) { + m.def("obj_class_name", [](py::handle obj) { return py::detail::obj_class_name(obj.ptr()); }); + // test_instance struct NoConstructor { NoConstructor() = default; diff --git a/tests/test_class.py b/tests/test_class.py index 7c1ed2060..9c964e001 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -1,10 +1,19 @@ import pytest -import env # noqa: F401 +import env from pybind11_tests import ConstructorStats, UserType from pybind11_tests import class_ as m +def test_obj_class_name(): + if env.PYPY: + expected_name = "UserType" + else: + expected_name = "pybind11_tests.UserType" + assert m.obj_class_name(UserType(1)) == expected_name + assert m.obj_class_name(UserType) == expected_name + + def test_repr(): assert "pybind11_type" in repr(type(UserType)) assert "UserType" in repr(UserType) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index ea8d03958..1028bb58e 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -99,6 +99,8 @@ void m_defs(py::module_ &m) { } // namespace handle_from_move_only_type_with_operator_PyObject TEST_SUBMODULE(pytypes, m) { + m.def("obj_class_name", [](py::handle obj) { return py::detail::obj_class_name(obj.ptr()); }); + handle_from_move_only_type_with_operator_PyObject::m_defs(m); // test_bool diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 079ee7ca5..8f9f2987e 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -9,6 +9,12 @@ from pybind11_tests import detailed_error_messages_enabled from pybind11_tests import pytypes as m +def test_obj_class_name(): + assert m.obj_class_name(None) == "NoneType" + assert m.obj_class_name(list) == "list" + assert m.obj_class_name([]) == "list" + + def test_handle_from_move_only_type_with_operator_PyObject(): # noqa: N802 assert m.handle_from_move_only_type_with_operator_PyObject_ncnst() assert m.handle_from_move_only_type_with_operator_PyObject_const() From 050de893cd4e2e9e7257a98420ed588f76aff3f1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 3 Jan 2023 05:46:55 -0800 Subject: [PATCH 4/7] ci: remove clang 10 C++20 (it broke recently) (#4438) * Remove clang 10 C++20 (it broke recently), add clang 15 C++20 while we are at it. * No luck trying clang15: Error response from daemon: manifest for silkeh/clang:15 not found: manifest unknown: manifest unknown. Giving up, this needs to be a separate PR. --- .github/workflows/ci.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af5a29d62..b36bbfe1b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -283,8 +283,6 @@ jobs: include: - clang: 5 std: 14 - - clang: 10 - std: 20 - clang: 10 std: 17 - clang: 11 From 9d6a79c090832afe9b3140fe9ea92142f7f2329c Mon Sep 17 00:00:00 2001 From: cyy Date: Tue, 3 Jan 2023 23:20:39 +0800 Subject: [PATCH 5/7] fix: issuses detected by static analyzer (#4440) * fix incorrect variable check * remove duplicated check * remove unneeded const cast --- include/pybind11/attr.h | 2 +- include/pybind11/eigen/tensor.h | 2 +- include/pybind11/pytypes.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index db7cd8eff..b5e3b7b22 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -399,7 +399,7 @@ struct process_attribute : process_attribute_default { template <> struct process_attribute : process_attribute_default { static void init(const char *d, function_record *r) { r->doc = const_cast(d); } - static void init(const char *d, type_record *r) { r->doc = const_cast(d); } + static void init(const char *d, type_record *r) { r->doc = d; } }; template <> struct process_attribute : process_attribute {}; diff --git a/include/pybind11/eigen/tensor.h b/include/pybind11/eigen/tensor.h index 568c641a2..0877da895 100644 --- a/include/pybind11/eigen/tensor.h +++ b/include/pybind11/eigen/tensor.h @@ -176,7 +176,7 @@ struct type_caster::ValidType> { return false; } - if (!convert && !temp.dtype().is(dtype::of())) { + if (!temp.dtype().is(dtype::of())) { return false; } } diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index a2824a0e0..f11ed5da7 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -501,7 +501,7 @@ struct error_fetch_and_normalize { "active exception."); } const char *exc_type_name_norm = detail::obj_class_name(m_type.ptr()); - if (exc_type_name_orig == nullptr) { + if (exc_type_name_norm == nullptr) { pybind11_fail("Internal error: " + std::string(called) + " failed to obtain the name " "of the normalized active exception type."); From d78de29522d9a12505e67b58f5e417587bea9d72 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 3 Jan 2023 10:21:05 -0500 Subject: [PATCH 6/7] chore(deps): update pre-commit hooks (#4439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/asottile/pyupgrade: v3.3.0 → v3.3.1](https://github.com/asottile/pyupgrade/compare/v3.3.0...v3.3.1) - [github.com/PyCQA/isort: 5.10.1 → 5.11.4](https://github.com/PyCQA/isort/compare/5.10.1...5.11.4) - [github.com/psf/black: 22.10.0 → 22.12.0](https://github.com/psf/black/compare/22.10.0...22.12.0) - [github.com/PyCQA/pylint: v2.15.8 → v2.15.9](https://github.com/PyCQA/pylint/compare/v2.15.8...v2.15.9) - [github.com/shellcheck-py/shellcheck-py: v0.8.0.4 → v0.9.0.2](https://github.com/shellcheck-py/shellcheck-py/compare/v0.8.0.4...v0.9.0.2) - [github.com/pre-commit/mirrors-clang-format: v15.0.4 → v15.0.6](https://github.com/pre-commit/mirrors-clang-format/compare/v15.0.4...v15.0.6) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4e80fbad1..d625d5726 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -41,20 +41,20 @@ repos: # Upgrade old Python syntax - repo: https://github.com/asottile/pyupgrade - rev: "v3.3.0" + rev: "v3.3.1" hooks: - id: pyupgrade args: [--py36-plus] # Nicely sort includes - repo: https://github.com/PyCQA/isort - rev: "5.10.1" + rev: "5.11.4" hooks: - id: isort # Black, the code formatter, natively supports pre-commit - repo: https://github.com/psf/black - rev: "22.10.0" # Keep in sync with blacken-docs + rev: "22.12.0" # Keep in sync with blacken-docs hooks: - id: black @@ -116,7 +116,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v2.15.8" + rev: "v2.15.9" hooks: - id: pylint files: ^pybind11 @@ -160,7 +160,7 @@ repos: # Check for common shell mistakes - repo: https://github.com/shellcheck-py/shellcheck-py - rev: "v0.8.0.4" + rev: "v0.9.0.2" hooks: - id: shellcheck @@ -175,7 +175,7 @@ repos: # Clang format the codebase automatically - repo: https://github.com/pre-commit/mirrors-clang-format - rev: "v15.0.4" + rev: "v15.0.6" hooks: - id: clang-format types_or: [c++, c, cuda] From 0bd8896a4010f2d91b2340570c24fa08606ec406 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 3 Jan 2023 11:34:22 -0500 Subject: [PATCH 7/7] chore: prepare for 2.10.3 (#4437) * docs: update changelog for v2.10.3 Signed-off-by: Henry Schreiner * chore: bump versions for 2.10.3 Signed-off-by: Henry Schreiner * chore: fix make changelog script with entry is empty Signed-off-by: Henry Schreiner Signed-off-by: Henry Schreiner --- docs/changelog.rst | 35 +++++++++++++++++++++++++++++++- include/pybind11/detail/common.h | 4 ++-- pybind11/_version.py | 2 +- tools/make_changelog.py | 9 ++++---- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9ff7281bc..bb111c5f2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,39 @@ IN DEVELOPMENT Changes will be summarized here periodically. +Changes: + +* ``PyGILState_Check()``'s in ``pybind11::handle``'s ``inc_ref()`` & + ``dec_ref()`` are now enabled by default again. + `#4246 `_ + +Build system improvements: + +* Update clang-tidy to 15 in CI. + `#4387 `_ + + +Version 2.10.3 (Jan 3, 2023) +---------------------------- + +Changes: + +* Temporarily made our GIL status assertions (added in 2.10.2) disabled by + default (re-enable manually by defining + ``PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF``, will be enabled in 2.11). + `#4432 `_ + +* Improved error messages when ``inc_ref``/``dec_ref`` are called with an + invalid GIL state. + `#4427 `_ + `#4436 `_ + +Bug Fixes: + +* Some minor touchups found by static analyzers. + `#4440 `_ + + Version 2.10.2 (Dec 20, 2022) ----------------------------- @@ -30,7 +63,7 @@ Changes: * ``PyGILState_Check()``'s were integrated to ``pybind11::handle`` ``inc_ref()`` & ``dec_ref()``. The added GIL checks are guarded by ``PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF``, which is the default only if - ``NDEBUG`` is not defined. + ``NDEBUG`` is not defined. (Made non-default in 2.10.3, will be active in 2.11) `#4246 `_ * Add option for enable/disable enum members in docstring. diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 62bf521fb..12bb173ad 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -11,11 +11,11 @@ #define PYBIND11_VERSION_MAJOR 2 #define PYBIND11_VERSION_MINOR 10 -#define PYBIND11_VERSION_PATCH 2 +#define PYBIND11_VERSION_PATCH 3 // Similar to Python's convention: https://docs.python.org/3/c-api/apiabiversion.html // Additional convention: 0xD = dev -#define PYBIND11_VERSION_HEX 0x020A0200 +#define PYBIND11_VERSION_HEX 0x020A0300 // Define some generic pybind11 helper macros for warning management. // diff --git a/pybind11/_version.py b/pybind11/_version.py index 4c814edfa..63078bbe6 100644 --- a/pybind11/_version.py +++ b/pybind11/_version.py @@ -8,5 +8,5 @@ def _to_int(s: str) -> Union[int, str]: return s -__version__ = "2.10.2" +__version__ = "2.10.3" version_info = tuple(_to_int(s) for s in __version__.split(".")) diff --git a/tools/make_changelog.py b/tools/make_changelog.py index 839040a93..b5bd83294 100755 --- a/tools/make_changelog.py +++ b/tools/make_changelog.py @@ -31,8 +31,10 @@ issues = (issue for page in issues_pages for issue in page) missing = [] for issue in issues: - changelog = ENTRY.findall(issue.body) - if changelog: + changelog = ENTRY.findall(issue.body or "") + if not changelog or not changelog[0]: + missing.append(issue) + else: (msg,) = changelog if not msg.startswith("* "): msg = "* " + msg @@ -44,9 +46,6 @@ for issue in issues: print(Syntax(msg, "rst", theme="ansi_light", word_wrap=True)) print() - else: - missing.append(issue) - if missing: print() print("[blue]" + "-" * 30)