Thealexbarney / LibHac

A library that reimplements parts of the Nintendo Switch OS
Other
397 stars 50 forks source link

Fix `E_ACCESSDENIED` error when renaming directory #276

Closed jamestiotio closed 1 year ago

jamestiotio commented 1 year ago

There are cases when file managers or file explorers are used to access and modify save data files in the save data directory. This may cause the save data directory to be locked if those applications do not properly release or close the directory handle. Thus, when calls to DirectorySaveDataFileSystem.DoCommit() are made, which would invoke LocalFileSystem.RenameDirInternal() in turn, .NET would keep throwing an IOException with an HResult error code of 0x80070005 (E_ACCESSDENIED), indicating an Access Denied error.

This commit somehow fixes this issue by manually moving and renaming each subdirectory and file from the source directory to the dest directory before manually deleting the source directory. It seems that DirectoryInfo operations are known to be quite janky and sensitive to those directories being used by other processes on the machine. This proposed implementation is supposed to be equivalent to the previous implementation and it is tested to be working on Ryujinx.

This fixes Ryujinx/Ryujinx#5024.

Signed-off-by: James Raphael Tiovalen jamestiotio@gmail.com

jamestiotio commented 1 year ago

Note that to test this, we just have to invoke a write operation (which does not necessarily have to involve an edited save file). I find the issue itself very easily and reliably reproducible by simply copying any file in the save directory (such as progress.sav) to a temporary folder anywhere else and copying it back to overwrite that file without closing the file manager application or navigating away from that directory after that. Using this approach, we can verify that the changes introduced in this PR would fix this issue.

Thealexbarney commented 1 year ago

This solution won't work that well for at least a few reasons:

  1. Renaming a directory needs to be an atomic operation to avoid the save data being left in an inconsistent state if something goes wrong during the commit.
  2. Even outside the context of committing save data, if something goes wrong during the directory rename, the file system could be left in an inconsistent state.
  3. It just punts the issue of FS entries being held open down to the subdirectory level. You'll still have the same issue if something in a subdirectory is being held open, except this time the FS could be left in an inconsistent state with some of the entries in the new directory and some in the old directory
jamestiotio commented 1 year ago

Fair enough, thank you for the comments! I think I will have to find a different approach to solving this issue. Will close this PR.