ClassiCube / MCGalaxy

A Minecraft Classic / ClassiCube server software
GNU General Public License v3.0
168 stars 79 forks source link

Level saving should use File.Replace #683

Open rdebath opened 2 years ago

rdebath commented 2 years ago

Currently the process is:

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 ...

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 commented 2 years ago

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 commented 7 months ago

See #779 for patch