john/McRogueFace
1
0
Fork 0
Code Issues 57 Pull requests Projects Releases Wiki Activity

[Testing] mcrfpy.lock() / FrameLock has near-zero coverage — stress, fuzz, and soak the #219 barrier #395

New issue
Open
opened 2026-07-26 20:33:09 +00:00 by john · 0 comments
john commented 2026-07-26 20:33:09 +00:00
Owner
Copy link

Problem

The #219 thread-synchronization path is well-designed but effectively untested. grep -rl "mcrfpy.lock" tests/ returns exactly one file — tests/fuzz/fuzz_property_types.py — and that reference is incidental, not a test of the barrier itself.

The implementation under test:

  • src/PyLock.cpp — the mcrfpy._LockContext context manager; __enter__ no-ops on the main thread via isMainThread() (PyLock.cpp:72-76), otherwise blocks in FrameLock::acquire()
  • src/GameEngine.cpp:28-45 — FrameLock::acquire() / release(), waiting on a condvar with the GIL released
  • src/GameEngine.cpp:508-516 — the safe window: after display(), before the next frame's processing, openWindow() then a GIL-released closeWindow() that blocks the main thread until every lock holder completes

That last property is the one that needs evidence. The render thread's frame time is a direct function of how long background handlers hold the lock, and nothing currently measures or bounds that.

Why now

This stops being theoretical the moment a background thread services a real event stream — an in-process NATS/asyncio client mutating entities under mcrfpy.lock() while the grid renders live (#55, #154, #156). That is a far heavier use than anything the suite exercises: a handler on every inbound event, sustained for hours, against a render loop that must stay smooth. If the barrier has a rare deadlock, a starvation case, or an unbounded-hold failure mode, it will surface there as "the engine is slow" rather than as a test failure.

What to cover

Correctness

  • Main-thread __enter__ is a genuine no-op — same code path works from a timer callback, script init, and a background thread
  • Exception raised inside the with block releases the lock and propagates (__exit__ returns False, PyLock.cpp:87-96)
  • The dealloc-while-holding safety net actually fires (PyLock.cpp:56-62) — construct a holder and drop it without __exit__
  • Nested / re-entrant acquisition from one thread: define the intended semantics, then test them. Today active is a plain counter and re-entry is not obviously handled
  • Mutations made under the lock are visible to the next rendered frame, and never tear a collection mid-iteration
  • Interaction with in-flight animations and a scene transition — both mutate the same structures from the main thread

Stress / soak

  • N threads (N ≥ 4) hammering the lock continuously for a sustained run with no deadlock, no lost wakeup, no starved thread
  • Long-hold handler: assert the frame-time relationship is what the design implies (a 50 ms hold costs the frame ~50 ms), so the cost is documented and bounded rather than discovered in production
  • Thread churn: threads created and destroyed while others hold the lock
  • Shutdown with holders active / waiting — engine exit must not hang or crash

Fuzz

  • Extend the existing fuzz targets to mutate entities and UI collections from a background thread under the lock, with ASan/UBSan on. The #312 crash-corpus workflow is the right vehicle; the entity collection and PythonObjectCache identity-pin paths (#266/#373) are the interesting targets, since they were designed single-threaded

Documentation deliverable

The two usage rules should end up written down somewhere durable, because both fail silently rather than loudly:

  1. Inside the lock, mutate only. No I/O, no await, no network calls — closeWindow() blocks the render thread until all holders finish, so a slow handler is indistinguishable from engine stutter.
  2. Whatever the nesting semantics turn out to be, state them.

Notes

Free-threading (#336) is explicitly not in scope here — this is the GIL-holding model, which is the correct one for an I/O-bound background thread. #220 (subinterpreters) is likewise separate.

Blocks

Prerequisite for using McRogueFace as a live simulation environment driven by an external event bus (#55, #154). Along with #394, should land before that work starts.

## Problem The `#219` thread-synchronization path is well-designed but effectively untested. `grep -rl "mcrfpy.lock" tests/` returns exactly one file — `tests/fuzz/fuzz_property_types.py` — and that reference is incidental, not a test of the barrier itself. The implementation under test: - `src/PyLock.cpp` — the `mcrfpy._LockContext` context manager; `__enter__` no-ops on the main thread via `isMainThread()` (`PyLock.cpp:72-76`), otherwise blocks in `FrameLock::acquire()` - `src/GameEngine.cpp:28-45` — `FrameLock::acquire()` / `release()`, waiting on a condvar with the GIL released - `src/GameEngine.cpp:508-516` — the safe window: after `display()`, before the next frame's processing, `openWindow()` then a GIL-released `closeWindow()` that **blocks the main thread until every lock holder completes** That last property is the one that needs evidence. The render thread's frame time is a direct function of how long background handlers hold the lock, and nothing currently measures or bounds that. ## Why now This stops being theoretical the moment a background thread services a real event stream — an in-process NATS/asyncio client mutating entities under `mcrfpy.lock()` while the grid renders live (#55, #154, #156). That is a far heavier use than anything the suite exercises: a handler on every inbound event, sustained for hours, against a render loop that must stay smooth. If the barrier has a rare deadlock, a starvation case, or an unbounded-hold failure mode, it will surface there as "the engine is slow" rather than as a test failure. ## What to cover **Correctness** - [ ] Main-thread `__enter__` is a genuine no-op — same code path works from a timer callback, script init, and a background thread - [ ] Exception raised inside the `with` block releases the lock and propagates (`__exit__` returns `False`, `PyLock.cpp:87-96`) - [ ] The dealloc-while-holding safety net actually fires (`PyLock.cpp:56-62`) — construct a holder and drop it without `__exit__` - [ ] Nested / re-entrant acquisition from one thread: define the intended semantics, then test them. Today `active` is a plain counter and re-entry is not obviously handled - [ ] Mutations made under the lock are visible to the next rendered frame, and never tear a collection mid-iteration - [ ] Interaction with in-flight animations and a scene transition — both mutate the same structures from the main thread **Stress / soak** - [ ] N threads (N ≥ 4) hammering the lock continuously for a sustained run with no deadlock, no lost wakeup, no starved thread - [ ] Long-hold handler: assert the frame-time relationship is what the design implies (a 50 ms hold costs the frame ~50 ms), so the cost is *documented and bounded* rather than discovered in production - [ ] Thread churn: threads created and destroyed while others hold the lock - [ ] Shutdown with holders active / waiting — engine exit must not hang or crash **Fuzz** - [ ] Extend the existing fuzz targets to mutate entities and UI collections from a background thread under the lock, with ASan/UBSan on. The `#312` crash-corpus workflow is the right vehicle; the entity collection and `PythonObjectCache` identity-pin paths (`#266`/`#373`) are the interesting targets, since they were designed single-threaded ## Documentation deliverable The two usage rules should end up written down somewhere durable, because both fail *silently* rather than loudly: 1. **Inside the lock, mutate only.** No I/O, no `await`, no network calls — `closeWindow()` blocks the render thread until all holders finish, so a slow handler is indistinguishable from engine stutter. 2. Whatever the nesting semantics turn out to be, state them. ## Notes Free-threading (`#336`) is explicitly *not* in scope here — this is the GIL-holding model, which is the correct one for an I/O-bound background thread. `#220` (subinterpreters) is likewise separate. ## Blocks Prerequisite for using McRogueFace as a live simulation environment driven by an external event bus (#55, #154). Along with #394, should land before that work starts.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
feat/381-snippet-shots
feature/issues-317-319-verify-bugs
feature/issue-316-sparse-perspective
feature/issue-314-doc-followthrough
feature/api-freeze-313-314
threedee
emscripten-mcrogueface
rogueliketutorial25
alpha_presentable
alpha_streamline_2
alpha_streamline_1
interpreter_mode
grid_entity_integration
reprs_and_member_names
break_up_ui_h
standardize_font_handling
standardize_vector_handling
standardize_color_handling
standardize_texture_handling
raii_pyobjects
rebase-for-7DRL_Feb-2024
engjam_uicollection
engjam_ui_overhaul
0.2.3-prerelease-7drl2026-emscripten
Labels
Clear labels   
Alpha Release Requirement

This issue is a blocker to 0.1 Alpha release of McRogueFace.

  
Bugfix

   
Demo Target

Functionality to demonstrate; Depends on new features, but this issue isn't for writing them.

  
Documentation

   
Major Feature

Significant time and effort required.

  
Minor Feature

Some effort required to create or overhaul functionality.

  
priority:tier1-active

Current development focus - critical path to v1.0

  
priority:tier2-foundation

Important foundation work - not blocking v1.0

  
priority:tier3-future

Future features - deferred until after v1.0

  
priority:tier4-deferred

   
Refactoring & Cleanup

no new functionality, just improving the codebase.

  
system:animation

Animation and property interpolation

  
system:documentation

Documentation infrastructure

  
system:grid

Grid system and spatial containers

  
system:input

Input handling and events

  
system:performance

Performance optimization and profiling

  
system:procgen

Procedural Generation (Noise, BSP, Heightmap)

  
system:python-binding

Python/C++ binding layer

  
system:rendering

Rendering pipeline and visuals

  
system:ui-hierarchy

UI component hierarchy and composition

  
Tiny Feature

quick and easy - a few lines or with little interconnection around the codebase

  
workflow:blocked

Blocked by other work - waiting on dependencies

  
workflow:needs-benchmark

Needs performance testing and benchmarks

  
workflow:needs-documentation

Needs documentation before or after implementation

No labels
Alpha Release Requirement
Bugfix
Demo Target
Documentation
Major Feature
Minor Feature
priority:tier1-active
priority:tier2-foundation
priority:tier3-future
priority:tier4-deferred
Refactoring & Cleanup
system:animation
system:documentation
system:grid
system:input
system:performance
system:procgen
system:python-binding
system:rendering
system:ui-hierarchy
Tiny Feature
workflow:blocked
workflow:needs-benchmark
workflow:needs-documentation
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
john/McRogueFace#395
No description provided.