Repository navigation
Failed assertion in Python/legacy_tracing.c:431 on a free-threading build #127235
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 24, 2024 Does this occur without use of the private
_start_joinable_thread?I'm pretty sure that this is not limited to free threading. It's not safe to access other
PyThreadStates without holding the runtime lock because those threads may exit. I think that we release the lock here because of reentrancy/deadlock concerns, but it's still not safe.The reproducers involving the GIL are slightly more complicated because
PyThreadState_DeleteCurrentis called with the GIL, which prevents most of these crashes. However,PyThreadState_Deleteis called without the GIL and can lead to the crash.Lines 2402 to 2421 in 3e7ce6e
void PyEval_SetProfileAllThreads(Py_tracefunc func, PyObject *arg) { PyThreadState *this_tstate = _PyThreadState_GET(); PyInterpreterState* interp = this_tstate->interp; _PyRuntimeState *runtime = &_PyRuntime; HEAD_LOCK(runtime); PyThreadState* ts = PyInterpreterState_ThreadHead(interp); HEAD_UNLOCK(runtime); while (ts) { if (_PyEval_SetProfile(ts, func, arg) < 0) { PyErr_FormatUnraisable("Exception ignored in PyEval_SetProfileAllThreads"); } HEAD_LOCK(runtime); ts = PyThreadState_Next(ts); HEAD_UNLOCK(runtime); } } I think we have various reports of the same underlying issue. This issue is a nice reproducer for free threading, though.
For example:
Reacted by Peter BiermaOk, I've pondered this issue for a day or so and I think that the simplest solution for this specific case is to just wait for the per-interpreter thread state lock (GH-127115) to get merged.
HEAD_LOCKtends to be annoyingly reentrant, so by just switching to that, we should be able to fix any unsafe usage because it shouldn't be nearly as reentrancy prone. (FWIW, I'm somewhat in favor of making the runtime lock recursive with how much of a PITA it is to use, but whatever.)The much bigger issue at hand here is that
PyThreadState_Nextand friends are public APIs, so they aren't able toHEAD_LOCK. Somehow, we need to lock inside of those functions rather than relying on the caller to lock the runtime. I don't think there's anything we can do aboutPyThreadState_Next, because even if we somehow got thread states in a thread-safe way internally, and then magically made it safe to use the thread state without worrying about concurrent modifications (which would be impossible, unless we did something silly like copying the thread state and returning that), there's nothing stopping the list of thread states from being changed in between calls, so it could possibly skip over entries or return duplicates. Our best shot is probably to just deprecatePyThreadState_Nextand invent something new and safer for users.Some other functions we need to worry about in terms of thread safety:
PyInterpreterState_ThreadHead: Same issue asPyThreadState_Next, pretty much.PyGILState_GetThisThreadState: I'm not sure how this will work with subinterpreters, because apparentlyPyGILStatedoesn't play nicely with them. I could imagine some interesting edge cases coming up related to locking there because apparently GILState checks are disabled if subinterpreters are active.PyThreadState_GetDict: Returns an evil borrowed reference. If the thread state gets switched out, the user can still hold a reference to that dictionary. Even worse, on a GILful build, trying to use that dictionary without having the thread state attached is just not thread safe at all if you try to access it from a different GIL.PyThreadState_GetFrame: Not borrowed, but still has the object thread safety issue with the cross-GIL that I mentioned above. I think for both of these functions it's probably fine to just mention in the docs that you can only use the references while the thread state is attached.PyThreadState_Swap: Not inherently dangerous, but could lead to misue that we should document. This returns a not-attached thread state, which is ripe for abuse by any of the otherPyThreadStateAPIs. (Also, what happens if e.g. the interpreter starts to exit while its detached?)
For the future, I think we should probably steer clear of any support for using a detached thread state. Perhaps we could invent some new API (
PyThreadStateView?) that has more clear thread safety requirements and push that for users.TL;DR we probably need to document and deprecate some non-thread-safe thread state APIs.
I can't reproduce this on current main, @devdanzin are you able to reproduce?
Can't test it right now, but will try later. But on a quick search, maybe it was fixed by #137518?
The reproducer fails with this error on the main branch:
Exception ignored in thread started by <function errback at 0x7f130084abd0>: (...) Exception ignored in thread started by <function errback at 0x7f130084abd0>: Exception ignored in thread started by <function errback at 0x7f130084abd0>: Exception ignored in thread started by <function errback at 0x7f130084abd0>: Fatal Python error: _enter_buffered_busy: could not acquire lock for <_io.BufferedWriter name='<stderr>'> at interpreter shutdown, possibly due to daemon threads Python runtime state: finalizing (tstate=0x0000000000a0c640) Current thread 0x00007f130e8cc780 [python] (most recent call first): <no Python frame> Abandon (core dumped)./python x.pyOh sorry, ignore my previous comment. I missed the fact that the issue is specific to Free Threading.
Can't test it right now, but will try later. But on a quick search, maybe it was fixed by #137518?
Before this change, I randomly got crashes like:
Thread 1 "python" received signal SIGSEGV, Segmentation fault. 0x0000000000649df8 in setup_profile (tstate=tstate@entry=0xac5db0, func=func@entry=0x6754b0 <profile_trampoline>, arg=arg@entry='', old_profileobj=old_profileobj@entry=0x7fffffffd288) at Python/legacy_tracing.c:485 485 tstate->interp->sys_profiling_threads += delta; Missing rpms, try: dnf --enablerepo='*debug*' install glibc-debuginfo-2.42-4.fc43.x86_64 (gdb) p tstate->interp $1 = (PyInterpreterState *) 0xddddddddddddddddOn the main branch, I can no longer reproduce the
setup_profile()bug, but I get randomly the following error:Fatal Python error: _enter_buffered_busy: could not acquire lock for <_io.BufferedWriter name='<stderr>'> at interpreter shutdown, possibly due to daemon threads Python runtime state: finalizing (tstate=0x0000000000a44a90) Stack (most recent call first): <no Python frame> Thread 1 "python" received signal SIGABRT, Aborted.Moreover, sometimes the program hangs, likely a deadlock related to
stderr.I can't reproduce this on current main, @devdanzin are you able to reproduce?
Me neither. I suggest closing this issue as FIXED.
Reacted by devdanzinCannot reproduce after extensive testing with tweaked test cases.
Reacted by Victor StinnerCannot reproduce after extensive testing with tweaked test cases.
Thanks for double checking!
Crash report
What happened?
It's possible to cause an abort on
!_PyMem_IsPtrFreed(tstate)while running withPYTHON_GIL=0by calling the following code:Abort message:
Not sure this isn't an expected failure mode.
Found using fusil by @vstinner.
CPython versions tested on:
3.13, 3.14, CPython main branch
Operating systems tested on:
Linux
Output from running 'python -VV' on the command line:
Python 3.14.0a2+ experimental free-threading build (heads/main-dirty:a13e94d84bf, Nov 23 2024, 07:16:19) [GCC 11.4.0]