MCGalaxy icon indicating copy to clipboard operation
MCGalaxy copied to clipboard

Level saving should use File.Replace

Open rdebath opened this issue 2 years ago • 2 comments

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 avatar Feb 06 '22 14:02 rdebath

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 avatar Feb 17 '22 20:02 rdebath

See #779 for patch

rdebath avatar Jan 10 '24 08:01 rdebath