[Bugfix] FrameLock::acquire() deadlocks the whole interpreter with two background threads (GIL/mutex lock-order inversion) #412

New issue

Open

opened 2026-07-27 18:09:50 +00:00 by john · 0 comments

john commented

2026-07-27 18:09:50 +00:00

Owner

Two background threads using mcrfpy.lock() concurrently hang the entire process. One thread is fine. Two is a hard deadlock that takes the GIL with it, so the main thread, every timer and every other thread stop too. No error, no log line, no timeout — FrameLock's condition variable has no deadline (#398).

Found by the contention coverage added for #395. Fixed in the same commit.

Reproduction

import mcrfpy, threading, time
s = mcrfpy.Scene("abba"); mcrfpy.current_scene = s
N = 2                       # 1 -> fine.  2 -> hang.
def w(i):
    for _ in range(300):
        with mcrfpy.lock():
            pass
ts = [threading.Thread(target=w, args=(i,), daemon=True) for i in range(N)]
for t in ts: t.start()
end = time.monotonic() + 10.0
while any(t.is_alive() for t in ts) and time.monotonic() < end:
    mcrfpy.step(0.016)

N=1  ->  frames=70  counts=[300]              completed
N=2  ->  timeout, process wedged

The defect

void FrameLock::acquire() {
    waiting++;
    std::unique_lock<std::mutex> lock(mtx);        // takes mtx WHILE HOLDING THE GIL

    Py_BEGIN_ALLOW_THREADS                          // releases the GIL
    cv.wait(lock, [this]{ return safe_window; });   // returns with mtx RE-LOCKED
    Py_END_ALLOW_THREADS                            // re-acquires the GIL, STILL HOLDING mtx

    waiting--;
    active++;
}

Textbook ABBA:

thread A is woken from cv.wait, so it holds mtx, and then blocks in Py_END_ALLOW_THREADS acquiring the GIL;
thread B entered acquire() holding the GIL and blocks in std::unique_lock on mtx.

Neither can proceed, and because B holds the GIL, the interpreter stops with them.

Confirmed by backtrace on the wedged process rather than by inspection:

Thread-2:  #7  FrameLock::acquire()
           #5  ...libpython... take_gil       <- HOLDS mtx, waiting for the GIL
Thread-1:  #4  FrameLock::acquire()
           #3  ___pthread_mutex_lock          <- HOLDS the GIL, waiting for mtx

It needs exactly two threads and entirely ordinary usage. It survived because nothing in the suite had ever put two threads on this barrier at once — which is the whole of #395's point.

The fix

Release the GIL before taking mtx and re-acquire it only after mtx is dropped:

Py_BEGIN_ALLOW_THREADS
{
    std::unique_lock<std::mutex> lock(mtx);
    cv.wait(lock, [this]{ return safe_window; });
    waiting--;                 // under mtx, so closeWindow()'s two predicates see the
    active++;                  // waiting->active handoff as one step
}
Py_END_ALLOW_THREADS

No thread now holds mtx while blocking on the GIL, so the cycle cannot form. release() and openWindow() still take mtx while holding the GIL, but they never wait for the GIL, so they are only ever the second edge; the two GIL-free holders (acquire() and closeWindow()) both drop mtx inside cv.wait(), so neither holds it for long.

Verified after the fix at N = 1, 2, 4 and 8 threads: every worker completes.

Coverage added with the fix

tests/integration/frame_lock_stress_test.py — 8 workers x 40 iterations, mutual exclusion, fairness, per-acquire wait distribution, nesting.
tests/integration/frame_lock_soak_test.py — 3 workers under sustained load, drift in wait time and frame time, allocator-block growth.

Both run their load in a subprocess child with a heartbeat watchdog, deliberately: a wedged GIL cannot be detected from inside the process it has wedged, so an in-process deadline would simply never run. That is how this defect was caught at all, and it is why the machinery stays.

Related: #395, #398, #410.

**Two background threads using `mcrfpy.lock()` concurrently hang the entire process.** One thread is fine. Two is a hard deadlock that takes the GIL with it, so the main thread, every timer and every other thread stop too. No error, no log line, no timeout — `FrameLock`'s condition variable has no deadline (#398). Found by the contention coverage added for #395. Fixed in the same commit. ## Reproduction ```python import mcrfpy, threading, time s = mcrfpy.Scene("abba"); mcrfpy.current_scene = s N = 2 # 1 -> fine. 2 -> hang. def w(i): for _ in range(300): with mcrfpy.lock(): pass ts = [threading.Thread(target=w, args=(i,), daemon=True) for i in range(N)] for t in ts: t.start() end = time.monotonic() + 10.0 while any(t.is_alive() for t in ts) and time.monotonic() < end: mcrfpy.step(0.016) ``` ``` N=1 -> frames=70 counts=[300] completed N=2 -> timeout, process wedged ``` ## The defect ```cpp void FrameLock::acquire() { waiting++; std::unique_lock<std::mutex> lock(mtx); // takes mtx WHILE HOLDING THE GIL Py_BEGIN_ALLOW_THREADS // releases the GIL cv.wait(lock, [this]{ return safe_window; }); // returns with mtx RE-LOCKED Py_END_ALLOW_THREADS // re-acquires the GIL, STILL HOLDING mtx waiting--; active++; } ``` Textbook ABBA: - **thread A** is woken from `cv.wait`, so it **holds `mtx`**, and then blocks in `Py_END_ALLOW_THREADS` acquiring **the GIL**; - **thread B** entered `acquire()` **holding the GIL** and blocks in `std::unique_lock` on **`mtx`**. Neither can proceed, and because B holds the GIL, the interpreter stops with them. Confirmed by backtrace on the wedged process rather than by inspection: ``` Thread-2: #7 FrameLock::acquire() #5 ...libpython... take_gil <- HOLDS mtx, waiting for the GIL Thread-1: #4 FrameLock::acquire() #3 ___pthread_mutex_lock <- HOLDS the GIL, waiting for mtx ``` It needs exactly two threads and entirely ordinary usage. It survived because nothing in the suite had ever put two threads on this barrier at once — which is the whole of #395's point. ## The fix Release the GIL *before* taking `mtx` and re-acquire it only after `mtx` is dropped: ```cpp Py_BEGIN_ALLOW_THREADS { std::unique_lock<std::mutex> lock(mtx); cv.wait(lock, [this]{ return safe_window; }); waiting--; // under mtx, so closeWindow()'s two predicates see the active++; // waiting->active handoff as one step } Py_END_ALLOW_THREADS ``` No thread now holds `mtx` while blocking on the GIL, so the cycle cannot form. `release()` and `openWindow()` still take `mtx` while holding the GIL, but they never *wait* for the GIL, so they are only ever the second edge; the two GIL-free holders (`acquire()` and `closeWindow()`) both drop `mtx` inside `cv.wait()`, so neither holds it for long. Verified after the fix at N = 1, 2, 4 and 8 threads: every worker completes. ## Coverage added with the fix - `tests/integration/frame_lock_stress_test.py` — 8 workers x 40 iterations, mutual exclusion, fairness, per-acquire wait distribution, nesting. - `tests/integration/frame_lock_soak_test.py` — 3 workers under sustained load, drift in wait time and frame time, allocator-block growth. Both run their load in a **subprocess child with a heartbeat watchdog**, deliberately: a wedged GIL cannot be detected from inside the process it has wedged, so an in-process deadline would simply never run. That is how this defect was caught at all, and it is why the machinery stays. Related: #395, #398, #410.