From d5202f76ca71b85be3b6d05dafe4877544f1f06a Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 9 Oct 2026 21:09:48 -0500 Subject: [PATCH] fix(platform): write atomic files through a non-inheritable handle On Windows writeFileAtomically wrote the temp file through std::ofstream, whose CRT handle is inheritable. xmrig and the hidden helper commands (where, nvidia-smi, xmrig --version) are started with handle inheritance on, so one started during a save kept ".tmp" open without delete sharing, and every later replace of that file failed for as long as the child ran: hours while mining, with settings saves quietly not landing. The flush then reopened the file with share mode 0, so it was silently skipped whenever AV or the search indexer had the file open, and a replace refused by them just failed. - Windows: one CreateFileA handle, not inheritable, writes and flushes the temp file; the replace is retried up to 4 times (20+40+80+160 ms) on access-denied, sharing or lock violations, where it used to fail. - Linux/macOS: the temp file and directory fds are opened O_CLOEXEC, so a dragonxd or xmrig fork+exec doesn't inherit them. A later commit moves settings writes to a background thread, which makes every one of them concurrent with those spawns. The vault, the exports and DRAGONX.conf use the same function and get the same fix. Co-Authored-By: Claude Opus 5.5 --- src/util/platform.cpp | 52 ++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/src/util/platform.cpp b/src/util/platform.cpp index b177f65..7d4e70c 100644 --- a/src/util/platform.cpp +++ b/src/util/platform.cpp @@ -298,30 +298,50 @@ bool Platform::writeFileAtomically(const std::string& path, #ifdef _WIN32 { - std::ofstream out(tmp, std::ios::binary | std::ios::trunc); - if (!out.is_open()) return false; - out.write(content.data(), static_cast(content.size())); - out.flush(); - if (!out) { out.close(); DeleteFileA(tmp.c_str()); return false; } - } - // Flush the temp file's contents to stable storage before the rename. - HANDLE h = CreateFileA(tmp.c_str(), GENERIC_WRITE, 0, nullptr, - OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); - if (h != INVALID_HANDLE_VALUE) { - FlushFileBuffers(h); + // One handle for write + flush. CreateFileA with no SECURITY_ATTRIBUTES is NOT inheritable: the old + // std::ofstream (CRT) handle was, so a child spawned with bInheritHandles=TRUE at that moment (xmrig, + // runHiddenCapture) kept ".tmp" open without FILE_SHARE_DELETE and every later MoveFileEx + // failed while it ran. Also no second share-mode-0 open, which silently skipped the flush whenever + // AV / the indexer had the new file open. + HANDLE h = CreateFileA(tmp.c_str(), GENERIC_WRITE, 0, nullptr, + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); + if (h == INVALID_HANDLE_VALUE) return false; + bool ok = true; + size_t written = 0; + while (written < content.size()) { + const size_t left = content.size() - written; + const DWORD chunk = static_cast(left > (1u << 30) ? (1u << 30) : left); + DWORD n = 0; + if (!WriteFile(h, content.data() + written, chunk, &n, nullptr) || n == 0) { ok = false; break; } + written += n; + } + // Flush the temp file's contents to stable storage before the rename. + if (ok && !FlushFileBuffers(h)) ok = false; CloseHandle(h); + if (!ok) { DeleteFileA(tmp.c_str()); return false; } } (void)restrictPermissions; // owner-only ACLs rely on the per-user profile dir on Windows - // MoveFileEx with REPLACE_EXISTING is atomic on NTFS; WRITE_THROUGH makes it durable. - if (MoveFileExA(tmp.c_str(), path.c_str(), - MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH) == 0) { + // MoveFileEx with REPLACE_EXISTING is atomic on NTFS; WRITE_THROUGH makes it durable. AV / the search + // indexer can hold the destination briefly (ACCESS_DENIED / SHARING_VIOLATION / LOCK_VIOLATION): retry + // a few times (20+40+80+160 ms worst case, only on a write that used to just fail) instead of dropping it. + BOOL moved = FALSE; + DWORD delayMs = 20; + for (int attempt = 0; attempt < 5; ++attempt) { + moved = MoveFileExA(tmp.c_str(), path.c_str(), MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH); + if (moved) break; + const DWORD err = GetLastError(); + if (err != ERROR_ACCESS_DENIED && err != ERROR_SHARING_VIOLATION && err != ERROR_LOCK_VIOLATION) break; + if (attempt < 4) { Sleep(delayMs); delayMs *= 2; } + } + if (!moved) { DeleteFileA(tmp.c_str()); return false; } return true; #else const mode_t mode = restrictPermissions ? 0600 : 0644; - int fd = ::open(tmp.c_str(), O_WRONLY | O_CREAT | O_TRUNC, mode); + // O_CLOEXEC: the background settings writer can race a dragonxd/xmrig fork+exec; don't leak this fd. + int fd = ::open(tmp.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, mode); if (fd < 0) return false; if (restrictPermissions) { ::fchmod(fd, 0600); // enforce even if the temp file pre-existed with looser perms @@ -350,7 +370,7 @@ bool Platform::writeFileAtomically(const std::string& path, } // fsync the directory so the rename (the metadata that makes the new file // visible) survives a crash, not just the file contents. - int dfd = ::open(dir.empty() ? "." : dir.c_str(), O_RDONLY); + int dfd = ::open(dir.empty() ? "." : dir.c_str(), O_RDONLY | O_CLOEXEC); if (dfd >= 0) { ::fsync(dfd); ::close(dfd);