Fix PinMAME busy-wait freeze on Controller.Stop() (dispose() hang)
Controller::Stop() (real desktop vpinball code, unmodified until now) busy-waits for PinmameIsRunning() to clear after calling PinmameStop(), in a sleep loop meant for a real OS thread to notice the quit flag and finish stopping on its own. Under Emscripten there is no such thread - PinmameStop() there only sets the quit flag - so the loop spun forever on the single available thread, with nothing left to ever clear it. This fires from Player::~Player()'s GameEvents_Exit script event (controller.vbs's default Exit handler calls Controller.Stop), which runs synchronously inside dispose() - hanging any consumer's teardown for a ROM-based table, most visibly React StrictMode/unmount cleanup calling stop() then dispose() shortly after. Fixed by driving one more (now-instant, since the quit flag is already set) PinmameEmscriptenStep() call directly under __EMSCRIPTEN__ instead of busy-waiting - it runs cpu_post_run() and OnStateChange(0) synchronously right there. Also adds a "Stop & Dispose" button to the basic example, exercising the same stop()-then-wait-two-frames-then-dispose() sequence consumers use, to make this kind of regression visible without a separate app. Confirmed fixed by hands-on testing: dispose() on a running ROM-based table now returns immediately instead of hanging the tab. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -39,6 +39,13 @@
|
||||
background: rgba(255,255,255,0.15); color: #eee; cursor: pointer;
|
||||
}
|
||||
#fullscreen-button.hidden { display: none; }
|
||||
|
||||
#dispose-button {
|
||||
position: absolute; top: 12px; left: 12px; z-index: 10;
|
||||
padding: 8px 14px; border-radius: 6px; border: none;
|
||||
background: rgba(220,38,38,0.85); color: #fff; cursor: pointer;
|
||||
}
|
||||
#dispose-button.hidden { display: none; }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
@@ -54,6 +61,7 @@
|
||||
<button id="start-button" disabled>Loading...</button>
|
||||
</div>
|
||||
<button id="fullscreen-button" class="hidden">Fullscreen</button>
|
||||
<button id="dispose-button" class="hidden">Stop & Dispose</button>
|
||||
</div>
|
||||
|
||||
<script type="module">
|
||||
@@ -66,6 +74,7 @@
|
||||
const fileInput = document.getElementById('table-file');
|
||||
const romInput = document.getElementById('rom-file');
|
||||
const fullscreenButton = document.getElementById('fullscreen-button');
|
||||
const disposeButton = document.getElementById('dispose-button');
|
||||
|
||||
let uploadedTableData;
|
||||
fileInput.addEventListener('change', async () => {
|
||||
@@ -104,6 +113,7 @@
|
||||
pinball.start();
|
||||
overlay.classList.add('hidden');
|
||||
fullscreenButton.classList.remove('hidden');
|
||||
disposeButton.classList.remove('hidden');
|
||||
|
||||
// Touch controls are additive UI for touch-capable devices - not
|
||||
// required for desktop mouse+keyboard play.
|
||||
@@ -118,6 +128,26 @@
|
||||
// browser denies it) - nothing to recover from here.
|
||||
});
|
||||
});
|
||||
|
||||
// Mirrors the stop()-then-wait-a-couple-frames-then-dispose() pattern
|
||||
// consumers need: stop() only takes effect on the engine's next
|
||||
// internal step, so disposing synchronously right after can race it.
|
||||
disposeButton.addEventListener('click', () => {
|
||||
console.log('[dispose test] calling stop()...');
|
||||
pinball.stop();
|
||||
requestAnimationFrame(() => {
|
||||
requestAnimationFrame(() => {
|
||||
console.log('[dispose test] calling dispose()...');
|
||||
pinball.dispose();
|
||||
console.log('[dispose test] dispose() returned - no hang');
|
||||
disposeButton.classList.add('hidden');
|
||||
fullscreenButton.classList.add('hidden');
|
||||
overlay.classList.remove('hidden');
|
||||
startButton.textContent = 'Disposed (reload page to restart)';
|
||||
startButton.disabled = true;
|
||||
});
|
||||
});
|
||||
});
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
diff --git a/plugins/pinmame/Controller.cpp b/plugins/pinmame/Controller.cpp
|
||||
index 29c2405..a498fee 100644
|
||||
--- a/plugins/pinmame/Controller.cpp
|
||||
+++ b/plugins/pinmame/Controller.cpp
|
||||
@@ -265,8 +265,20 @@ void Controller::Stop()
|
||||
if (PinmameIsRunning())
|
||||
{
|
||||
PinmameStop();
|
||||
+#ifdef __EMSCRIPTEN__
|
||||
+ // PinmameStop() only sets a "please quit" flag here (see its own
|
||||
+ // Emscripten branch) - there's no separate OS thread that will ever
|
||||
+ // notice it and finish stopping on its own, so busy-waiting for
|
||||
+ // PinmameIsRunning() to clear on this single thread would spin
|
||||
+ // forever. Drive one more step directly instead: with the quit flag
|
||||
+ // already set, it's a bounded, instant call that completes the
|
||||
+ // pending teardown (calls cpu_post_run() and OnStateChange(0))
|
||||
+ // synchronously right here.
|
||||
+ PinmameEmscriptenStep();
|
||||
+#else
|
||||
while (PinmameIsRunning() != 0) // Wait until the machine is stopped
|
||||
std::this_thread::sleep_for(std::chrono::milliseconds(75));
|
||||
+#endif
|
||||
if (m_onGameEndHandler)
|
||||
m_onGameEndHandler(this);
|
||||
}
|
||||
Reference in New Issue
Block a user