[Bugfix] Headless step()/advance() never opens the #219 frame-lock window — mcrfpy.lock() from a background thread blocks forever #398

New issue

Open

opened 2026-07-27 03:38:37 +00:00 by john · 1 comment

john commented

2026-07-27 03:38:37 +00:00

Owner

Summary

mcrfpy.lock() (#219) is documented as the way a background Python thread hands mutations back to the engine safely. It works in windowed mode and deadlocks in headless mode.

Cause

FrameLock::acquire() waits on a condition variable with no timeout. The only thing that signals it is openWindow(), and that is called from exactly one place:

src/GameEngine.cpp:510 — inside doFrame(), after display().

The headless clock does not go through that path. mcrfpy.step() / mcrfpy.advance() both funnel into GameEngine::runSimFrame() (src/GameEngine.cpp:954), which deliberately skips render and input — and therefore also skipped the safe window. A background thread that enters with mcrfpy.lock(): under a step()-driven main loop parks in acquire() and never wakes.

Reproduction (pre-fix)

import mcrfpy, sys, threading, time
entered, acquired = threading.Event(), threading.Event()
def worker():
    entered.set()
    with mcrfpy.lock():
        acquired.set()
threading.Thread(target=worker, daemon=True).start()
entered.wait(2.0)
t0 = time.time()
n = 0
while time.time() - t0 < 2.0 and not acquired.is_set():
    mcrfpy.step(0.016); n += 1
print("steps:", n, "acquired:", acquired.is_set())
sys.exit(0 if acquired.is_set() else 1)

Pre-fix output: steps: 4951148 acquired: False — 4.95 million frames, never acquired.
Post-fix output: steps: 1 acquired: True.

Why it matters

Headless is the mode embedders drive a world from (simulators, agent environments, CI). It is precisely the mode where a bus/worker thread needs to hand mutations back, and it was the one mode where the documented primitive could not work. Nothing caught it because mcrfpy.lock() had essentially no coverage — see #395; the single existing use is a with mcrfpy.lock(): pass smoke poke in tests/fuzz/, which no-ops on the main thread and so never exercised acquire() at all.

Fix

Open the safe window once per simulation frame in runSimFrame(), mirroring doFrame(): after the frame's work, before currentFrame++, guarded by hasWaiting() (one relaxed atomic load per frame when no threads are waiting), with the GIL released across closeWindow().

Gated by tests/integration/background_thread_network_test.py, which asserts a background thread's locked mutation actually lands while the main thread steps. That test fails on the pre-fix binary.

#219 — original frame-lock feature
#395 — mcrfpy.lock() / FrameLock coverage (this is an instance of the gap that issue describes)
#396 — the step()/advance() simulation clock this interacts with

## Summary `mcrfpy.lock()` (#219) is documented as the way a background Python thread hands mutations back to the engine safely. It works in windowed mode and **deadlocks in headless mode**. ## Cause `FrameLock::acquire()` waits on a condition variable with **no timeout**. The only thing that signals it is `openWindow()`, and that is called from exactly one place: - `src/GameEngine.cpp:510` — inside `doFrame()`, after `display()`. The headless clock does not go through that path. `mcrfpy.step()` / `mcrfpy.advance()` both funnel into `GameEngine::runSimFrame()` (`src/GameEngine.cpp:954`), which deliberately skips render and input — and therefore also skipped the safe window. A background thread that enters `with mcrfpy.lock():` under a `step()`-driven main loop parks in `acquire()` and never wakes. ## Reproduction (pre-fix) ```python import mcrfpy, sys, threading, time entered, acquired = threading.Event(), threading.Event() def worker(): entered.set() with mcrfpy.lock(): acquired.set() threading.Thread(target=worker, daemon=True).start() entered.wait(2.0) t0 = time.time() n = 0 while time.time() - t0 < 2.0 and not acquired.is_set(): mcrfpy.step(0.016); n += 1 print("steps:", n, "acquired:", acquired.is_set()) sys.exit(0 if acquired.is_set() else 1) ``` Pre-fix output: `steps: 4951148 acquired: False` — 4.95 million frames, never acquired. Post-fix output: `steps: 1 acquired: True`. ## Why it matters Headless is the mode embedders drive a world from (simulators, agent environments, CI). It is precisely the mode where a bus/worker thread needs to hand mutations back, and it was the one mode where the documented primitive could not work. Nothing caught it because `mcrfpy.lock()` had essentially no coverage — see #395; the single existing use is a `with mcrfpy.lock(): pass` smoke poke in `tests/fuzz/`, which no-ops on the main thread and so never exercised `acquire()` at all. ## Fix Open the safe window once per simulation frame in `runSimFrame()`, mirroring `doFrame()`: after the frame's work, before `currentFrame++`, guarded by `hasWaiting()` (one relaxed atomic load per frame when no threads are waiting), with the GIL released across `closeWindow()`. Gated by `tests/integration/background_thread_network_test.py`, which asserts a background thread's locked mutation actually lands while the main thread steps. That test fails on the pre-fix binary. ## Related - #219 — original frame-lock feature - #395 — `mcrfpy.lock()` / FrameLock coverage (this is an instance of the gap that issue describes) - #396 — the `step()`/`advance()` simulation clock this interacts with

john added the

Bugfix

system:python-binding

priority:tier1-active

labels

2026-07-27 03:38:37 +00:00

john commented

2026-07-27 07:46:39 +00:00

Author

Owner

Fixed on branch sim/integration, commit 1523533 (not yet on master).

runSimFrame() now opens the safe window once per simulation frame — after the frame's work, before currentFrame++, mirroring doFrame() — guarded by hasWaiting(), with the GIL released across closeWindow().

A dedicated regression test was added alongside the integration test named in the issue body: tests/regression/issue_398_headless_frame_lock_test.py. It is the minimal statement of the bug (one background thread, one mcrfpy.lock(), one assignment, a step()-driven main loop — no sockets, no asyncio), so it stays a lock regression rather than becoming a second copy of tests/integration/background_thread_network_test.py.

A/B verified by rebuilding the pre-fix GameEngine.cpp and re-running both tests:

binary	`issue_398_headless_frame_lock_test.py`	`background_thread_network_test.py`
pre-fix (`b77aeec`)	exit 1 — 12,191,764 `step()` calls, lock never opened, thread still parked in `acquire()`	exit 1 — worker never finished, `target.x == 0.0`, thread never exited
post-fix	exit 0 — acquired after 1 step	exit 0 — 597,339 frames completed while the worker was parked in `selectors.select()`

Full suite 625/625.

Fixed on branch `sim/integration`, commit `1523533` (not yet on master). `runSimFrame()` now opens the safe window once per simulation frame — after the frame's work, before `currentFrame++`, mirroring `doFrame()` — guarded by `hasWaiting()`, with the GIL released across `closeWindow()`. A dedicated regression test was added alongside the integration test named in the issue body: **`tests/regression/issue_398_headless_frame_lock_test.py`**. It is the minimal statement of the bug (one background thread, one `mcrfpy.lock()`, one assignment, a `step()`-driven main loop — no sockets, no asyncio), so it stays a lock regression rather than becoming a second copy of `tests/integration/background_thread_network_test.py`. A/B verified by rebuilding the pre-fix `GameEngine.cpp` and re-running both tests: | binary | `issue_398_headless_frame_lock_test.py` | `background_thread_network_test.py` | |---|---|---| | pre-fix (`b77aeec`) | exit 1 — 12,191,764 `step()` calls, lock never opened, thread still parked in `acquire()` | exit 1 — worker never finished, `target.x == 0.0`, thread never exited | | post-fix | exit 0 — acquired after **1** step | exit 0 — 597,339 frames completed while the worker was parked in `selectors.select()` | Full suite 625/625.

john referenced this issue

2026-07-27 07:46:50 +00:00

[Bugfix] Flaky suite failure: OpenAL-soft asserts during device teardown (snippets/253_Music.py) #400

john referenced this issue

2026-07-27 17:51:26 +00:00

[Bugfix] FrameLock: a background thread that re-acquires without yielding starves the main loop (31 frames in 2s) #410

john referenced this issue

2026-07-27 17:55:30 +00:00

[Bugfix] mcrfpy.step()/advance() from a background thread (without the lock) silently runs a frame off the main thread #411

john referenced this issue

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

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