godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.1k stars 21.16k forks source link

ResourceSaver.save writes without using a temporary file, allowing corruption. #48225

Open lyuma opened 3 years ago

lyuma commented 3 years ago

Godot version: 4.0.dev d1dc28e

OS/device including version: Windows 10.0.19041.928

Issue description: If the user hits ctrl-c or kills Godot while it is writing a resource, that resource is corrupted.

The case I'm running into is from killing godot while it's still loading the project list.

ERROR: C:\Users/USERNAME/AppData/Roaming/Godot/editor_settings-4.tres:1 - Parse Error:
   at: ResourceLoaderText::open (scene\resources\resource_format_text.cpp:887)
ERROR: Failed loading resource: C:\Users/USERNAME/AppData/Roaming/Godot/editor_settings-4.tres. Make sure resources have been imported by opening the project in the editor at least once.
   at: (core\io\resource_loader.cpp:203)
WARNING: Could not open config file.
     at: EditorSettings::create (editor\editor_settings.cpp:1044)

It is never acceptable for a crash to cause resource corruption. It should either save the old resource state, or the new state, not corrupt the resource.

Steps to reproduce:

  1. Edit scene\resources\resource_format_text.cpp and add an error log at line 1448.
  2. Build Godot in debug
  3. Open the project selector window.
  4. While it is slowly printing error logs to the console (takes half a minute for me), kill godot, e.g. from task manager.
  5. Open the project selector window again. the project list or editor theme will be corrupted.

Minimal reproduction project: occurs during project selector window.

nonunknown commented 3 years ago

I can confirm the bug, I just created a 3d scene with some tool scripts, and opening them again the following error message is thrown:

  scene/resources/resource_format_text.cpp:555 - res://Scene.tscn:12 - Parse Error: Expected float in constructor
  Failed loading resource: res://Scene.tscn. Make sure resources have been imported by opening the project in the editor at least once.
  editor/editor_data.cpp:539 - Index p_idx = 1 is out of bounds (edited_scene.size() = 1).
Calinou commented 3 years ago

FileAccess already has a safe backup mechanism: https://github.com/godotengine/godot/blob/8cd1b59ea78f5145eae1762e2b1311c1a1b92cbc/editor/editor_node.cpp#L5689

Example on UNIX: https://github.com/godotengine/godot/blob/8cd1b59ea78f5145eae1762e2b1311c1a1b92cbc/drivers/unix/file_access_unix.cpp#L113-L117

Example on Windows: https://github.com/godotengine/godot/blob/8cd1b59ea78f5145eae1762e2b1311c1a1b92cbc/drivers/windows/file_access_windows.cpp#L111-L114