Skip to content
Permalink
Browse files

filesys: safeWriteToFile(): Remove the target file before rename only…

… on Windows

Removing the target file on other platforms was enabled likely unintentionally
by commit 5f1f115.

This may be the reason why there has been corruption of files on Linux on hard
shutdowns.

Previously I described the problem and this fix in issue #3084.
  • Loading branch information
celeron55 committed Jan 1, 2016
1 parent 43c804a commit 64c060e1f2d94d8277246d8cdd8a886010564770
Showing with 10 additions and 1 deletion.
  1. +10 −1 src/filesys.cpp
@@ -693,13 +693,22 @@ bool safeWriteToFile(const std::string &path, const std::string &content)
os.flush();
os.close();
if (os.fail()) {
// Remove the temporary file because writing it failed and it's useless.
remove(tmp_file.c_str());
return false;
}

// Copy file
// Move the finished temporary file over the real file
#ifdef _WIN32
// On POSIX compliant systems rename() is specified to be able to swap the
// file in place of the destination file, making this a truly error-proof
// transaction.
// However, on Windows, the target file has to be removed first.
remove(path.c_str());
#endif
if(rename(tmp_file.c_str(), path.c_str())) {
// Remove the temporary file because moving it over the target file
// failed.
remove(tmp_file.c_str());
return false;
} else {

1 comment on commit 64c060e

@ShadowNinja

This comment has been minimized.

Copy link
Member

@ShadowNinja ShadowNinja commented on 64c060e Jan 2, 2016

ReplaceFile(path.c_str(), tmp_file.c_str(), NULL, 0, NULL, NULL) seems to be the "correct" way to do this on Windows. It isn't quite atomic either though. There's a transactional NTFS API, but that's only available on Vista and above and it's deprecated. A full combo approach would be something like this:

#ifdef _WIN32
#if WIN_VISTA_OR_HIGHER
    HANDLE trans = CreateTransaction(NULL, 0, 0, 0, 0, 0, "Safely replace file");
    if (trans == INVALID_HANDLE_VALUE) {
        goto generic_win_rename;
    }
    book ok = MoveFileTransacted(tmp_file.c_str(), path.c_str(), NULL,
            NULL, MOVEFILE_REPLACE_EXISTING, trans);
    CloseHandle(trans);
    if (ok)
        return true;
#endif
generic_win_rename:
    if (ReplaceFile(path.c_str(), tmp_file.c_str(), NULL, 0, NULL, NULL)) {
        return true;
    } else {
        DWORD e = GetLastError();
        if (e == ERROR_UNABLE_TO_REMOVE_REPLACED) {
            // Failed with no changes made to disk
            remove(tmp_file.c_str());
            return false;
        } else if (e == ERROR_UNABLE_TO_MOVE_REPLACEMENT) {
            errorstream << "Failed to replce " << path
                << ", file has been deleted, leaving updated version "
                << tmp_path << " on disk." << std::endl;
        } else if (e == ERROR_UNABLE_TO_MOVE_REPLACEMENT_2) {
            errorstream << "Failed to replce " << path
                << ", file still exists under a different name, "
                "leaving updated version "
                << tmp_path << " on disk." << std::endl;
        }
    }
#else
    // POSIX rename code
#endif
Please sign in to comment.
You can’t perform that action at this time.