A new API for ensuring/releasing thread states

[ZeroIntensity]

History

There's been a desire from users to have an API to safely acquire the GIL (or really, an attached thread state) when Python could finalize, which isn't possible with PyGILState_Ensure. The problem is that even with a check like Py_IsFinalizing, the runtime could still finalize during the call to PyGILState_Ensure, which will probably end up crashing. There's an open issue about this too: python/cpython#124622

@vstinner created a PR in response to that discussion, implementing a PyGILState_EnsureOrFail function: python/cpython#129688

I had some concerns about the naming of that function. Primarily, the term "GIL" isn't something we want in the public API anymore; there can be more than one GIL (so "the GIL" isn't something that really exists), and the GIL can be disabled entirely on free-threaded builds. I've been on a bit of a quest to try and remove as much of this as possible.

PyGILState is especially bad, because some library developers erroneously believe that PyGILState_Ensure isn't needed on FT builds, and who knows what they might think about subinterpreters. (For example, in python/cpython#123134, a user thought that they could flat-out omit PyGILState_Ensure entirely, as the docs said it wasn't supported--we definitely don't want that in the future.)

Additionally, a few months ago, @ericsnowcurrently, @encukou, and I discussed some of the implications that PyGILState_Ensure has on subinterpreters in python/cpython#123728. If a thread spawned by a subinterpreter were to call PyGILState_Ensure, then the thread will have the GIL of the main interpreter, rather than the subinterpreter. That means the thread can't safely interact with the calling interpreter at all.

To humor me, Victor created a second PR which spells PyThreadState_Ensure instead: python/cpython#130012

Proposal

So, what's the best way forward? Back in September, Petr had a few good suggestions for a new thread-state-ensuring API, most importantly taking an explicit interpreter rather than an implicit one. To help incentivize users to shy away from PyGILState_Ensure, I think we can tie a new API into also being thread-safe at finalization.

I want to propose two new functions:

PyStatus PyThreadState_Ensure(PyInterpreterState *interp); // analogous to PyGILState_Ensure
void PyThreadState_Release(); // analogous to PyGILState_Release

Open Issues

• I've chosen PyStatus as the return value, because it's useful for indicating what went wrong, rather than just a -1 return with no exception set (e.g., in userspace, I could see a desire for differentiating between whether it failed because Python is finalizing or because the program is out of memory). PyStatus is more clunky to use, though.
• We might want to use an interpreter ID rather than an interpreter state pointer, because an interpreter might exit and its state will get invalidated. It's pretty easy to check, though--just iterate through all the interpreters and check if the pointer is in there. But my main concern is whether that will always be viable; would an int interp_id have less maintenance burden?
• Victor's PR currently reuses PyGILState_STATE to determine when it's time to delete the thread state in nested calls, but I don't think that's sufficient (or nice to use) here. We can avoid any int pointer at all by just storing a call-counter on the thread state itself, and deleting it (along with switching to a prior thread state) when a PyThreadState_Release call sets it to zero.

Deprecate PyGILState?

If we add a new PyThreadState_Ensure/PyThreadState_Release API, I think it would be good to fully deprecate all PyGILState APIs. Even in the public API today, you can replicate everything in PyGILState with a better PyThreadState API:

• PyGILState_Ensure -> PyThreadState_Swap/PyThreadState_New (would instead be PyThreadState_Ensure)
• PyGILState_Release -> PyThreadState_Clear/PyThreadState_Delete (would instead be PyThreadState_Release)
• PyGILState_GetThisThreadState -> PyThreadState_Get
• PyGILState_Check -> PyThreadState_GetUnchecked() != NULL

[davidhewitt]

Even in the public API today, you can replicate everything in PyGILState with a better PyThreadState API:

I would be interested in trying out how this works out in PyO3. My main concern is about where to store the PyThreadState, presumably in a TLS. I think PyThreadState_Ensure would presumably solve that concern.

[encukou]

May I suggest putting this proposal on Discourse, to build consensus that the WG would then rubber-stamp (or bicker over details like whether to use int or a pointer.)

We haven't always been great at doing that, but I think this should have a slightly wider audience.

[ZeroIntensity]

I could bring it to DPO/Discourse, but I'm worried that we'll come right back with little or no changes to what I've proposed. What kind of feedback are you looking for?

But if we're going all the way to Discourse, it also might be worth turning this into a small PEP, considering how widely used PyGILState_Ensure is (and how disruptive a deprecation might be), but I personally don't see the need. I'm happy to write something up, though.

[encukou]

I could see this becoming a PEP, especially if we go all the way and (soft-)deprecate PyGILState_* in favour of PyThreadState_*.

We might want to use an interpreter ID rather than an interpreter state pointer, because an interpreter might exit and its state will get invalidated. It's pretty easy to check, though--just iterate through all the interpreters and check if the pointer is in there. But my main concern is whether that will always be viable; would an int interp_id have less maintenance burden?

That's a good point. The proposed API seems to encourage stashing away a long-lived pointer, and praying that shutdown will work somehow.

I wonder if we should add (optional) reference counting to interpreter states.

[ZeroIntensity]

I wonder if we should add (optional) reference counting to interpreter states.

I think we already do! See _interpreters.incref/_interpreters.decref, and there's a few private C APIs for it. I just don't know if it really works yet.

[ZeroIntensity]

@encukou @vstinner I'd like to see this in 3.14 before the beta freeze, so I'm bumping this. Would you like me to make a DPO post to gather further consensus?

[encukou]

Yes. DPO is the best place to close those 3 open issues.

[vstinner]

Can we close this issue until a consensus is reached on Discourse?

[ZeroIntensity]

Yeah, I think this is probably worthy of its own PEP, considering that it looks like we'll need additional APIs to make this useful. This won't make it to 3.14 :(