All commit authors signed the Contributor License Agreement.

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

I am unable to reproduce the failure oftest_queue, seen inhttps://github.com/python/cpython/actions/runs/14844414631/job/41674826787?pr=133466, even after running the test repeatedly with high levels of--parallel-threads. I will update the branch with a fix for the doc issue shortly.

TSan failure looks unrelated. I've restarted the job for you.

@ZeroIntensity wrote:

I agree with the motivation, butPyGILState_Ensure isevil.

Yikes. I knew it could be trouble but I didn't know it could bethat bad. I'm still reading the PEP and will have more to say once I have finished.

We might want this in a new API (e.g., something likePyErr_CheckSignalsFast).

I would argue that relaxing a requirement is not a breaking change, but I'm willing to give the function a new name if you all think it is necessary.

So I think PEP 788 is trying to solve arelated problem with thread states to the one I have in this PR, but it doesn't directly address my problem.

I set out to makePyErr_CheckSignals be a function that could be calledwhether or not the calling thread has an attached thread state. Perhaps this would be clearer with an example:3e25a93 (the second of the two commits in this PR) makes this abstract change...

     int res;+    Py_BEGIN_ALLOW_THREADS     do {-        Py_BEGIN_ALLOW_THREADS         res = some_system_call(arguments...);-        Py_END_ALLOW_THREADS     } while (res < 0 && errno == EINTR && !PyErr_CheckSignals());+    Py_END_ALLOW_THREADS

tosome but not all of the loops with that structure in the CPython codebase.

PyGILState_Ensure() is the onlyexisting C-API function (as far as I can tell) that you're allowed to call fromeither inside or outside aPy_BEGIN_ALLOW_THREADS ... Py_END_ALLOW_THREADS block, and it promises that when it returns you'll have an attached thread state.PyEval_RestoreThread (either directly or in the form ofPy_BLOCK_THREADS) deadlocks in GIL builds if called by a thread that already holds the GIL, so that's no good; the lower-level_PyThreadState_Attach appears to have the same problem;PyThreadState_Swap does something unrelated.

ButPyGILState_Ensure does considerably more than what I need. If I understood PEP 788 correctly, it's the bits I don't need -- particularly,creation of a temporary thread state for a thread that has never had one -- that are troublesome. PEP 788 sets out to fix those bits, but itdoesn't add a new API that does the specific thing I need ("reattach this thread's stateif it's detached") without any of the extra bits.

Tangentially, part of the problem is thatPyErr_CheckSignals doesn't take an interpreter or thread-state argument, so Ido needPyGILState_Ensure's machinery for guessing which thread state to reattach; this is perhaps an argument for leaving it alone and adding aPyErr_CheckSignalsDetached(tstate) function as suggested above. Connected to that is thatPy_BEGIN_ALLOW_THREADS doesn'tovertly give you the thread state handle it just detached; it's a hidden variable. I'm tempted to suggest soft-deprecating BEGIN/END_ALLOW_THREADS in favor of a new macro which would be used like this (for the same sample loop as above):

intres;Py_WITH_DETACHED_TSTATE (ts) {do {res=some_system_call(arguments...);         }while (res<0&&errno==EINTR&& !PyErr_CheckSignalsDetached(ts));    }

(A macro like this can be implemented in plain C by off-label use of afor loop.)

I'm open to helping with work toward this end, but I would very much like to find a way to do itindependently of this PR, which I would like to keep focused on the goal described in#133465.

Ithink we don't have to worry about the cross-interpreter problems because subinterpreters can't handle signals anyway--that's a job for the main thread and main interpreter. My approach would be to checkPyGILState_GetThisThreadState() == &_PyRuntime._main_interpreter._initial_thread. That shields us from any concurrent finalization shenanigans, so it should be a bit more thread-safe. If that passes,PyGILState_Ensure should be safe.

Also cc@encukou@vstinner as members of the C API WG. I'm not sure what the rules are for extending functionality of stable APIs. Should this be in a new function?

The main issue with this kind of extension is that code tested with new versions of Python will fail in older ones.
Itcan be extended, but I'd prefer adding a new function instead.

So I think we have consensus that this PR should introduce a new function rather than change the semantics of an existing one. I propose to call the new functionPyErr_CheckSignalsDetached and I'll go ahead and start working on that change.

I see some related design issues that need to be resolved, though:

• Should the new function take aPyThreadState* argument?

  If it does, it's substantially easier to implement, but callers need to supply that argument, which can be difficult, both because it might need to be passed down through several layers of intermediate functions and becausePy_BEGIN_ALLOW_THREADS does not officially give one access to aPyThreadState (the_state variable it declares is an undocumented implementation detail).

• Should one be allowed to call the new functionwith or without an attached thread state, or onlywithout?

  Again this is a tradeoff between implementation and usage convenience. Implementation would be easier if we required users of the new function tonot have an attached thread state. As far as I can tell, theonly function with the semantics of "give me an attached thread stateif I don't already have one, do nothing if I do" isPyGILState_Ensure, which, per PEP 788, we'd rather not introduce new uses of.PyEval_RestoreThread is documented to deadlock if called by a thread that already has an attached thread state;PyThreadState_Swap is documented to require the outgoing thread state to be attached.

  However, it would be substantially easier for extension module authors touse the new API if they didn't have to worry about whether they did or didn't hold an attached thread state at each point where they want to introduce a signals check/cancellation point.

• Should the new function run the cycle collector?

  PyErr_CheckSignals only actually checks for signals if it's running in the main interpreter on the main thread. However, regardless of which interpreter or thread it's running on, it may run the cycle collector (the "do we need to run the cycle collector soon?" flag is also checkable without the GIL/an attached thread state). This is not currently documented (this PR adds it to the documentationen passant).

  If the new function shouldn't try to run the cycle collector, it's easier to implement; in particular, the question of how we access thecorrect thread state becomes simpler. However, the rationale forPyErr_CheckSignals having the side effect of running the cycle collector is

  Opportunistically check if the GC is scheduled to run and run it if we have a request. This is done here because native code needs to call this API if is going to run for some time without executing Python code to ensure signals are handled. Checking for the GC here allows long running native code to clean cycles created using the C-API even if it doesn't run the evaluation loop

  which would seem to apply equally to the newPyErr_CheckSignalsDetached.

• While we're in here, do we needis_tripped anymore?

  The actual C signal handler setsthree semi-redundant flags: the per-signal&Handlers[sig_num].tripped, the global variableis_tripped, and the_PY_SIGNALS_PENDING_BIT bit in the eval breaker word for_PyRuntime.main_tstate. As far as I can tell,is_tripped is fully redundant to the eval breaker bit. Is there anything that needs it that isn't apparent to a recursive grep of the CPython source tree?

Let's open an issue with theC API working group so they can bicker about bikeshedding and the API.

In person at the sprints, Guido suggested that this PR should be split up into three: one each for the core functional changes, the docs changes, and the "look what we can do now" changes to various stdlib modules. I'll make that happen, but not until Wednesday. Meantime, can we please focus discussion on the list of questions I posted?@ZeroIntensity I'd appreciate it if you could open that issue; I'm tired enough that I'm making foolish mistakes.

@zackw: Sorry we didn't get a chance to say goodbye at the sprint.

Let me try answering your questions, briefly.

* [ ]  Should the new function take a `PyThreadState*` argument?

I agree with your current choice to not ask for a thread state -- we want the function to be as convenient as possible, and to add minimum friction.

* [ ]  Should one be allowed to call the new function _with or without_ an attached thread state, or only _without_?

I think only without. I can imagine many patterns that make it easier for extensions to choose betweenPyErr_CheckSignalsandPyErr_CheckSignals_Detached.

* [ ]  Should the new function run the cycle collector?

I think not. Calling the cycle collector is important when holding the GIL because we might be creating/updating/destroying Python objects. But when not holding the GIL we shouldn't be doing that, so the set of objects shouldn't change at all! Other threads that are creating lots of objects from C should be callingPyErr_CheckSignals(), so they'll run the GC (unlike signal handlers, it's not limited to the main thread).

* [ ]  While we're in here, do we need `is_tripped` anymore?  The actual C signal handler sets _three_ semi-redundant flags: the per-signal `&Handlers[sig_num].tripped`, the global variable `is_tripped`, and the `_PY_SIGNALS_PENDING_BIT` bit in the eval breaker word for `_PyRuntime.main_tstate`.  As far as I can tell, `is_tripped` is fully redundant to the eval breaker bit.  Is there anything that needs it that isn't apparent to a recursive grep of the CPython source tree?

I don't know, but it sounds like a "here be dragons" area to me. Let's not try to fix this and then found out we shouldn't have. Someone else (or you :-) could endeavor to removeis_tripped at a later point.

It does look like you need to do a manual merge of a newer main branch before we merge this, since the branch is flagged as having conflicts. I recommend doing that at the latest possible moment (e.g. when you've received all the Approvals you need) so you won't have to do twice. Maybe moving the docs and other files to new PRs will remove the conflict.

Should the new function take aPyThreadState* argument?Py_BEGIN_ALLOW_THREADS does not officially give one access to a PyThreadState (the _state variable it declares is an undocumented implementation detail).

I'll dissent here. It should, just likePy_END_ALLOW_THREADS explicitly grabs the locally saved state.

A new macro likePy_CHECK_SIGNALS would both keep the secret variable hidden and make this easy to use.

(For the record, it'sdocumented, the question being is whether that's a guarantee of how things work or just an illustration. In my opinion, it is at least an implementation guide for non-C/C++ users.)

Should the new function take aPyThreadState* argument?Py_BEGIN_ALLOW_THREADS does not officially give one access to a PyThreadState (the _state variable it declares is an undocumented implementation detail).

I'll dissent here. It should, just likePy_END_ALLOW_THREADS explicitly grabs the locally saved state.

A new macro likePy_CHECK_SIGNALS would both keep the secret variable hidden and make this easy to use.

(For the record, it'sdocumented, the question being is whether that's a guarantee of how things work or just an illustration. In my opinion, it is at least an implementation guide for non-C/C++ users.)

I don't understand why those save/restore operations need to be done using a macro. They could just as easily be done by a function that does that save/restore behavior itself. I don't want to turn the old PyErr_CheckSignals (unchanged in functionality, just refactored) into a macro, and I don't believe things are so perf-critical that the first checks in PyErr_CheckSignals_Detached need to be a macro.

So I still don't think there's anything wrong with calling a function that retrieves and restores the tstate.

Well, explicit is better than implicit.

The code betweenPy_BEGIN_ALLOW_THREADS andPy_END_ALLOW_THREADS is usually low-level, but that's not a requirement. It can do anything. Including attaching thread states to do Python work, or even making (sub)interpreters come and go. How sure can we be that thethis state can't be clobbered?

For example:

#include<Python.h>#include<stdio.h>#defineN_ITERATIONS 2staticvoidcall_into_some_library(void);// For simplicity, store our main interpreter globallyPyInterpreterState*main_interp=NULL;staticPyObject*demo_func(PyObject*mod,PyObject*arg) {main_interp=PyInterpreterState_Get();printf("Start! This thread state = %p\n",PyGILState_GetThisThreadState());Py_BEGIN_ALLOW_THREADS;for (inti=0;i<N_ITERATIONS;i++) {// Do some C work using some library...call_into_some_library();// if PyGILState_GetThisThreadState() is NULL, the// PyErr_CheckSignals from this PR will crashprintf("%d/%d: this thread state = %p\n",i,N_ITERATIONS,PyGILState_GetThisThreadState());    }Py_END_ALLOW_THREADS;Py_RETURN_NONE;}// Meanwhile, the library wants to do some Python...staticvoidcall_into_some_library(void) {/* ... */if (PyThreadState_GetUnchecked()==NULL) {// no thread state attached!// so make one & attach itPyThreadState*ts=PyThreadState_New(main_interp);PyEval_AcquireThread(ts);// call some fun Python APIPyObject*s=PyUnicode_FromString("all OK down here\n");PyObject_Print(s,stdout,Py_PRINT_RAW);Py_DECREF(s);// and get rid of the thread statePyThreadState_Clear(ts);PyThreadState_DeleteCurrent();// by this we have reset "this" thread state to NULL    }else {/* ... */ }}staticPyModuleDefmodule= {    .m_name="extension",    .m_methods= (PyMethodDef[]) {        {"demo_func",demo_func,METH_NOARGS},        {0},    },};PyMODINIT_FUNCPyInit_extension(void) {returnPyModuleDef_Init(&module);}

I don't understand why those save/restore operations need to be done using a macro. They could just as easily be done by a function that does that save/restore behavior itself.

I agree with that part, the macros could just have been functions. But they should be explicit about the state they want to get back to.

The new function is implemented withPyGILState_Ensure() andPyGILState_Release() which doesn't support sub-interpreters. I'm not sure if it's a good idea to add such new function. We may check the wrong interpreter. The currentPyErr_CheckSignals() API doesn't have such issue.

PyGILState_STATEst=PyGILState_Ensure();interr=check_signals_attached(tstate,cycle_collect);PyGILState_Release(st);

Itmight be fine, because subinterpreters don't currently support handling signals anyway. But, I don't know if that will always be the case. I was talking to Eric at the sprints about handling signals in any interpreter active in the main thread.

It might be fine, because subinterpreters don't currently support handling signals anyway.

Let's say that the function is called in asubinterpreter running in the main thread, andPyGILState_Ensure() picks instead the main thread of themain interpreter: we can handle signals in this case, whereas subinterpreters doesn't support handling signals. There is a problem, no?

Hmm, yeah, that doesn't sound great.

Yeah, I think we need a somewhat different design to handle subinterpreters and free threading smoothly.

And PyErr_CheckSignals may need to be changed as well? Maybe the forward-thinking approach would be a single function then??? (Sorry@zackw).

int PyErr_CheckSignals_Detached(PyInterpreterState *interp) is probably good enough. I don't think we'll need anything different for free-threading.

I'm not convinced that we have to provide a way to "Allow PyErr_CheckSignals to be called without holding the GIL".PEP 788 shows that it's quite complicated to acquire/release the GIL, there are corner cases and we need a new (more elaborated) API to prevent bugs/crashes.

int PyErr_CheckSignals_Detached(PyInterpreterState *interp)

Is there a risk that the interpreter pointer can become a dangling pointer?

Is there a risk that the interpreter pointer can become a dangling pointer?

Yes, but that's true of the existing APIs too. If PEP 788 is accepted, we can change this to use a strong interpreter reference.

The function already checks that it is the main thread. Is there a way to check that we’re in the main interpreter? Assuming we can check those without having the GIL or a tstate, would that simplify the requirements? Or is there still ambiguity?

I think the goal is to eventually get signal handling working in subinterpreters that are running in the main thread too.

Obligatory ping@ericsnowcurrently

Yeah, that's a possibility. We'll have to see what makes the most sense.

I think the goal is to eventually get signal handling working in subinterpreters that are running in the main thread too.

Let's let the folks working on that feature worry about it.

Butis there way to check that you're in the main interpreter?

_Py_IsMainInterpreter