grumpycoders / grumpycoders/pcsx-redux
Loading a savestate from a VSync Lua callback partially mutates the restored state
- 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
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