grumpycoders / grumpycoders/pcsx-redux

Loading a savestate from a VSync Lua callback partially mutates the restored state

Open
#2,088 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
994
Forks
151
Avg merge
1d 15h
Merged PRs (30d)
18

Description

### Describe the bug

Calling `PCSX.loadSaveState()` from a `GPU::VSync` Lua listener does not restore a clean, repeatable state.

The listener runs inside `Counters::update()`. After `loadSaveState()` restores the emulator, the interrupted `Counters::update()` call resumes using values from before the load and modifies the newly restored state. In repeated tests, loading the same state and running the same two frames produced identical game RAM and inputs, but different serialized savestates depending on how many loads that emulator process had performed. Differences included the `PSXINT_CDREAD` target and SPU state.

The problematic call path appears to be:

```text
Counters::update()
-> Emulator::vsync()
-> GPU::VSync Lua listener
-> PCSX.loadSaveState()
-> return to the old Counters::update() invocation
```

### Expected behavior

I would expect `PCSX.loadSaveState()` either to defer the load until the CPU/device stack has unwound, or to reject calls from an unsafe callback context. As currently exposed, it is easy for Lua code (and apparently UI code) running during VSync to create a state that is partly restored and partly inherited from the execution being replaced.

### Steps to reproduce the bug

Start whatever game and run the below LUA which will take a save state and keep reloading it then go forward few frames and check that the forwarding reproduces the same state exactly each time. Not sure I got it completely right but you should get the idea

```lua
local ffi = require("ffi")
local parent, expected
local trial, frames = 0, nil

local function bytes(slice)
return ffi.string(ffi.cast("const char *", slice.data), tonumber(slice.size))
end

local function restart()
PCSX.loadSaveState(parent) -- called from inside GPU::VSync
frames = 0
end

_G.repro = PCSX.Events.createEventListener("GPU::VSync", function()
if not parent then
parent = PCSX.createSaveState()
restart()
return
end

frames = frames + 1
if frames < 2 then return end

local child = bytes(PCSX.createSaveState())
trial = trial + 1
if expected then
assert(child == expected,
"same parent and execution produced a different state on trial " .. trial)
else
expected = child
end

if trial == 20 then
print("all trials happened to match")
PCSX.quit(0)
else
restart()
end
end)
```

I'm running -no-ui.

### Operating System

NixOS

### PCSX-Redux version

221e96bdbd9bf52e7af631864aa22b9b0513581e but the code in question seems to be the same on most recent master

### CPU model

AMD Ryzen 9 9950X3D

### GPU model & Drivers

NVIDIA GeForce GTX 1080 Ti, OpenGL 4.6.0, NVIDIA 580.126.09

### BIOS version

OpenBIOS (768938a0)

### Options

- [x] Dynarec CPU
- [ ] 8MB
- [ ] OpenGL GPU
- [ ] Fastboot
- [x] Debugger

### Iso checks

_No response_

### Logs

_No response_

### Additional information

_No response_

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the Lua listener case, then trace the named path from Counters::update() through Emulator::vsync(), GPU::VSync, and PCSX.loadSaveState(). Inspect how the interrupted update resumes after a load and how save-state serialization captures the resulting state. Done means the documented callback behavior is safe and repeated loads produce consistent serialized savestates or are explicitly rejected.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.