Skip to content

Commit

Permalink
fix: improve the error reporting for inc_ref GIL failures (#4427)
Browse files Browse the repository at this point in the history
* First

* Fixs

* Improve

* Additional assertions comment

* Improve docs
  • Loading branch information
EthanSteinberg authored and henryiii committed Jan 2, 2023
1 parent 2de6e39 commit e414c4b
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 5 deletions.
28 changes: 28 additions & 0 deletions docs/advanced/misc.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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<py::gil_scoped_release>());
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.
==================================================================

Expand Down
30 changes: 25 additions & 5 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,9 @@ class handle : public detail::object_api<handle> {
#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);
Expand All @@ -265,9 +265,9 @@ class handle : public detail::object_api<handle> {
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);
Expand Down Expand Up @@ -296,8 +296,28 @@ class handle : public detail::object_api<handle> {
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;
Expand Down

0 comments on commit e414c4b

Please sign in to comment.