Fix file rename errors on Windows

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.
This commit is contained in:
Dentomologist 2020-11-14 16:49:25 -08:00
parent 8a621c2d5e
commit 1734cf55d8

View File

@ -3,6 +3,7 @@
// Refer to the license.txt file included.
#include <algorithm>
#include <chrono>
#include <cstddef>
#include <cstdio>
#include <cstring>
@ -11,6 +12,7 @@
#include <limits.h>
#include <string>
#include <sys/stat.h>
#include <thread>
#include <vector>
#include "Common/Assert.h"
@ -276,33 +278,65 @@ bool DeleteDir(const std::string& filename)
return false;
}
// Repeatedly invokes func until it returns true or max_attempts failures.
// Waits after each failure, with each delay doubling in length.
template <typename FuncType>
static bool AttemptMaxTimesWithExponentialDelay(int max_attempts, std::chrono::milliseconds delay,
std::string_view func_name, const FuncType& func)
{
for (int failed_attempts = 0; failed_attempts < max_attempts; ++failed_attempts)
{
if (func())
{
return true;
}
if (failed_attempts + 1 < max_attempts)
{
INFO_LOG_FMT(COMMON, "{} attempt failed, delaying for {} milliseconds", func_name,
delay.count());
std::this_thread::sleep_for(delay);
delay *= 2;
}
}
return false;
}
// renames file srcFilename to destFilename, returns true on success
bool Rename(const std::string& srcFilename, const std::string& destFilename)
{
INFO_LOG_FMT(COMMON, "Rename: {} --> {}", srcFilename, destFilename);
#ifdef _WIN32
auto sf = UTF8ToTStr(srcFilename);
auto df = UTF8ToTStr(destFilename);
// The Internet seems torn about whether ReplaceFile is atomic or not.
// Hopefully it's atomic enough...
if (ReplaceFile(df.c_str(), sf.c_str(), nullptr, REPLACEFILE_IGNORE_MERGE_ERRORS, nullptr,
nullptr))
const std::wstring source_wstring = UTF8ToTStr(srcFilename);
const std::wstring destination_wstring = UTF8ToTStr(destFilename);
// On Windows ReplaceFile can fail spuriously due to antivirus checking or other noise.
// Retry the operation with increasing delays, and if none of them work there's probably a
// persistent problem.
const bool success = AttemptMaxTimesWithExponentialDelay(
3, std::chrono::milliseconds(5), "Rename", [&source_wstring, &destination_wstring] {
if (ReplaceFile(destination_wstring.c_str(), source_wstring.c_str(), nullptr,
REPLACEFILE_IGNORE_MERGE_ERRORS, nullptr, nullptr))
{
return true;
}
// Might have failed because the destination doesn't exist.
if (GetLastError() == ERROR_FILE_NOT_FOUND)
{
if (MoveFile(sf.c_str(), df.c_str()))
return true;
return MoveFile(source_wstring.c_str(), destination_wstring.c_str()) != 0;
}
ERROR_LOG_FMT(COMMON, "Rename: MoveFile failed on {} --> {}: {}", srcFilename, destFilename,
GetLastErrorString());
#else
if (rename(srcFilename.c_str(), destFilename.c_str()) == 0)
return true;
ERROR_LOG_FMT(COMMON, "Rename: rename failed on {} --> {}: {}", srcFilename, destFilename,
LastStrerrorString());
#endif
return false;
});
constexpr auto error_string_func = GetLastErrorString;
#else
const bool success = rename(srcFilename.c_str(), destFilename.c_str()) == 0;
constexpr auto error_string_func = LastStrerrorString;
#endif
if (!success)
{
ERROR_LOG_FMT(COMMON, "Rename: rename failed on {} --> {}: {}", srcFilename, destFilename,
error_string_func());
}
return success;
}
#ifndef _WIN32