Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Level saving should use File.Replace #683

Open
rdebath opened this issue Feb 6, 2022 · 2 comments
Open

Level saving should use File.Replace #683

rdebath opened this issue Feb 6, 2022 · 2 comments

Comments

@rdebath
Copy link
Contributor

rdebath commented Feb 6, 2022

Currently the process is:

  • The level is first copied from the *.lvl to the prev, this may leave the prev file corrupted as a File.Copy needs sufficient space to write the newer version of the file.
  • Secondly the level is written to the *.backup file, this may fail for many reasons leaving a corrupt *.backup file.
  • Thirdly the *.backup file is copied to the *.lvl file, this can leave the *.lvl file corrupted due to the disk filling up.

The file is loaded from *.lvl, *.backup or prev in that order.

Because of the way this is saved none of the files are guaranteed to be undamaged.

The usual way of doing a "safe replace" like this is to write the data to a temporary file then rename it to it's correct name once it has been completely written. That way no file that will be read ever exists in a partially written state.

Looking at the files you are writing and the available DotNET libraries I think the correct one is to use is File.Replace.
In this way ...

  • Write the level to a *.temp file (use Strm.Flush(true) if possible).
  • Use File.Replace (temp, level, prev) to place the new *.lvl and prev files.
  • Use File.Copy to recreate the *.temp file from the *.lvl file.
  • Use File.Replace (temp, backup, null) to update the *.backup file.

The File.Replace operation just has to do metadata updates and so should succeed or fail as a single operation leaving the filesystem in either the new or previous states, both of which have a complete and undamaged level file.
The File.Copy and Export functions only write to a temp so if they fail part way through nothing important is damaged.
On Mono the 3 argument Replace becomes two independent renames; this is ok.

All three files will always be complete (or missing) and the current load order will get the most recent of them.
The prev file is not rewritten so will be on the physical disk even on a hardware failure.

@rdebath
Copy link
Contributor Author

rdebath commented Feb 17, 2022

Looking back at this I feel that I should mention that the loading of the *.lvl file will not detect a truncated file as the format does not contain an end of file marker. This means that a truncated *.backup file will override a fully intact prev file.

@rdebath
Copy link
Contributor Author

rdebath commented Jan 10, 2024

See #779 for patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant