-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Clarify GIL documentation #4057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks again :)
docs/advanced/misc.rst
Outdated
@@ -39,15 +39,35 @@ The ``PYBIND11_MAKE_OPAQUE`` macro does *not* require the above workarounds. | |||
Global Interpreter Lock (GIL) | |||
============================= | |||
|
|||
When calling a C++ function from Python, the GIL is always held. | |||
pybind11 will always ensure the Global Interpreter Lock (GIL) is held from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's great to clarify this section, but I don't think pybind11 does anything actively, which means that "pybind11 will always ensure" isn't accurate.
It's just that there is a general (but not enforced) contract that everyone is meant to adhere to: any thread calling any Python C API function must hold the GIL.
Maybe:
When Python calls into C++ via pybind11, and assuming the process is healthy, the current thread is guaranteed to hold the Global Interpreter Lock (GIL) and pybind11 will never implicitly release it:
new code block here
The classes :class:gil_scoped_release
and :class:gil_scoped_acquire
can be
used to acquire and release the global interpreter lock in the body of a C++
function call. In this way, long-running C++ code can be parallelized using
multiple Python threads, but great care must be taken when any :class:gil_scoped_release
appear: if there is any way that the called C++ code can call back into Python, via callbacks, or trampolines, gil_scoped_acquire
will be needed.
For callbacks based on std::function
(pybind11/functional.h), pybind11 will implicitly manage the GIL, most importantly when a Python callback is passed to
C++ code via std::function
, and the C++ code calls the function, the GIL is acquired before the Python callback is invoked. Similarly, the PYBIND11_OVERRIDE
family of macros will acquire the GIL before calling back into Python.
docs/advanced/misc.rst
Outdated
@@ -56,7 +76,7 @@ could be realized as follows (important changes highlighted): | |||
|
|||
/* Trampoline (need one for each virtual function) */ | |||
std::string go(int n_times) { | |||
/* Acquire GIL before calling Python code */ | |||
/* go is called from C++, so GIL must be acquired before calling Python code */ | |||
py::gil_scoped_acquire acquire; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this documentation is out of date, and I'm not sure what to do about it.
Unless I'm overlooking something, the py::gil_scoped_acquire
here is redundant, because we have this:
pybind11/include/pybind11/pybind11.h
Line 2751 in f9f0049
pybind11::gil_scoped_acquire gil; \ |
I'd say it's best to delete this entire existing code block, because the main point, that the py::gil_scoped_release
below must be matched by a py::gil_scoped_acquire
here is no longer relevant.
When people decide to use py::gil_scoped_release
, they are signing up for all kinds of pitfalls that they have to carefully avoid. An outstanding pain point is when callbacks run while the Python interpreter is finalizing already (e.g. python/cpython#28525). I'm not sure this section is the right place to go into any of that. Giving a comprehensive and accurate overview is not easy. I'd want to limit this section to only state what pybind11 does.
It would be nice to end this section with a tutorial-like reference about the GIL. On and off I've been looking for a good resource to learn more about the GIL, but I've yet to find one that I'd want to reference from here. Does anyone have suggestions?
Updated from feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, thanks Dustin!
I tried to push the diff below, but got "permission denied". Could you please make these trivial changes?
git commit -a -m 'Change 3 `python` to `Python` for consistency.'
--- a/docs/advanced/misc.rst
+++ b/docs/advanced/misc.rst
@@ -47,7 +47,7 @@ will never implicitly release the GIL.
.. code-block:: cpp
void my_function() {
- /* GIL is held when this function is called from python */
+ /* GIL is held when this function is called from Python */
}
PYBIND11_MODULE(example, m) {
@@ -55,9 +55,9 @@ will never implicitly release the GIL.
}
pybind11 will ensure that the GIL is held when it knows that it is calling
-Python code. For example, if a python callback is passed to C++ code via
+Python code. For example, if a Python callback is passed to C++ code via
``std::function``, when C++ code calls the function the built-in wrapper
-will acquire the GIL before calling the python callback. Similarly, the
+will acquire the GIL before calling the Python callback. Similarly, the
``PYBIND11_OVERRIDE`` family of macros will acquire the GIL before calling
back into Python.
Sorry for the delay yet again. Rebased against master, and added your suggestion. |
Thanks Dustin! |
Description
Answered a GIL/pybind11 question on stack overflow, and it seemed like it would be useful to put some of that information into the documentation.
Suggested changelog entry:
The "Global Interpreter Lock (GIL)" section in the documentation was expanded and updated.