mirror of
https://github.com/spice2x/spice2x.github.io.git
synced 2026-08-01 14:20:42 -07:00
cfg: fix config save atomicity (#760)
## Link to GitHub Issue or related Pull Request, if one exists n/a ## Description of change Fully flush `tmp` file and then use `MoveFileExW` instead of `CopyFileW` which guarantees atomic operation on NTFS. ## Testing
This commit is contained in:
@@ -1331,17 +1331,32 @@ bool Config::firstFillConfigFile() {
|
||||
}
|
||||
|
||||
void Config::saveConfigFile() {
|
||||
// create a .tmp file and write to it...
|
||||
// write the new config to a .tmp file first...
|
||||
const auto xml_result = this->configFile.SaveFile(this->configLocationTemp.c_str(), false);
|
||||
if (xml_result != tinyxml2::XMLError::XML_SUCCESS) {
|
||||
log_info("cfg", "failed to write file: {}", this->configLocationTemp);
|
||||
return;
|
||||
}
|
||||
// copy the .tmp file to the main file...
|
||||
if (CopyFileW(this->configLocationTemp.c_str(), this->configLocation.c_str(), false) == 0) {
|
||||
log_warning("cfg", "CopyFileA failed: 0x{:08x}", GetLastError());
|
||||
|
||||
// ...flush the .tmp file to disk so a crash/power loss can't leave it half-written...
|
||||
HANDLE tmp_handle = CreateFileW(
|
||||
this->configLocationTemp.c_str(),
|
||||
GENERIC_WRITE, FILE_SHARE_READ, nullptr,
|
||||
OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
|
||||
if (tmp_handle != INVALID_HANDLE_VALUE) {
|
||||
FlushFileBuffers(tmp_handle);
|
||||
CloseHandle(tmp_handle);
|
||||
}
|
||||
|
||||
// ...then atomically replace the real config with the .tmp file.
|
||||
// Unlike CopyFile (which truncates and rewrites the destination in place),
|
||||
// an NTFS rename is atomic: the existing config is never left half-overwritten,
|
||||
// so an interrupted save can't corrupt it. MoveFileEx also removes the .tmp on success.
|
||||
if (MoveFileExW(
|
||||
this->configLocationTemp.c_str(),
|
||||
this->configLocation.c_str(),
|
||||
MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH) == 0) {
|
||||
log_warning("cfg", "MoveFileExW failed: 0x{:08x}", GetLastError());
|
||||
return;
|
||||
}
|
||||
// delete the .tmp file (not critical if this fails)
|
||||
DeleteFileW(this->configLocationTemp.c_str());
|
||||
}
|
||||
|
||||
@@ -359,13 +359,12 @@ namespace overlay::windows {
|
||||
return false;
|
||||
}
|
||||
|
||||
std::error_code ec;
|
||||
std::filesystem::rename(path_tmp, path, ec);
|
||||
if (ec) {
|
||||
log_warning("templates", "failed to rename templates file: {}", ec.message());
|
||||
if (MoveFileExW(path_tmp.c_str(), path.c_str(), MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH) == 0) {
|
||||
log_warning("templates", "failed to rename templates file: 0x{:08x}", GetLastError());
|
||||
return false;
|
||||
}
|
||||
|
||||
log_info("templates", "templates file saved successfully");
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -397,8 +396,7 @@ namespace overlay::windows {
|
||||
auto path_tmp = path;
|
||||
path_tmp.replace_extension(L"tmp");
|
||||
if (doc.SaveFile(path_tmp.c_str()) == tinyxml2::XML_SUCCESS) {
|
||||
std::error_code ec;
|
||||
std::filesystem::rename(path_tmp, path, ec);
|
||||
MoveFileExW(path_tmp.c_str(), path.c_str(), MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -434,8 +432,7 @@ namespace overlay::windows {
|
||||
auto path_tmp = path;
|
||||
path_tmp.replace_extension(L"tmp");
|
||||
if (doc.SaveFile(path_tmp.c_str()) == tinyxml2::XML_SUCCESS) {
|
||||
std::error_code ec;
|
||||
std::filesystem::rename(path_tmp, path, ec);
|
||||
MoveFileExW(path_tmp.c_str(), path.c_str(), MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -536,7 +536,7 @@ namespace overlay::windows {
|
||||
}
|
||||
|
||||
void CardManager::config_save() {
|
||||
log_info("cardmanager", "saving config");
|
||||
log_misc("cardmanager", "saving config...");
|
||||
|
||||
// create document
|
||||
Document doc;
|
||||
|
||||
@@ -298,5 +298,19 @@ bool fileutils::write_config_file(const std::string_view &module, const std::fil
|
||||
|
||||
// save file
|
||||
log_info(module, "saving config file: {}", censored_display);
|
||||
return fileutils::text_write(path, text);
|
||||
|
||||
// write to a .tmp file first, then atomically replace the real config — this way
|
||||
// a crash or disk-full mid-write can't corrupt the existing config file.
|
||||
auto path_tmp = path;
|
||||
path_tmp += L".tmp";
|
||||
if (!fileutils::text_write(path_tmp, text)) {
|
||||
log_warning(module, "failed to write temp config file: {}", censored_display);
|
||||
return false;
|
||||
}
|
||||
if (MoveFileExW(path_tmp.c_str(), path.c_str(), MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH) == 0) {
|
||||
log_warning(module, "MoveFileExW failed: 0x{:08x}", GetLastError());
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user