Repository navigation
fix(saves): journaled save commits: a crash mid-save keeps the last save - #2453
Draft
hckrman101 wants to merge 6 commits into
Draft
hckrman101 wants to merge 6 commits into
hckrman101 wants to merge 6 commits into
Conversation
nn::fs::Commit did nothing and the game wrote its save files in place, so a crash, a kill or a power cut during a save could leave a slot half written. The save mount's root is now a working copy (save-tx/<user id>/work). Commit copies it to a staging folder, flushes it, renames it ready-<n> (the commit point) and swaps it in for save/<user id>; the replaced save goes to save-backup/<user id>/<time> (the newest 3 kept). The next start finishes or rolls back a commit that was cut short and drops changes no Commit took, as on the console. save/<user id> keeps the emulators' layout, so existing saves load as they are and docs/saves.md still holds. scripts/oot_saves.py refuses to copy while a finished commit waits in save-tx. HOS_SAVE_JOURNAL=0: as before; HOS_SAVE_CRASH_AT=<n> ends the process at a commit step, for testing. host/save_journal.c is adapted from valentderah/totk-pc-recomp runtime/fs_host.c (MIT), whose copyright and permission notice it keeps. Co-authored-by: Valiantsin Dzerakh <41872026+valentderah@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The check's tree() keyed each file by os.path.relpath, which has backslashes on
Windows, while the saves it compares with are written with /: every comparison
failed on the Windows runner from the first one ("a save from before the
journal was touched"). Keys are now POSIX paths. The driver is also built with
_CRT_SECURE_NO_WARNINGS, as Windows' C runtime marks some calls deprecated.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ixes) From review of the journal: - A Commit with its working copy gone committed an empty save. It now fails instead. Each save's journal is held by a session lock (save-tx/<uid>/lock, flock / LockFileEx): a second Hookshot on the same data folder gets no journal (written in place, as before, and logged) and never runs recovery, never deletes the first one's working copy. - A Commit that failed was reported as done and its changes dropped at the last Unmount. Now the working copy is written over the save file by file instead (each file replaced whole, as before the journal). If that fails too, the changes are kept: the next Commit or the last Unmount tries again, and the next start moves the working copy to save-backup (unsaved-<time>) rather than dropping it. - A commit waiting in save-tx is put in place only over the save it was made over (a fingerprint of names, sizes and times beside it, ready-<n>.base). If something replaced the save since (HOS_SAVE_JOURNAL=0, an older Hookshot, a copied-in save, the in-place fallback), the newer save stays and the waiting commit goes to save-backup (unapplied-<time>). - A save folder that is a link (a cloud folder), or one on another drive than save-tx and save-backup, gets no journal (written in place, logged). - journal_lock is taken with no virtual core held (core given up first), and Unmounts of other mounts no longer take it. - The control bridge's saves command lists the working copy (it was unavailable with the journal); docs/control.md says so. - Windows: durable renames flush both folders after MoveFileExW (WRITE_THROUGH does not flush a rename within one volume); a name too long in UTF-8 fails a listing instead of being skipped. - A rollback that failed (the save left in save-tx/old) stops the game with a message instead of mounting a new, empty save over it. - Backups are pruned by their exact name pattern only. The MIT notice notes that totk-pc-recomp's fs_host.c is itself from MacNativeGames; the readme's licence list and docs/design.md's reuse table name it. docs/saves.md's restore steps empty the save folder first. tests/test_save_journal.py checks each: a second process, a Commit with no working copy, a stale waiting commit, a linked save, the in-place fallback, changes that cannot be saved at all, and a failed rollback. Co-authored-by: Valiantsin Dzerakh <41872026+valentderah@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From re-review of the journal (a write() -> ENOSPC driver lost a save two ways while the game was told it saved): - A working copy that differs from the newest save is never deleted. The unsaved marker no longer needs space: save-tx/<uid>/unsaved.armed, made with each working copy, is renamed to unsaved. The session also keeps an in-memory flag, and a stray unsaved.tmp counts as a marker. A remount in the same run (after a last Unmount whose retry failed) mounts the working copy as it is instead of copying the older save over it, or stopping the game. The next start commits the kept changes (journal, then in place); only if that fails too are they moved to save-backup/unsaved-<time>. Changes the game never committed are set aside as uncommitted-<time> (the newest 3) rather than deleted. - The in-place fallback runs in two phases: every file is written beside the save as <name>.hs-new and flushed, then all are renamed over. A failure in the first phase removes them and leaves the save untouched, never part old, part new. - A commit waiting over a save that was replaced since is no longer the newest save when it cannot be moved aside. - Windows: only symbolic links and junctions count as links. Other reparse points (OneDrive placeholders) are copied as files and folders instead of being left out of the copy. replace_file flushes its folder. - The in-place target is the same for every mount of a run and for IsSaveDataExisting. A run without the journal no longer makes an empty save over a failed rollback (it stops, as the journal does). The next journaled start puts the old save back over an empty one. - join no longer trips gcc -O2 -Wformat-truncation. The test builds at -O2. The backup list is sorted before it is capped. tests/test_save_journal.py builds a second driver whose write() fails with ENOSPC from a chosen call on (Linux). It covers a full disk at Commit and at the next start, a remount with no marker on disk, the in-place fallback failing at each write call, and a stale commit that cannot be moved aside. All of these fail on the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Joey-Magnus
reviewed
Nov 11, 2026
Joey-Magnus
left a comment
Contributor
There was a problem hiding this comment.
The journal is on by default (HOS_SAVE_JOURNAL=0 turns it off), but no in-game save with changes, reload, or kill during a save has been run yet. Uncommitted changes are now dropped, where before Commit did nothing, so any save path that never calls Commit would silently lose progress. The game is also told a save worked when it failed, and the review fixes haven't been built. The crash-recovery design is solid in the game-free test. One real save/reload/kill run before marking ready would settle it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the third review of the journal. The full-disk handling (in-place fallback, unsaved marker, committing a kept working copy at the next start) could leave a mixed save after a kill or a held file, commit a torn working copy, or commit over a newer save. It is replaced by a simpler design in which those states cannot arise: - A Commit needs no free space. The working copy is flushed, then renamed to ready-<n> (the commit point), then swapped in by renames as before. The fingerprint of the save it was made over is written when the working copy is made (work.base) and renamed with it. - Only the next working copy needs space. If it cannot be made, the rest of the run writes the newest save in place, as before the journal, and says so in the log. The next start uses the journal again. - A Commit that fails (a file still open for writing, a folder another program holds) leaves the save as it was and the working copy mounted. The next Commit or the last Unmount tries again. - A working copy left at start (a crash, a close before saving, a failed Commit) is never committed or loaded. Unless it holds the newest save, it is renamed to save-tx/<uid>/uncommitted-<time>, needing no space; the newest 3 are kept. One that cannot be renamed is never mounted again: the run writes in place. - Removed: the two-phase in-place fallback, the unsaved marker and its in-memory flag, and committing a kept working copy at the next start. - A swap that landed just before a crash leaves its ready-<n>.base (now deleted last). Recovery reads it as such, so a committed empty save is not rolled back. - A waiting commit that cannot be moved aside keeps its base, so it is not loaded at the next start either. unapplied-* is pruned to 3. - With the journal off (HOS_SAVE_JOURNAL=0, an unsuitable folder), a commit waiting in save-tx that was made over the current save is the save that is written. MountSaveDataReadOnly mounts the newest save the same way. Save files are opened with FILE_SHARE_DELETE, so a read handle never blocks the renames. - Windows: a reparse point whose tag cannot be read is treated as a file or folder, not a link. The fault-injection driver now also fails or kills at rename(). It covers the third review's repros: a kill before each of a commit's renames, a working copy written after a failed Commit, a save written in place after that, a torn working copy that cannot be set aside, and a stale waiting commit. It also covers a full disk at Commit, a committed empty save, and a journal-off run finding a waiting commit. Each ends with the old save or the new one, whole. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 12, 2026
Draft
hckrman101
marked this pull request as ready for review
November 12, 2026 03:33
hckrman101
marked this pull request as draft
November 12, 2026 03:33
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.
A crash, a kill or a power cut in the middle of a save can leave a save slot half written. Hookshot's
nn::fs::Commitdid nothing, so the game's writes went straight onto the save files.
What changes
The save mount's root is now a working copy,
save-tx/<user id>/work. A Commit only renames folders, so it needs nofree space:
ready-<n>. This is the commit point.save/<user id>is renamedold, andready-<n>is renamedsave/<user id>.oldgoes tosave-backup/<user id>/<UTC time>. The newest 3 are kept.Then the game's next working copy is made. That is the only step that needs space. If the disk is too full for it,
the rest of the run writes the save in place, as Hookshot did before, and the log says so. The next start uses the
journal again.
At the next start, a commit that was cut short is finished if it passed the commit point and rolled back if not. A
Commit that changed nothing writes nothing.
When things go wrong:
are never committed or deleted either: they are renamed to
save-tx/<user id>/uncommitted-<time>, and the newest 3are kept. A working copy that cannot be renamed is not mounted again; that run writes in place.
save stays as it was and the working copy stays mounted. The next Commit or the last Unmount tries again. The game is
told the save worked either way; whether it would show its own "save failed" dialog was not tried.
save-tx/<user id>/lock, flock orLockFileEx). A second Hookshot gets no journal: it writes in place, as before, and logs why.
the commit waits in
save-txand is the save the game loads. It is put in place only over the save it was made over,checked by a fingerprint of names, sizes and times. If something replaced the save since, such as
HOS_SAVE_JOURNAL=0or a copied-in save, the newer save stays. The waiting commit goes tosave-backup/<user id>/unapplied-<time>, or, if it cannot be moved, stays insave-txwithout being loaded.drive than
save-txandsave-backup. The save is written in place. On Windows only symbolic links and junctionscount as links; OneDrive placeholders are copied like any file.
empty save. A run without the journal stops too.
Other details:
save/<user id>keeps the emulators' layout, so existing saves load as they are.docs/saves.mddescribes
save-txandsave-backup.save-txis the save that gets written.MountSaveDataReadOnlymounts the newest save.scripts/oot_saves.pyrefuses to copy while a commit waits insave-tx.savescommand lists the working copy, which is what the game has now.\\?\for long paths. Save files areopened with FILE_SHARE_DELETE, so the game's own read handles never block the renames.
HOS_SAVE_JOURNAL=0writes the save in place as before.HOS_SAVE_CRASH_AT=<n>ends the process at acommit step, for testing.
Credit
host/save_journal.cis adapted from valentderah'stotk-pc-recomp
runtime/fs_host.c, which isunder the MIT licence (Copyright (c) 2026 Valiantsin Dzerakh). That file is itself adapted from MacNativeGames
games/totk/runtime/fs_host.c(MIT). Both are named at the top of the file, with the permission notice, and in thereadme's licence list and
docs/design.md's reuse table. Hookshot's changes:scripts/oot_saves.pyand players write save files in place);Verified
Game-free test.
tests/test_save_journal.pybuildshost/save_journal.cinto a small driver at-O2 -Werror.It ends the process at each of the commit's steps. After each one, the next start has the old save or the new one,
whole, and the new one exactly from the commit point on. It also checks:
Faults (Linux). A second driver makes
write()fail with ENOSPC and makesrename()fail, or end the process,at a chosen call. It covers:
Each ends with the old save or the new one, whole, never mixed, torn or silently rolled back.
Copy script test.
tests/test_oot_saves.py.Full test suite.
python -I -B tests/run_all.pypasses, apart from 3 tests this PC cannot run (no zstandard ortkinter).
Compiles.
host/nn_fs.candhost/save_journal.ccompile without warnings for Linux (gcc, clang) and Windows(llvm-mingw).
In the game. On Linux (RTX 3090), headless, on a copy of a test save, with this branch merged with main:
next working copy.
save/<user id>held the new slot afterwards, and the replaced saves were insave-backup(the newest 3).the saved slot, and its Commit at load wrote nothing.
Not tested
game.
open by another program, long paths, or a OneDrive folder. The Windows CI runs the game-free test.
thread, and making the next working copy is a copy of the save (about 1 MB).
Nearby: #2317 moves saves to a per-user data folder. This PR keeps
save-txandsave-backupbesidesaveunderhos_user_dir, so it should follow that move unchanged.