-
Notifications
You must be signed in to change notification settings - Fork 782
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
docs: Expand docs on when and why allow_threads is necessary #4767
base: main
Are you sure you want to change the base?
Conversation
8b47a5d
to
c2bb87d
Compare
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.
noticed a grammar issue on my phone
615d8f2
to
b71acef
Compare
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.
When I read this next to https://docs.python.org/3.13/c-api/init.html I notice there's a mismatch in the terminology used. For example this documentation uses (and used) "attach" and "detach" a lot, but the C api docs don't - they talk about setting, resetting and associating threadstate. I think it would be clearer if we used the same terminology.
Otherwise it looks great, thank you!
guide/src/free-threading.md
Outdated
objects or call into the CPython C API. If you are not yet attached to the | ||
Python runtime, you can register a thread using the [`Python::with_gil`] | ||
function. Threads created via the Python [`threading`] module do not not need to | ||
do this, but all other OS threads that interact with the Python runtime must | ||
explicitly attach using `with_gil` and obtain a `'py` liftime. | ||
|
||
Since there is no GIL in the free-threaded build, releasing the GIL for | ||
long-running tasks is no longer necessary to ensure other threads run, but you | ||
should still detach from the interpreter runtime using [`Python::allow_threads`] | ||
when doing long-running tasks that do not require the CPython runtime. The | ||
garbage collector can only run if all threads are detached from the runtime (in | ||
a stop-the-world state), so detaching from the runtime allows freeing unused | ||
memory. | ||
do this, and pyo3 will handle setting up the [`Python<'py>`] token when CPython | ||
calls into your extension, but all other OS threads that interact with the | ||
Python runtime must explicitly attach using `with_gil` and obtain a `'py` | ||
liftime. |
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.
This is a lot of words to say "you still need to call with_gil
and use Python
, to ensure that the current thread has a Python threadstate associated with it". Can we pare it down?
Co-authored-by: Bruno Kolenbrander <[email protected]>
We intentionally went with the attach/detach terminology in #4577 (see e.g. #4577 (review)). Let me see if I can make it make sense using "save/restore a thread state"... |
I just asked about this issue on the CPython discussion forum: https://discuss.python.org/t/c-api-docs-for-free-threading/74183. IMO the CPython docs need free-threading specific copy here, and the terminology we're using about attaching and detaching from the runtime (IMO) makes more sense when thinking about the free-threaded build. |
There's at least some interest to change the CPython docs to emphasize having an "attached thread state" over just "holding the GIL". |
That's some good news :) Please disregard my terminology comment then. |
This is a response to the confusion encountered in #4738
I verified that the example I added in
parallelism.md
runs sucessfully despite the fact that it's decorated withrust,no_run
.