Repository navigation
_interpreters is not thread safe on the free-threaded build #126644
Description
Activity
- addedtype-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump
on Nov 10, 2024 - addedextension-modulesC modules in the Modules dirC modules in the Modules dir3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixes
on Nov 10, 2024 Confirmed on main. I'll try and deal with this today.
Reacted by devdanzin- changed the title
[-]Failed assertion in `index_pool.c: heap_pop` with interpreters and threads aborts free-threading build[/-][+]Failed assertion in `index_pool.c: heap_pop` with interpreters and threads aborts on free-threading build[/+]on Nov 10, 2024 There are two bugs here. First, the initial check for if the subinterpreter is running in
_interpreters.destroydoesn't account for the fact that another thread could start running it on the free-threaded build. That's a relatively simple fix, though--just set up some synchronization and prevent interpreters from starting oncedestroyhas started.The much more difficult issue is how we capture exceptions. Because we need to send the object to the caller, we have to detach from the subinterpreter, and then access it from the calling interpreter, which isn't thread safe, even on the GIL-ful build. The moment we detach from the thread state it's fair game for another thread to try and attach to it, and overwrite the exception that we want. There are a few possible fixes, and I'm not sure which one I want to do yet:
- Add a big ugly lock around interpreter switching to any interpreter (will contend very badly).
- Add an equally ugly lock to
_interpreters(as in, marking it as needing the GIL). - Add a per-interpreter lock for switching. This is what I'm leaning towards, but it hurts the C API, because exception capturing happens outside of
_PyXI_Enterand_PyXI_Exit, so those APIs will need a lock to call them, foiling my plans to make them public. - Capture exceptions before detaching the thread, serialize them and store them on the crossinterpreter session, and then deserialize once we're back in the calling interpreter (will be a PITA to implement).
- changed the title
[-]Failed assertion in `index_pool.c: heap_pop` with interpreters and threads aborts on free-threading build[/-][+]Interpreter exception capturing is not thread safe[/+]on Nov 10, 2024 - changed the title
[-]Interpreter exception capturing is not thread safe[/-][+]Subinterpreter exception capturing is not thread safe[/+]on Nov 10, 2024 OK, it looks like the exception-capturing problem is less severe than I thought. We only rely on the state of the interpreter for
_PyXI_ERR_ALREADY_RUNNING, so the easy fix is to just manually raise the error there. The much bigger problem is that it seems subinterpreters just isn't meant to run on the free-threaded build. I guess I'll do what I can to help deal with what.- changed the title
[-]Subinterpreter exception capturing is not thread safe[/-][+]`_interpreters` is not thread safe on the free-threaded build[/+]on Nov 11, 2024 Because we need to send the object to the caller, we have to detach from the subinterpreter, and then access it from the calling interpreter, which isn't thread safe, even on the GIL-ful build
Can you expand on this? Accessing Python objects from another interpreter is not safe in the free-threaded build, even with a lock. The GC and some other code paths assume that interpreter's and their objects are isolated.
I was wrong with my analysis. I thought originally that we just moved the exception between the interpreter, but instead it gets stored and serialized. The issue right now is that
_PyXI_ERR_ALREADY_RUNNINGuses_PyInterpreterState_FailIfRunningMain, which isn't right because another thread could start and stop the interpreter in the meantime. I just fixed it on my end by manually raising the interpreter error and not using that function.The much bigger concern I have is that it seems
_PyInterpreterState_SetRunningMainand_PyInterpreterState_SetNotRunningMainaren't thread-safe. On the free-threaded build, I'm assuming there's no waiting for_PyThreadState_Attach, because there's no GIL? If that's the case, then a few of the interpreter state functions need to get some locking. (For example,_PyInterpreterState_SetRunningMainchecks if the main thread state has already been set, and then sets the current thread state, but on the free-threaded build it's possible for it to start running after that check, and then overwrite the old thread state, which causes a fatal error upon finalization.)I'm closing this at "not planned" because it's just collecting dust. I don't think this really qualifies as a tracking issue, so any new issues related to subinterpreters + free-threading should just get their own issue. Thanks @devdanzin for the report, and Sam for teaching me more about thread safety via #126696 :)
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Crash report
What happened?
First off, sorry for not being able to offer code that is more reduced and certain to trigger a repro.
The code below non-deterministically triggers
python: Python/index_pool.c:92: heap_pop: Assertion 'heap->size > 0' failed.in a free-threading build withPYTHON_GIL=0.Backtrace:
This code most usually results in one of the following errors, from most common to rarest:
Given that, running it multiple times seems necessary to trigger the correct abort in
heap_pop. The affected code seems to have been included in #123926, so cc @mpage.Found using fusil by @vstinner.
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Output from running 'python -VV' on the command line:
Python 3.14.0a1+ experimental free-threading build (heads/main-dirty:54c63a32d0, Nov 8 2024, 20:16:36) [GCC 11.4.0]
Linked PRs
_interpreters#126696