Fix file rename errors on Windows #9250
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
see also #9023 |
|
That said, retrying a rename on failure is not the worst idea in the world, though 8 attempts with a total of ~1 second delay seems pretty excessive. Imagine if you had a case where it legitimately can't rename files due to eg. missing rights and something attempted to rename even just 10 files in succession. |
|
I'm totally open to adjusting the number and timing of delays. The current set were chosen with an eye to minimizing the likelihood of a transient error still popping up a dialog, but your point about multiple files is well taken. I'll read up on #9023 but I notice it's from back in August. If finishing it would take a while we could use this PR (with adjusted delays; maybe 5/10/25 milliseconds) as a stopgap until #9023 is done. |
leoetlino
reviewed
Nov 23, 2020
a896ae7 to
c3e9c8e
Compare
lioncash
reviewed
Nov 24, 2020
c3e9c8e to
0386da9
Compare
leoetlino
requested changes
Nov 25, 2020
Source/Core/Common/FileUtil.cpp
Outdated
| const auto attempt_func = [&, source_ptr, destination_ptr]() { | ||
| if (ReplaceFile(destination_ptr, source_ptr, nullptr, REPLACEFILE_IGNORE_MERGE_ERRORS, nullptr, | ||
| nullptr)) | ||
| { |
There was a problem hiding this comment.
Same regarding the braces here -- this would be more readable if it were more compact.
Source/Core/Common/FileUtil.cpp
Outdated
| } | ||
| // Might have failed because the destination doesn't exist. | ||
| if (GetLastError() == ERROR_FILE_NOT_FOUND) | ||
| { |
leoetlino
reviewed
Nov 25, 2020
On Windows, when the Rename function fails to replace an existing file it will now retry the operation multiple times with increasingly long delays between attempts. This fixes transient rename failures. I've been getting sporadic yet annoyingly frequent errors saying: 'IOS_FS: Failed to rename temporary FST file' These typically appear on startup but I've also gotten them randomly. Investigation shows this happens when the Windows ReplaceFile function returns the error ERROR_UNABLE_TO_REMOVE_REPLACED. That happens in the context of using ReplaceFile to perform an atomic file overwrite, which is required when saving updates to a file to avoid corruption. The error mainly happens with the /Wii/fst.bin file but I've seen it happen with multiple other files as well. I haven't been able to definitively pin down why the error occurs, though online discussions suggest antivirus scanning may be a major culprit. That said, I've excluded the Dolphin folder from Windows Defender scans to no avail and don't have any other antivirus running, so this is likely to be a problem others are experiencing as well. The number and duration of retry delays is arbitrary but I feel like a combined second or so in the worst case is an acceptable tradeoff for the reduction (actually elimination in my experience) of those errors. This is even more true when you consider the time it takes to read and dismiss the error dialogs.
0386da9 to
1734cf5
Compare
leoetlino
approved these changes
Nov 26, 2020
This comment has been minimized.
This comment has been minimized.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On Windows, when the Rename function fails to replace an existing file
it will now retry the operation multiple times with increasingly long
delays between attempts. This fixes transient rename failures.
I've been getting sporadic yet annoyingly frequent errors saying:
'IOS_FS: Failed to rename temporary FST file'
These typically appear on startup but I've also gotten them randomly.
Investigation shows this happens when the Windows ReplaceFile function
returns the error ERROR_UNABLE_TO_REMOVE_REPLACED. That happens in the
context of using ReplaceFile to perform an atomic file overwrite, which
is required when saving updates to a file to avoid corruption. The
error mainly happens with the /Wii/fst.bin file but I've seen it
happen with multiple other files as well.
I haven't been able to definitively pin down why the error occurs,
though online discussions suggest antivirus scanning may be a major
culprit. That said, I've excluded the Dolphin folder from Windows
Defender scans to no avail and don't have any other antivirus running,
so this is likely to be a problem others are experiencing as well.
The number and duration of retry delays is arbitrary but I feel like a
combined second or so in the worst case is an acceptable tradeoff for
the reduction (actually elimination in my experience) of those errors.
This is even more true when you consider the time it takes to read and
dismiss the error dialogs.