Savedata dialog: serialize ioThreadStatus (bump section to v3) - #21987
Open
Arkadyzja wants to merge 1 commit into
Open
Savedata dialog: serialize ioThreadStatus (bump section to v3)#21987Arkadyzja wants to merge 1 commit into
Arkadyzja wants to merge 1 commit into
Conversation
The s > 2 branch in PSPSaveDialog::DoState has been dead code since it was written - the section version was never raised past 2, so ioThreadStatus was reset to SAVEIO_NONE on every savestate load. Loading a state that was saved while a savedata operation was in flight then repeated the operation (or left the dialog waiting on a completion that had already happened), instead of resuming from the recorded status. Restoring the value is safe: DoState joins the IO thread before serializing, so the stored status is only ever SAVEIO_NONE or SAVEIO_DONE, and the operation's effects are already part of the serialized emulated RAM. Old (v2) states still load through the reset path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
One-line fix (plus a comment): bump the
PSPSaveDialogsavestate section from max version 2 to 3, which activates the existing-but-deads > 2branch that serializesioThreadStatus.The bug
The
if (s > 2) { Do(p, ioThreadStatus); }branch inPSPSaveDialog::DoStateappears to have been dead code since it was written — the section declaration stayed atp.Section("PSPSaveDialog", 1, 2), so the version can never exceed 2 and every load takes theelsepath, resettingioThreadStatustoSAVEIO_NONE.Consequence: loading a state that was taken while a savedata operation was in flight loses the operation's completion status. Depending on where the dialog's state machine was, it either repeats the IO or waits on a completion that has already happened, instead of resuming from the recorded status.
Why restoring the value is safe
DoStatejoins the IO thread (ioThread.join()) before serializing anything, so the value that ends up in the state is only everSAVEIO_NONEorSAVEIO_DONE— never a mid-operation status.SAVEIO_DONEis consistent with the rest of the loaded state.Compatibility