File.StoreXXX methods do not flush on write (and there is no flush method)

[andy-noisyduck]

Godot version:
3.1.1 (Mono)

OS/device including version:
Windows 10

Issue description:
When calling the store methods of file, e.g store_string(...) the content is not automatically flushed. There is also no method to do it manually instead. If the application is killed (GDScript), or even worse under Mono, just graceful exited, the unflushed content will not be written.

Closing the file before exit correctly persist the writes.

The GDScript implementation has decent cleanup and will close open files on application exit, and this will flush the contents. If you kill the application, rather than doing a graceful close, then the content will not be written.

Under Mono the behaviour is worse. You can simply close the application (e.g by clicking the X), and the un-flushed data will be lost.

Steps to reproduce:
(Mono)

1. Open a file using the File class
2. Write some content, e.g. using the StoreString method
3. Either (GDSscript) kill the running project or (Mono) simply exit the program with the X window button.
4. Examine the file for the stored content - it will not have been stored.

Minimal reproduction project:
Project includes GDScript and Mono versions for testing. Just attach whichever script version you need.
Testing-FileFlush.zip

[mhilbrunner]

(Also relevant: #29043)

[andy-noisyduck]

Separate from Mono, there's probably scope to have a discussion on whether Godot should implement a flush method anyway. I can see legit scenarios where you want to ensure data is written in real time, especially in the event of a crash (e.g. a log file), or where you don't have full control over the lifecycle (e.g. mobile apps).

[mhilbrunner]

Absolutely! Just to be clear, thats why I added both the mono and core labels. :)

[cpritchard]

I just saw a web project that simply saves a string followed by file close. Calling load right seemed to fetch the updated values but re-loading the tab then loading the value more often than not fails to get the updated value and pulls a previously persisted value. If you waited 10 seconds or more before clicking refresh it would "usually' work but still wasn't reliable.

Edit: This seems like a behaviour that would reflect a typical flush wasn't being called but there isn't one to call in godot

[andy-noisyduck]

I had another use-case today to write log files, so I've been looking at this again.

FileAccess (the abstraction used by all the specific file implementations) already exposes a public flush() method. The path of least resistance would be to just add core bindings so the method is exposed to scripts. It's not clear to me however if this is the "right" thing to do. The concept of flushing doesn't always make sense.

Both encrypted and compressed implementations do not support flushing. They are both designed having their content written to a buffer, and then on close the buffer is written to disk in either the compressed or encrypted format. FileAccessNetwork also has no concept of flushing. The behaviour of each seems to vary - encrypted and compressed both silently ignore the flush operation, whilst network will error.

Any work on this should probably make the behaviours consistent as part of the change. My preference would be to expose the flush() method with a new binding, and change all the cases where flush() makes no sense to explicitly error. This is pretty simple, but I'd like to get some thoughts on this before I go ahead and patch it.

Alternatively we could leave as is, and encourage developers to use File access as short lived only. This would only require documentation changes rather than implementation. I can't think of a use-case outside of logging where write-on-close would be a significant problem if documented.

[andy-noisyduck]

Wrong button ^^

[tx350z]

The use-case is all file systems operations MUST either work every time or return an error.

I've been chasing this problem for days. My game allows the player to save the current state and exit. The save works intermittently if the player exits the game (or web page) quickly. It is especially bad on HTML5 exports (tried multiple browsers - same behavior). Have experienced the same problem with Android app also.

We really need the ability to flush file operations manually because the automatic cache flush is unreliable in real world applications.

Code:

func save_player_data(sdata:Dictionary)->bool:
	var fd:File = File.new();
	var ferr:int = fd.open(PLAYER_DATA_FILE, File.WRITE);
	if ferr != OK:
		return false;
	var pdata:String = to_json(sdata);
	fd.store_line(pdata);
	fd.close();
	return true;

[tx350z]

I have a web export running on an IP restricted site that demonstrates the problem. If someone wants to see the problem first hand, please email maps.erpiv at homebuilt-lan.com with the IP you'll use to access the site and I'll add it to the authorized list then send you the URL for the game.

To Reproduce:

1. Browse to the game site (I'll send you the web address after your IP is authorized)
2. Click "Starting Line"
3. Click "Start Run"
4. Click "Suspend Run" at the bottom of the game board
5. Immediately close the browser
6. Browse to the game site again

If you see the main page then the player data saved did not get flushed.

Repeat the above except wait 10 seconds before closing the browser. When you browse to the site again you'll see the Resume/Discard scene. For what it's worth, if you discard this time and immediately close the browser, the saved state will still be there when you re-enter the game. Discard then wait 10 seconds before closing the browser and the saved game state will be gone.

Thanks!

[tx350z]

I've put further work on my game on hold until this problem is solved. I can't publish it if the game_state save feature is unreliable.

Suggestion: Rather than expose a flush() method, add a synchronous_write property to File and Directory that forces an immediate flush on all write operations. It can default to False. Add a warning to the docs about performance impact when synchronous write is used.

Thank you!

[Calinou]

This needs to be discussed in a feature proposal, since the current behavior is technically not a bug. (Flushing unconditionally on every write is a no-go for performance reasons. It needs to be made optional somehow.)

See also godotengine/godot-proposals#1912.

Edit: I opened a pull request to expose a flush() method in the File class: #44396