From 9cb0647199629a7c3959ab90121140b8d621bb2d Mon Sep 17 00:00:00 2001 From: Rix Date: Sat, 10 Oct 2026 17:10:27 -0700 Subject: [PATCH] rewind: take the snapshot off the ring before restoring it restore() signals SaveStateLoaded synchronously. When the rewind runs from the main loop, a listener calling rewindState() steps back right away, while rewindStateNow() still held the snapshot at the back of the ring: the nested call landed on it again, and with a single snapshot the outer call then popped an empty ring. The test helpers resumed on the emulation stack after a vsync pause, and runIntoCallStack() paused before the guest ran when called from an event loop callback; both are fixed so the new test can sit mid-suite. --- src/core/psxemulator.cc | 9 +++-- tests/lua/rewind.lua | 77 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/core/psxemulator.cc b/src/core/psxemulator.cc index 288a3aff..006348a1 100644 --- a/src/core/psxemulator.cc +++ b/src/core/psxemulator.cc @@ -260,10 +260,13 @@ bool PCSX::Emulator::rewindStateNow(unsigned steps) { ring.m_states.pop_back(); } // Step back to the most recent snapshot and consume it, so successive calls walk further - // into the past. restore() invalidates the code cache itself, since it does not reset the cpu. - SaveStates::restore(*ring.m_states.back()); - ring.m_spare.push_back(std::move(ring.m_states.back())); + // into the past. Take it off the ring first: restore() signals SaveStateLoaded, and a + // listener that rewinds again has to step past this snapshot, not land on it a second time. + // restore() invalidates the code cache itself, since it does not reset the cpu. + auto state = std::move(ring.m_states.back()); ring.m_states.pop_back(); + SaveStates::restore(*state); + ring.m_spare.push_back(std::move(state)); return true; } diff --git a/tests/lua/rewind.lua b/tests/lua/rewind.lua index 9a796aec..1e3e562e 100644 --- a/tests/lua/rewind.lua +++ b/tests/lua/rewind.lua @@ -71,10 +71,15 @@ end -- Hands control back to a test from the main loop, once the cpu has unwound out of Execute(). -- Resuming straight from a listener leaves the rest of the suite on the emulation stack, where a --- rewind is only queued. +-- rewind is only queued. A vsync polls the event loop too, so the first tick after a pause from +-- a vsync listener still runs on that stack; the cpu has unwound by the second one. local function resumeFromMainLoop(co) local timer = luv.new_timer() - timer:start(0, 0, function() + local ticks = 0 + timer:start(0, 1, function() + ticks = ticks + 1 + if ticks < 2 then return end + timer:stop() timer:close() coroutine.resume(co) end) @@ -94,7 +99,11 @@ local function runIntoCallStack() regs.GPR.n.ra = 0x80100100 regs.pc = 0x80100000 local co = coroutine.running() - PCSX.nextTick(function() + -- Pause on a vsync, which is certain to come after the guest ran: a nextTick queued from a + -- test that was itself resumed by an event loop callback fires before the cpu starts. + local vsync + vsync = PCSX.Events.createEventListener('GPU::Vsync', function() + vsync:remove() PCSX.pauseEmulator() resumeFromMainLoop(co) end) @@ -120,6 +129,68 @@ function TestRewind:test_round_trips_on_recycled_slots_stay_flat() end end +-- Queues a rewind from a vsync listener and lets the main loop land it. restore() signals +-- SaveStateLoaded before rewindStateNow() returns, and the listener rewinds once more from there: +-- it is off the emulation stack, so that one runs straight away, while the outer step is still +-- in flight. Returns what the nested call returned. +local function rewindTwiceFromTheMainLoop() + local co = coroutine.running() + local r = {} + -- One shot, and gone before it rewinds: the nested restore signals SaveStateLoaded too, and + -- that must not call back into Lua from inside the rewindState() call. + local loaded + loaded = PCSX.Events.createEventListener('ExecutionFlow::SaveStateLoaded', function() + loaded:remove() + r.loads = true + r.nested = PCSX.rewindState() + end) + local vsyncs = 0 + local vsync + vsync = PCSX.Events.createEventListener('GPU::Vsync', function() + vsyncs = vsyncs + 1 + if vsyncs == 1 then + r.queued = PCSX.rewindState() + elseif r.loads then + vsync:remove() + PCSX.pauseEmulator() + resumeFromMainLoop(co) + end + end) + PCSX.resumeEmulator() + coroutine.yield() + lu.assertTrue(r.queued) + return r.nested +end + +-- The nested rewind has to step past the snapshot the outer one is landing on, not land on it +-- a second time. +function TestRewind:test_rewind_from_a_load_listener() + while PCSX.getRewindStateCount() > 0 do PCSX.rewindState() end + -- Park the cpu in a loop before taking the snapshots, so it runs on from there after a rewind. + local code = ram32(0x100300) + code[0] = 0x080400c0 -- j 0x80100300 + code[1] = 0x00000000 -- nop + PCSX.invalidateCache() + PCSX.getRegisters().pc = 0x80100300 + local word = ram32(0x1f0008) + word[0] = 0x11111111 + PCSX.createRewindState() + word[0] = 0x22222222 + PCSX.createRewindState() + word[0] = 0x33333333 + lu.assertTrue(rewindTwiceFromTheMainLoop()) + lu.assertEquals(word[0], 0x11111111) + lu.assertEquals(PCSX.getRewindStateCount(), 0) + + -- With the last snapshot in flight there is nothing left for the listener to step back to. + word[0] = 0x44444444 + PCSX.createRewindState() + word[0] = 0x55555555 + lu.assertFalse(rewindTwiceFromTheMainLoop()) + lu.assertEquals(word[0], 0x44444444) + lu.assertEquals(PCSX.getRewindStateCount(), 0) +end + -- A rewind asked for from a vsync listener runs in the middle of Counters::update(). It has to -- wait for the main loop, then land, and the machine has to keep going from the snapshot. function TestRewind:test_vsync_rewind_waits_for_the_main_loop()