gh-144763: Don't detach the GIL in tracemalloc#144779
gh-144763: Don't detach the GIL in tracemalloc#144779vstinner wants to merge 2 commits intopython:mainfrom
Conversation
tracemalloc no longer detachs the GIL to acquire its internal lock.
Don't call PyRefTracer_SetTracer() while holding TABLES_LOCK().
| int lineno = -1; | ||
| PyCodeObject *code = _PyFrame_GetCode(pyframe); | ||
| // PyUnstable_InterpreterFrame_GetLine() cannot but used, since it uses | ||
| // a critical section which can trigger a deadlock. |
There was a problem hiding this comment.
I think the problem is that critical sections requires an active thread state and this code can be called with detached thread state iirc.
There was a problem hiding this comment.
tracemalloc_get_frame() is called with an attached thread state, see the caller traceback_get_frames() which has the code:
PyThreadState *tstate = _PyThreadState_GET();
assert(tstate != NULL);Example of deadlock when running #144763 (comment) reproducer on a free-threaded build:
- Thread A:
_PyCode_GetTLBC()=> ... =>tracemalloc_alloc():TABLES_LOCK() - Thread B:
_PyTraceMalloc_TraceRef()=> ... =>tracemalloc_get_frame()=>PyUnstable_InterpreterFrame_GetLine()=>PyCode_Addr2Line():Py_BEGIN_CRITICAL_SECTION(co)
Locks:
- Thread A is in a
Py_BEGIN_CRITICAL_SECTION(co)critical section (_PyCode_GetTLBC()) and waits forTABLES_LOCK(). - Thread B has
TABLES_LOCK()lock and waits forPy_BEGIN_CRITICAL_SECTION(co)critical section.
Thread A and thread B want to use Py_BEGIN_CRITICAL_SECTION(co) on the same code object (0x20000844810).
=> deadlock :-(
ZeroIntensity
left a comment
There was a problem hiding this comment.
Is it necessary to keep the attached thread state in TABLES_LOCK? It seems the bulk of the fix is just calling PyRefTracer_SetTracer without holding the lock.
Alternatively, we could just use Py_BEGIN_CRITICAL_SECTION_MUTEX instead of PyMutex_Lock.
| Fix a race condition in :mod:`tracemalloc`: it no longer detachs the GIL to | ||
| acquire its internal lock. Patch by Victor Stinner. |
There was a problem hiding this comment.
We should probably use "attached thread state" terminology here, since this only affects free-threaded builds.
tracemalloc no longer detachs the GIL to acquire its internal lock.
_io_TextIOWrapper_write_implwhen runningtracemalloc.clear_tracesin threads #144763