store_string can corrupt files on Windows

[tcoxon]

Godot version

3.5.1.stable

System information

Windows

Issue description

A handful of Cassette Beasts players have reported their save files become corrupt on Windows.

In Cassette Beasts save files are written on a background thread using File.open_compressed and File.store_string. Files are first saved to a temporary path, then flushed and closed once the game has finished writing them. If no error codes were returned from the File methods, it then uses Directory.copy to replace the save file.

When corruption occurs, both the canonical save file and the temporary file are 0 bytes long. No error codes are returned from the File or Directory methods, and yet this message appears repeatedly in the player's logs:

ERROR: Condition "fwrite(p_src, 1, p_length, f) != (size_t)p_length" is true.
   at: FileAccessWindows::store_buffer (drivers\windows\file_access_windows.cpp:301) - Condition "fwrite(p_src, 1, p_length, f) != (size_t)p_length" is true.

Microsoft's documentation for fwrite says that the number of bytes written can be less than the number given if an error occurs. Godot should return an error code when this happens so that games can handle errors and attempt to recover, instead of logging an error and returning OK.

I'm not sure what the error is in these cases, so I don't know exactly how to reproduce this. I've checked the obvious stuff with players - disk space, etc. and haven't found any particular reason for it.

Steps to reproduce

As mentioned, I'm unsure of the reproduction steps. However I have attached a minimal project that recreates every step along the path that Cassette Beasts takes when it writes a save file.

Minimal reproduction project

file_corruption.zip

[akien-mga]

CC @bruvzg

[goblinJoel]

Found this while looking for how to tell if the write was successful. In case anyone else finds it helpful, my workaround when updating an existing save file is to close the temporary file after writing, open it up again, var contents := file.get_as_text(false), close it again, and check if contents == json, where json is the string I passed to store_string(). Only if the expected and actual contents are equal do I dir.rename(temp_file, save_file).

All the checking at various stages of the process does make my code rather verbose. On large enough save files, this could also be slow.