From eb1013870874a86ba2bc28e117c355a0e284983e Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 27 Dec 2022 12:27:52 -0800 Subject: [PATCH 1/5] First --- include/pybind11/pytypes.h | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 56e0423ac4..98433b3be4 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -252,7 +252,7 @@ class handle : public detail::object_api { #endif #if defined(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); @@ -267,7 +267,7 @@ class handle : public detail::object_api { const handle &dec_ref() const & { #if defined(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); @@ -298,6 +298,22 @@ class handle : public detail::object_api { #ifdef PYBIND11_HANDLE_REF_DEBUG private: + void throw_gilstate_error(const std::string &function_name) const { + fprintf(stderr, + "%s is being called while PyGILState_Check() is failing. " + "This usually indicates that you are calling pybind11 functions without holding " + "the GIL or before " + "Python (and/or this Python module) is finished initializing.\n", + function_name.c_str()); + 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); + } + throw std::runtime_error(function_name + " PyGILState_Check() failure."); + } + static std::size_t inc_ref_counter(std::size_t add) { thread_local std::size_t counter = 0; counter += add; From c615629bc38cc33dfd7c9daf7bacac271235d9f2 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 27 Dec 2022 22:05:00 -0800 Subject: [PATCH 2/5] Fixs --- docs/advanced/misc.rst | 23 +++++++++++++++++++++++ include/pybind11/pytypes.h | 17 ++++++++++------- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 35a6ebcd60..1e22191189 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -118,6 +118,29 @@ 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 +common 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 are a particular troublesome setting as +destructors can sometimes get invoked in weird and unexpected circumstances. + + Binding sequence data types, iterators, the slicing protocol, etc. ================================================================== diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 98433b3be4..bed7b6a97b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -250,7 +250,7 @@ class handle : public detail::object_api { #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_gilstate_error("pybind11::handle::inc_ref()"); } @@ -265,7 +265,7 @@ class handle : public detail::object_api { 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_gilstate_error("pybind11::handle::dec_ref()"); } @@ -296,24 +296,27 @@ class handle : public detail::object_api { 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 PyGILState_Check() is failing. " - "This usually indicates that you are calling pybind11 functions without holding " - "the GIL or before " - "Python (and/or this Python module) is finished initializing.\n", + "%s is being called while the GIL is either not held or invalid. Please see " + "https://pybind11.readthedocs.io/en/stable/advanced/" + "misc.html#global-interpreter-lock-gil 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 8287490e1b22ef5e989be80e55e3841133cba123 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 27 Dec 2022 22:12:13 -0800 Subject: [PATCH 3/5] Improve --- docs/advanced/misc.rst | 28 +++++++++++++++------------- include/pybind11/pytypes.h | 11 ++++++----- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 1e22191189..967f88d1b1 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -126,19 +126,21 @@ 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 -common 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 are a particular troublesome setting as -destructors can sometimes get invoked in weird and unexpected circumstances. +- 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. Binding sequence data types, iterators, the slicing protocol, etc. diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index bed7b6a97b..3660c180d9 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -299,11 +299,12 @@ class handle : public detail::object_api { 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#global-interpreter-lock-gil for debugging advice.\n", - function_name.c_str()); + 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, From db38f69e32e6d58247511e3f5bae46cbfef201b9 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Wed, 28 Dec 2022 09:53:44 -0800 Subject: [PATCH 4/5] Additional assertions comment --- docs/advanced/misc.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 967f88d1b1..f789bd3fcc 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -142,6 +142,8 @@ following checklist. 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 can throw exceptions on some GIL errors, in particular during reference counting operations. Binding sequence data types, iterators, the slicing protocol, etc. ================================================================== From e00557c0bb6d6ac34b486b13f15962d4b7f856f2 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Thu, 29 Dec 2022 06:24:24 -0800 Subject: [PATCH 5/5] Improve docs --- docs/advanced/misc.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index f789bd3fcc..805ec838fc 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -143,7 +143,8 @@ following checklist. of exceptions. - You should try running your code in a debug build. That will enable additional assertions - within pybind11 that can throw exceptions on some GIL errors, in particular during reference counting operations. + within pybind11 that will throw exceptions on certain GIL handling errors + (reference counting operations). Binding sequence data types, iterators, the slicing protocol, etc. ==================================================================