Make collections.deque thread-safe in --disable-gil builds

[colesbury]

Feature 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

• gh-112050: Make collections.deque thread-safe in free-threaded builds #113830
• gh-112050: Convert most methods on collections.deque to use Argument Clinic #113963

[Hels15]

Can I work on this or someone is already assigned? @colesbury

[colesbury]

Sure - nobody else has started work on this yet.

[mpage]

@Hels15 - Would you mind if I took this over?

[Hels15]

@Hels15 - Would you mind if I took this over?

Feel free to work on this. I haven't had much time lately, unfortunately.

[mpage]

@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]

@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.

[mpage]

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 :)