GeezerVison is showing github.com — Make collections.deque thread-safe in `--disable-gil` builds · Issue #112050 · python/cpython
Make collections.deque thread-safe in --disable-gil builds #112050DescriptionFeature or enhancement
The collections.deque object has mutable internal state that would not be thread-safe without the GIL. We should use the critical section API to make it thread-safe. I think it might be cleanest to first convert most of the deque implementation to Argument Clinic and use the @critical_section directive rather than writing the critical sections manually.
Mostly for my own reference, here is the implementation from nogil-3.12: colesbury/nogil-3.12@f1e4742eaa. That implementation did not use the critical section API, which made it more complicated.
Depends on: #111903
Linked PRs
Reacted by Erlend E. Aasland ActivityHels15 commented on Nov 21, 2023 Can I work on this or someone is already assigned? @colesbury colesbury commented on Nov 27, 2023 ContributorAuthor More actions Sure - nobody else has started work on this yet. mpage commented on Dec 18, 2023 @Hels15 - Would you mind if I took this over? Hels15 commented on Dec 18, 2023
@Hels15 - Would you mind if I took this over?
Feel free to work on this. I haven't had much time lately, unfortunately. mpage commented on Dec 21, 2023 @colesbury - Have you run into any TOCTTOU / stale data bugs when making containers thread-safe? One thing that comes to mind as I'm converting this is that we have to be careful to revalidate any invariants that were established (or reload any fields) across any operations that may release the container's lock (implicitly if we're using critical sections). I guess these could still happen with the GIL, so maybe we're not actually any worse off. colesbury commented on Dec 21, 2023 ContributorAuthor More actions @mpage - The most common thing is you want to avoid starting a nested critical section on the same object. That's both slow and can implicitly end the critical section (invalidating assumptions). Other than that, it's basically the same concerns as with the GIL. You have to be careful with, for example, Py_DECREF calls that may implicitly release the gil/release the containers lock.
I think it's a good strategy to push the critical sections to the outer-most functions. That's easiest with Argument Clinic because you can generate the critical section calls as part of the binding code with the @critical_section directive. It's may be worth first converting deque to use argument clinic -- it'll probably be cleaner and easier to understand that sprinkling Py_BEGIN_CRITICAL_SECTION() calls all over the place. Reacted by Erlend E. Aasland mpage commented on Dec 21, 2023
It's may be worth first converting deque to use argument clinic...
Oh, yeah, that's the route I'm going (as suggested in the issue). I was mostly wondering what new classes of issues we need to pay attention to when making existing code thread-safe without the GIL. With the GIL you still have to be careful about revalidating assumptions across code that could release the GIL. If we push critical sections out to the binding functions using clinic, then I think the places where the critical section / container lock can be released are basically the same as where the GIL could be released, so making existing functions thread-safe using this strategy won't require substantial rewrites to the code. Anyway, I'm sure this is stuff you've already thought about and why you developed critical sections and the clinic strategy, but writing it out helps clarify my understanding, so thanks for humoring me :) added a commit that references this issue on Feb 15, 2024gh-112050: Make collections.deque thread-safe in free-threaded builds (…
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|