Fix #219: Add threading support with mcrfpy.lock() context manager
Enables background Python threads to safely modify UI objects by
synchronizing with the render loop at frame boundaries.
Implementation:
- FrameLock class provides mutex/condvar synchronization
- GIL released during window.display() allowing background threads to run
- Safe window opens between frames for synchronized UI updates
- mcrfpy.lock() context manager blocks until safe window, then executes
- Main thread detection: lock() is a no-op when called from callbacks
or script initialization (already synchronized)
Usage:
import threading
import mcrfpy
def background_worker():
with mcrfpy.lock(): # Blocks until safe
player.x = new_x # Safe to modify UI
threading.Thread(target=background_worker).start()
The lock works transparently from any context - background threads get
actual synchronization, main thread calls (callbacks, init) get no-op.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
14a6520593
commit
257e52327b
5 changed files with 269 additions and 1 deletions
|
|
@ -10,6 +10,48 @@
|
|||
#include "imgui.h"
|
||||
#include "imgui-SFML.h"
|
||||
#include <cmath>
|
||||
#include <Python.h>
|
||||
|
||||
// #219 - FrameLock implementation for thread-safe UI updates
|
||||
|
||||
void FrameLock::acquire() {
|
||||
waiting++;
|
||||
std::unique_lock<std::mutex> lock(mtx);
|
||||
|
||||
// Release GIL while waiting for safe window
|
||||
Py_BEGIN_ALLOW_THREADS
|
||||
cv.wait(lock, [this]{ return safe_window; });
|
||||
Py_END_ALLOW_THREADS
|
||||
|
||||
waiting--;
|
||||
active++;
|
||||
}
|
||||
|
||||
void FrameLock::release() {
|
||||
std::unique_lock<std::mutex> lock(mtx);
|
||||
active--;
|
||||
if (active == 0) {
|
||||
cv.notify_all(); // Wake up closeWindow() if it's waiting
|
||||
}
|
||||
}
|
||||
|
||||
void FrameLock::openWindow() {
|
||||
std::lock_guard<std::mutex> lock(mtx);
|
||||
safe_window = true;
|
||||
cv.notify_all(); // Wake up all waiting threads
|
||||
}
|
||||
|
||||
void FrameLock::closeWindow() {
|
||||
std::unique_lock<std::mutex> lock(mtx);
|
||||
// First wait for all waiting threads to have entered the critical section
|
||||
// (or confirm none were waiting). This prevents the race where we set
|
||||
// safe_window=false before a waiting thread can check the condition.
|
||||
cv.wait(lock, [this]{ return waiting == 0; });
|
||||
// Then wait for all active threads to finish their critical sections
|
||||
cv.wait(lock, [this]{ return active == 0; });
|
||||
// Now safe to close the window
|
||||
safe_window = false;
|
||||
}
|
||||
|
||||
GameEngine::GameEngine() : GameEngine(McRogueFaceConfig{})
|
||||
{
|
||||
|
|
@ -18,6 +60,9 @@ GameEngine::GameEngine() : GameEngine(McRogueFaceConfig{})
|
|||
GameEngine::GameEngine(const McRogueFaceConfig& cfg)
|
||||
: config(cfg), headless(cfg.headless)
|
||||
{
|
||||
// #219 - Store main thread ID for lock() thread detection
|
||||
main_thread_id = std::this_thread::get_id();
|
||||
|
||||
Resources::font.loadFromFile("./assets/JetbrainsMono.ttf");
|
||||
Resources::game = this;
|
||||
window_title = "McRogueFace Engine";
|
||||
|
|
@ -307,15 +352,30 @@ void GameEngine::run()
|
|||
metrics.workTime = clock.getElapsedTime().asSeconds() * 1000.0f;
|
||||
|
||||
// Display the frame
|
||||
// #219 - Release GIL during display() to allow background threads to run
|
||||
if (headless) {
|
||||
Py_BEGIN_ALLOW_THREADS
|
||||
headless_renderer->display();
|
||||
Py_END_ALLOW_THREADS
|
||||
// Take screenshot if requested
|
||||
if (config.take_screenshot) {
|
||||
headless_renderer->saveScreenshot(config.screenshot_path.empty() ? "screenshot.png" : config.screenshot_path);
|
||||
config.take_screenshot = false; // Only take one screenshot
|
||||
}
|
||||
} else {
|
||||
Py_BEGIN_ALLOW_THREADS
|
||||
window->display();
|
||||
Py_END_ALLOW_THREADS
|
||||
}
|
||||
|
||||
// #219 - Safe window for background threads to modify UI
|
||||
// This runs AFTER display() but BEFORE the next frame's processing
|
||||
if (frameLock.hasWaiting()) {
|
||||
frameLock.openWindow();
|
||||
// Release GIL so waiting threads can proceed with their mcrfpy.lock() blocks
|
||||
Py_BEGIN_ALLOW_THREADS
|
||||
frameLock.closeWindow(); // Wait for all lock holders to complete
|
||||
Py_END_ALLOW_THREADS
|
||||
}
|
||||
|
||||
currentFrame++;
|
||||
|
|
|
|||
|
|
@ -13,6 +13,10 @@
|
|||
#include "ImGuiConsole.h"
|
||||
#include <memory>
|
||||
#include <sstream>
|
||||
#include <mutex>
|
||||
#include <condition_variable>
|
||||
#include <atomic>
|
||||
#include <thread>
|
||||
|
||||
/**
|
||||
* @brief Performance profiling metrics structure
|
||||
|
|
@ -79,6 +83,59 @@ struct ProfilingMetrics {
|
|||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* @brief Thread synchronization primitive for safe UI updates from background threads (#219)
|
||||
*
|
||||
* Allows background Python threads to safely update UI objects by waiting for
|
||||
* a "safe window" between frames when the render loop is not iterating the scene graph.
|
||||
*
|
||||
* Usage from Python:
|
||||
* with mcrfpy.lock():
|
||||
* frame.x = new_value # Safe to modify UI here
|
||||
*/
|
||||
class FrameLock {
|
||||
private:
|
||||
std::mutex mtx;
|
||||
std::condition_variable cv;
|
||||
bool safe_window = false;
|
||||
std::atomic<int> waiting{0};
|
||||
std::atomic<int> active{0};
|
||||
|
||||
public:
|
||||
/**
|
||||
* @brief Acquire the lock, blocking until safe window opens
|
||||
*
|
||||
* Called by mcrfpy.lock().__enter__. Releases GIL while waiting.
|
||||
*/
|
||||
void acquire();
|
||||
|
||||
/**
|
||||
* @brief Release the lock
|
||||
*
|
||||
* Called by mcrfpy.lock().__exit__
|
||||
*/
|
||||
void release();
|
||||
|
||||
/**
|
||||
* @brief Open the safe window, allowing waiting threads to proceed
|
||||
*
|
||||
* Called by render loop between frames
|
||||
*/
|
||||
void openWindow();
|
||||
|
||||
/**
|
||||
* @brief Close the safe window, waiting for all active threads to finish
|
||||
*
|
||||
* Called by render loop before resuming rendering
|
||||
*/
|
||||
void closeWindow();
|
||||
|
||||
/**
|
||||
* @brief Check if any threads are waiting for the lock
|
||||
*/
|
||||
bool hasWaiting() const { return waiting.load() > 0; }
|
||||
};
|
||||
|
||||
class GameEngine
|
||||
{
|
||||
public:
|
||||
|
|
@ -136,6 +193,10 @@ private:
|
|||
ImGuiConsole console;
|
||||
bool imguiInitialized = false;
|
||||
|
||||
// #219 - Thread synchronization for background Python threads
|
||||
FrameLock frameLock;
|
||||
std::thread::id main_thread_id; // For detecting if lock() is called from main thread
|
||||
|
||||
void updateViewport();
|
||||
|
||||
void testTimers();
|
||||
|
|
@ -198,6 +259,10 @@ public:
|
|||
int getSimulationTime() const { return simulation_time; }
|
||||
void renderScene(); // Force render current scene (for synchronous screenshot)
|
||||
|
||||
// #219 - Thread synchronization for background threads
|
||||
FrameLock& getFrameLock() { return frameLock; }
|
||||
bool isMainThread() const { return std::this_thread::get_id() == main_thread_id; }
|
||||
|
||||
// global textures for scripts to access
|
||||
std::vector<IndexTexture> textures;
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@
|
|||
#include "PyHeightMap.h" // Procedural generation heightmap (#193)
|
||||
#include "PyBSP.h" // Procedural generation BSP (#202-206)
|
||||
#include "PyNoiseSource.h" // Procedural generation noise (#207-208)
|
||||
#include "PyLock.h" // Thread synchronization (#219)
|
||||
#include "McRogueFaceVersion.h"
|
||||
#include "GameEngine.h"
|
||||
#include "ImGuiConsole.h"
|
||||
|
|
@ -292,6 +293,17 @@ static PyMethodDef mcrfpyMethods[] = {
|
|||
{"__getattr__", mcrfpy_module_getattr, METH_VARARGS,
|
||||
"Module-level __getattr__ for dynamic properties (current_scene, scenes)"},
|
||||
|
||||
// #219: Thread synchronization
|
||||
{"lock", PyLock::lock, METH_NOARGS,
|
||||
MCRF_FUNCTION(lock,
|
||||
MCRF_SIG("()", "_LockContext"),
|
||||
MCRF_DESC("Get a context manager for thread-safe UI updates from background threads."),
|
||||
MCRF_RETURNS("_LockContext: A context manager that blocks until safe to modify UI")
|
||||
MCRF_NOTE("Use with `with mcrfpy.lock():` to safely modify UI objects from a background thread. "
|
||||
"The context manager blocks until the render loop reaches a safe point between frames. "
|
||||
"Without this, modifying UI from threads may cause visual glitches or crashes.")
|
||||
)},
|
||||
|
||||
{NULL, NULL, 0, NULL}
|
||||
};
|
||||
|
||||
|
|
@ -479,7 +491,13 @@ PyObject* PyInit_mcrfpy()
|
|||
PyUILineType.tp_weaklistoffset = offsetof(PyUILineObject, weakreflist);
|
||||
PyUICircleType.tp_weaklistoffset = offsetof(PyUICircleObject, weakreflist);
|
||||
PyUIArcType.tp_weaklistoffset = offsetof(PyUIArcObject, weakreflist);
|
||||
|
||||
|
||||
// #219 - Initialize PyLock context manager type
|
||||
if (PyLock::init() < 0) {
|
||||
std::cout << "ERROR: PyLock::init() failed" << std::endl;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// Process exported types - PyType_Ready AND add to module
|
||||
int i = 0;
|
||||
auto t = exported_types[i];
|
||||
|
|
|
|||
100
src/PyLock.cpp
Normal file
100
src/PyLock.cpp
Normal file
|
|
@ -0,0 +1,100 @@
|
|||
// #219 - Thread synchronization context manager for mcrfpy.lock()
|
||||
#include "PyLock.h"
|
||||
#include "GameEngine.h"
|
||||
#include "Resources.h"
|
||||
|
||||
// Forward declarations
|
||||
static PyObject* PyLockContext_enter(PyLockContextObject* self, PyObject* args);
|
||||
static PyObject* PyLockContext_exit(PyLockContextObject* self, PyObject* args);
|
||||
static void PyLockContext_dealloc(PyLockContextObject* self);
|
||||
static PyObject* PyLockContext_new(PyTypeObject* type, PyObject* args, PyObject* kwds);
|
||||
|
||||
// Context manager methods
|
||||
static PyMethodDef PyLockContext_methods[] = {
|
||||
{"__enter__", (PyCFunction)PyLockContext_enter, METH_NOARGS,
|
||||
"Acquire the frame lock, blocking until safe window opens"},
|
||||
{"__exit__", (PyCFunction)PyLockContext_exit, METH_VARARGS,
|
||||
"Release the frame lock"},
|
||||
{NULL}
|
||||
};
|
||||
|
||||
// Type definition
|
||||
PyTypeObject PyLockContextType = {
|
||||
.ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0},
|
||||
.tp_name = "mcrfpy._LockContext",
|
||||
.tp_basicsize = sizeof(PyLockContextObject),
|
||||
.tp_itemsize = 0,
|
||||
.tp_dealloc = (destructor)PyLockContext_dealloc,
|
||||
.tp_flags = Py_TPFLAGS_DEFAULT,
|
||||
.tp_doc = "Thread synchronization context manager for safe UI updates from background threads",
|
||||
.tp_methods = PyLockContext_methods,
|
||||
.tp_new = PyLockContext_new,
|
||||
};
|
||||
|
||||
static PyObject* PyLockContext_new(PyTypeObject* type, PyObject* args, PyObject* kwds) {
|
||||
PyLockContextObject* self = (PyLockContextObject*)type->tp_alloc(type, 0);
|
||||
if (self) {
|
||||
self->acquired = false;
|
||||
}
|
||||
return (PyObject*)self;
|
||||
}
|
||||
|
||||
static void PyLockContext_dealloc(PyLockContextObject* self) {
|
||||
// Safety: if we're destroyed while holding the lock, release it
|
||||
if (self->acquired && Resources::game) {
|
||||
Resources::game->getFrameLock().release();
|
||||
}
|
||||
Py_TYPE(self)->tp_free((PyObject*)self);
|
||||
}
|
||||
|
||||
static PyObject* PyLockContext_enter(PyLockContextObject* self, PyObject* args) {
|
||||
if (!Resources::game) {
|
||||
PyErr_SetString(PyExc_RuntimeError, "Game engine not initialized");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// #219 - If we're on the main thread, no-op (already synchronized)
|
||||
// This allows the same code to work from callbacks, script init, AND background threads
|
||||
if (Resources::game->isMainThread()) {
|
||||
self->acquired = false; // Don't try to release what we didn't acquire
|
||||
Py_INCREF(self);
|
||||
return (PyObject*)self;
|
||||
}
|
||||
|
||||
// Acquire the frame lock - this will block (with GIL released) until safe window
|
||||
Resources::game->getFrameLock().acquire();
|
||||
self->acquired = true;
|
||||
|
||||
// Return self for the `with` statement
|
||||
Py_INCREF(self);
|
||||
return (PyObject*)self;
|
||||
}
|
||||
|
||||
static PyObject* PyLockContext_exit(PyLockContextObject* self, PyObject* args) {
|
||||
// args contains (exc_type, exc_val, exc_tb) - we don't suppress exceptions
|
||||
if (self->acquired && Resources::game) {
|
||||
Resources::game->getFrameLock().release();
|
||||
self->acquired = false;
|
||||
}
|
||||
|
||||
// Return False to not suppress any exceptions
|
||||
Py_RETURN_FALSE;
|
||||
}
|
||||
|
||||
// The lock() function that users call - returns a new context manager
|
||||
PyObject* PyLock::lock(PyObject* self, PyObject* args) {
|
||||
if (!Resources::game) {
|
||||
PyErr_SetString(PyExc_RuntimeError, "Game engine not initialized");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// Create and return a new context manager object
|
||||
return PyObject_CallObject((PyObject*)&PyLockContextType, NULL);
|
||||
}
|
||||
|
||||
int PyLock::init() {
|
||||
if (PyType_Ready(&PyLockContextType) < 0) {
|
||||
return -1;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
25
src/PyLock.h
Normal file
25
src/PyLock.h
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
#pragma once
|
||||
// #219 - Thread synchronization context manager for mcrfpy.lock()
|
||||
|
||||
#include <Python.h>
|
||||
|
||||
// Forward declaration
|
||||
class GameEngine;
|
||||
|
||||
// PyLockContext - the context manager object returned by mcrfpy.lock()
|
||||
typedef struct {
|
||||
PyObject_HEAD
|
||||
bool acquired; // Track if we've acquired the lock
|
||||
} PyLockContextObject;
|
||||
|
||||
// The type object for the context manager
|
||||
extern PyTypeObject PyLockContextType;
|
||||
|
||||
// Module initialization function - adds lock() function to module
|
||||
namespace PyLock {
|
||||
// Create the lock() function that returns a context manager
|
||||
PyObject* lock(PyObject* self, PyObject* args);
|
||||
|
||||
// Initialize the type (call PyType_Ready)
|
||||
int init();
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue