gh-124622: Add PyGILState_EnsureOrFail() function

[vstinner]

Add new "OrFail" functions:

• _PyEval_AcquireLockOrFail()
• _PyEval_RestoreThreadOrFail()
• _PyThreadState_AttachOrFail()
• take_gil_or_fail()

• Issue: Fail PyGILState_Ensure() If Finalizing? #124622

---

📚 Documentation preview 📚: https://cpython-previews--129688.org.readthedocs.build/

[vstinner]

See the discussion: https://discuss.python.org/t/safely-using-the-c-api-when-python-might-shut-down/78850

[vstinner]

cc @gpshead @ZeroIntensity

[ZeroIntensity]

As I said in the issue, I really don't want new GILState APIs, because they're quite broken for subinterpreters and misleading for free-threading (FT users sometimes think they don't need a tstate to use the C API).

I'd rather call this PyThreadState_Ensure, or something along those lines, and then deprecate PyGILState. cc @encukou

[pitrou]

This is confusing and misleading. The C or C++ thread must not exit. It's just that the function failed taking the GIL, probably because Python is finalized.

[pitrou]

LGTM on the principle, but see comments below.

[wjakob]

2cts from me: for this to be useful for extensions, it would be good if it is immediately part of the stable ABI. The implementation looks fine except for comment describing the function.

I would change it to:

This function resembles :c:func:`PyGILState_Ensure`, except that it returns with a status code even in the case of failure. 
Specifically, it returns ``0`` when the operation succeeded, and ``-1`` otherwise. In contrast to :c:func:`PyGILState_Ensure`, *state* is an argument. 

In the case of failure, it is *unsafe* to use the Python API following the call. Releasing the obtained *state* via :c:func:`PyGILState_Release` should only be done in the case of success.

[vstinner]

for this to be useful for extensions, it would be good if it is immediately part of the stable ABI.

Ok, done.

The implementation looks fine except for comment describing the function. I would change it to: (...)

Done.

[vstinner]

I'd rather call this PyThreadState_Ensure, or something along those lines, and then deprecate PyGILState. cc @encukou

If we adopt a different API, I would suggest avoiding also the enum, use a simple int instead.

API: int PyThreadState_Ensure(void):

• On success, return a state >= 0 which should be passed to PyThreadState_Release().
• On error, return -1.

API: void PyThreadState_Release(int state)

Usage:

int state = PyThreadState_Ensure();
if (state < 0) return;

// ... use the Python C API ...

PyThreadState_Release(state);

[encukou]

I'd rather call this PyThreadState_Ensure, or something along those lines, and then deprecate PyGILState

I'd also prefer that, but AFAIK such a function would need an extra argument (to specify which interpreter you want to end up in). Is that right?
Extensions would need to pass that value through their callback mechanisms.

As an incremental improvement for current users of PyGILState_Ensure, this looks good. But let's not close the issue with this PR.

Could you add the mention that these are “not compatible with sub-interpreters” directly to the docs?

This should go through the C API WG.

[vstinner]

Could you add the mention that these are “not compatible with sub-interpreters” directly to the docs?

Which API should be recommended instead, if this one is not compatible with sub-interpreters?

[ZeroIntensity]

PyThreadState_Swap, or anything of that nature.

[vstinner]

PyThreadState_Swap, or anything of that nature.

PyThreadState_Swap() requires a tstate. How do you create the tstate in a new thread to support sub-interpreters?

[ZeroIntensity]

Py_NewInterpreter or Py_NewInterpreterFromConfig, but creating the interpreter isn't the problem with PyGILState. The issue is that if you use PyGILState_Ensure in a thread that was created by a subinterpreter, then that thread will have the GIL of the main interpreter, not the subinterpreter. That means the thread can't safely interact with the subinterpreter in any way, but it will just end up racing instead of failing, which is really bad.

I think turning this API into PyThreadState_Ensure (along with deprecating PyGILState) would be good incentive for people to switch in 3.14. Obligatory ping @ericsnowcurrently

[pitrou]

Indeed it might be a good idea to add:

int PyThreadState_Ensure(PyInterpreterState*);
void PyThreadState_Release(int state, PyInterpreterState*);

[ZeroIntensity]

An int handle is probably extra work. Why not just advise use of PyThreadState_DeleteCurrent?

[vstinner]

I created #130012 to add PyThreadState_Ensure() and PyThreadState_Release() functions.

[encukou]

Could you add the mention that these are “not compatible with sub-interpreters” directly to the docs?
Which API should be recommended instead, if this one is not compatible with sub-interpreters?

I meant repeating the note above PyGILState_Ensure that says The following functions use thread-local storage, and are not compatible with sub-interpreters:. Users that click the What's New link go directly to the function docs and are quite likely to miss the warning.
Something like:

Suggested change

	Like :c:func:`!PyGILState_Ensure`, this function is not safe to use with
	supinterpreters, since it is not possible to specify which interpreter
	is activated.

[vstinner]

#130012 PR should be subinterpreter-friendly, I closed this PR instead.

[encukou]

Suggested change

	operation succeeded, and ``-1`` otherwise. In contrast to
	:c:func:`PyGILState_Ensure`, *state* is an argument.
	operation succeeded, and ``-1`` otherwise.
	The return value of :c:func:`!PyGILState_Ensure`
	is stored in *\*state*.

[encukou]

Suggested change

	call. Releasing the obtained *state* via :c:func:`PyGILState_Release` should
	call. Releasing the obtained *state* via :c:func:`PyGILState_Release` must

[vstinner]

I applied this suggestion to #130012 since I closed this PR.

[vstinner]

I created #130012 to add PyThreadState_Ensure() and PyThreadState_Release() functions.

It seems like the trend is more towards passing interp explicitly, so I close this PR.