diff --git a/sdk_v2/cpp/CMakeLists.txt b/sdk_v2/cpp/CMakeLists.txt index f4a3c2dd5..ff606ed41 100644 --- a/sdk_v2/cpp/CMakeLists.txt +++ b/sdk_v2/cpp/CMakeLists.txt @@ -102,11 +102,15 @@ if(WIN32) list(APPEND FOUNDRY_LOCAL_PLATFORM_SOURCES src/util/stacktrace_windows.cc src/platform/windows/path.cc + src/platform/windows/file_io.cc + src/platform/windows/cross_process_file_lock.cc ) else() list(APPEND FOUNDRY_LOCAL_PLATFORM_SOURCES src/util/stacktrace_posix.cc src/platform/posix/path.cc + src/platform/posix/file_io.cc + src/platform/posix/cross_process_file_lock.cc ) endif() diff --git a/sdk_v2/cpp/src/download/cross_process_file_lock.cc b/sdk_v2/cpp/src/download/cross_process_file_lock.cc index 5aa82c769..9ea5d3b87 100644 --- a/sdk_v2/cpp/src/download/cross_process_file_lock.cc +++ b/sdk_v2/cpp/src/download/cross_process_file_lock.cc @@ -1,199 +1,20 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// Cross-platform orchestration for CrossProcessFileLock. The platform-specific +// pieces — the lock handle (State) and its releasing destructor, +// FormatProcessInfo, the CrossProcessFileLock destructor/constructor, and +// TryAcquireForDirectory — live in +// src/platform/{windows,posix}/cross_process_file_lock.cc. #include "download/cross_process_file_lock.h" #include "exception.h" -#include "logger.h" #include #include -#include -#include -#include -#include #include -#ifdef _WIN32 -#define WIN32_LEAN_AND_MEAN -#define NOMINMAX -#include -#include -#else -#include -#include -#include -#include -#include -#endif - namespace fl { -namespace { - -constexpr const char* kLockFileName = ".download.lock"; - -/// `PID:,Time:\n` -std::string FormatProcessInfo() { -#ifdef _WIN32 - auto pid = static_cast(_getpid()); -#else - auto pid = static_cast(getpid()); -#endif - auto t = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now()); - std::tm tm{}; -#ifdef _WIN32 - gmtime_s(&tm, &t); -#else - gmtime_r(&t, &tm); -#endif - std::ostringstream oss; - oss << "PID:" << pid << ",Time:" << std::put_time(&tm, "%Y-%m-%dT%H:%M:%SZ") << '\n'; - return oss.str(); -} - -} // namespace - -// Platform-specific resource handle. The destructor here is the only thing -// that releases the lock; CrossProcessFileLock's destructor is defaulted. -#ifdef _WIN32 -struct CrossProcessFileLock::State { - HANDLE handle; - ~State() { - if (handle != INVALID_HANDLE_VALUE) { - // FILE_FLAG_DELETE_ON_CLOSE removes the file when the last handle closes. - CloseHandle(handle); - } - } -}; -#else -struct CrossProcessFileLock::State { - int fd; - std::filesystem::path path; - ~State() { - if (fd >= 0) { - // Unlink before close so the file disappears the instant the lock - // releases; a concurrent acquirer simply recreates it. This is the - // classic flock()+unlink() pattern, and it is safe here because every - // acquirer verifies, while holding the flock, that the inode it locked is - // still the one at `path` (see the fstat/stat check in - // TryAcquireForDirectory). An acquirer that raced in on the old inode - // between our unlink and a third party's recreate will see the inode - // mismatch and retry, so two processes never hold "the lock" at once. - // There is also no protected work between this unlink and close. - ::unlink(path.c_str()); - ::close(fd); - } - } -}; -#endif - -CrossProcessFileLock::CrossProcessFileLock(std::filesystem::path path, - std::unique_ptr state, - ILogger& logger) - : path_(std::move(path)), state_(std::move(state)), logger_(logger) {} - -CrossProcessFileLock::~CrossProcessFileLock() { - // Release the OS handle first so the "released" log message is accurate. - state_.reset(); - logger_.Log(LogLevel::Debug, "CrossProcessFileLock released: " + path_.string()); -} - -std::unique_ptr CrossProcessFileLock::TryAcquireForDirectory( - const std::filesystem::path& directory, ILogger& logger) { - std::error_code ec; - std::filesystem::create_directories(directory, ec); - // Best-effort: if create_directories failed, the platform open below will - // surface a clearer error message. - - auto lock_path = directory / kLockFileName; - std::unique_ptr state; - -#ifdef _WIN32 - // dwShareMode=0 blocks any other open (cross- and in-process) until this - // handle closes. FILE_FLAG_DELETE_ON_CLOSE pairs OPEN_ALWAYS into a - // self-cleaning lock that doesn't require unlink-then-close races. - auto wide = lock_path.wstring(); - HANDLE handle = CreateFileW(wide.c_str(), - GENERIC_READ | GENERIC_WRITE, - 0, - nullptr, - OPEN_ALWAYS, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, - nullptr); - if (handle == INVALID_HANDLE_VALUE) { - DWORD err = GetLastError(); - if (err == ERROR_SHARING_VIOLATION || err == ERROR_LOCK_VIOLATION || err == ERROR_ACCESS_DENIED) { - // SHARING/LOCK_VIOLATION: another handle already holds the share-none - // lock. ACCESS_DENIED: the holder is mid-release — FILE_FLAG_DELETE_ON_CLOSE - // puts the file into STATUS_DELETE_PENDING during the close window, and a - // concurrent open of a delete-pending file is reported as ACCESS_DENIED. - // All three mean "another process has it"; treat as contention so the - // caller retries. (A genuine permission error also lands here and would - // poll until timeout, but the directory was just created successfully so - // that is improbable.) - return nullptr; - } - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "CreateFileW failed for lock '" + lock_path.string() + - "' (GetLastError=" + std::to_string(err) + ")"); - } - - auto info = FormatProcessInfo(); - DWORD written = 0; - WriteFile(handle, info.data(), static_cast(info.size()), &written, nullptr); - FlushFileBuffers(handle); - - state = std::unique_ptr(new State{handle}); -#else - int fd = ::open(lock_path.c_str(), O_CREAT | O_RDWR | O_CLOEXEC, 0644); - if (fd < 0) { - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "open failed for lock '" + lock_path.string() + "' (errno=" + std::to_string(errno) + ")"); - } - if (::flock(fd, LOCK_EX | LOCK_NB) != 0) { - int err = errno; - ::close(fd); - if (err == EWOULDBLOCK || err == EAGAIN) { - return nullptr; - } - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "flock failed for '" + lock_path.string() + "' (errno=" + std::to_string(err) + ")"); - } - - // Robust-flock inode check. We now hold an exclusive flock on whatever inode - // `fd` refers to, but a releaser unlink()s the lock file in its destructor — - // so between our open() and flock() the path may have been unlinked and a - // third process may have recreated it. If so, we are holding a lock on an - // orphaned inode that guards nothing while the live file at `lock_path` is a - // different inode. Confirm the inode we locked is still the one at the path; - // if not, drop it and report contention so the caller retries against the - // live file. This closes the flock()+unlink() orphan-inode race, which is - // what lets two processes never both believe they hold the lock. - struct stat fd_stat {}; - struct stat path_stat {}; - if (::fstat(fd, &fd_stat) != 0 || ::stat(lock_path.c_str(), &path_stat) != 0 || - fd_stat.st_dev != path_stat.st_dev || fd_stat.st_ino != path_stat.st_ino) { - ::close(fd); // releases the flock on the stale / orphaned inode - return nullptr; - } - - auto info = FormatProcessInfo(); - // Best-effort: record this process's identity in the lock file for diagnostics. - // A failure here doesn't affect lock correctness, so it is only logged at Debug. - if (::ftruncate(fd, 0) != 0 || ::write(fd, info.data(), info.size()) < 0) { - logger.Log(LogLevel::Debug, - "CrossProcessFileLock: failed to write diagnostic process info to lock file '" + - lock_path.string() + "' (errno=" + std::to_string(errno) + ")"); - } - - state = std::unique_ptr(new State{fd, lock_path}); -#endif - - logger.Log(LogLevel::Debug, "CrossProcessFileLock acquired: " + lock_path.string()); - return std::unique_ptr( - new CrossProcessFileLock(std::move(lock_path), std::move(state), logger)); -} - std::unique_ptr CrossProcessFileLock::WaitForDirectoryLock( const std::filesystem::path& directory, const CancellationPredicate& is_cancelled, diff --git a/sdk_v2/cpp/src/download/file_writer.cc b/sdk_v2/cpp/src/download/file_writer.cc index 6ffb6dd5e..cdde85d24 100644 --- a/sdk_v2/cpp/src/download/file_writer.cc +++ b/sdk_v2/cpp/src/download/file_writer.cc @@ -3,6 +3,7 @@ #include "download/file_writer.h" #include "exception.h" #include "logger.h" +#include "platform/file_io.h" #include @@ -10,18 +11,6 @@ #include #include -#ifdef _WIN32 -#ifndef NOMINMAX -#define NOMINMAX -#endif -#include -#else -#include -#include -#include -#include -#endif - namespace fl { namespace fs = std::filesystem; @@ -69,112 +58,33 @@ void EnsureFileExistsAtSize(const fs::path& path, int64_t expected_size) { FileWriter::FileWriter(ILogger& logger) : logger_(logger) {} -#ifdef _WIN32 - FileWriter::~FileWriter() { Close(); } void FileWriter::Open(const fs::path& path, int64_t expected_size) { EnsureFileExistsAtSize(path, expected_size); - // FILE_SHARE_READ | FILE_SHARE_WRITE so the lock file / other tools can peek - // at the partial file without us erroring; positional WriteFile is safe - // regardless of share mode. - HANDLE h = ::CreateFileW(path.wstring().c_str(), GENERIC_READ | GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, nullptr); - if (h == INVALID_HANDLE_VALUE) { + std::string error; + handle_ = platform::OpenWritableFile(path, error); + if (handle_ == platform::kInvalidFileHandle) { FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "FileWriter open failed for " + path.string() + " (Win32 err " + - std::to_string(::GetLastError()) + ")"); + "FileWriter open failed for " + path.string() + " (" + error + ")"); } - handle_ = h; } void FileWriter::WriteAt(int64_t offset, const uint8_t* data, size_t len) { - // Concurrent WriteFile calls with distinct OVERLAPPED offsets on the same - // handle are safe for non-overlapping ranges; the kernel orders them. - while (len > 0) { - OVERLAPPED ov{}; - // Split the 64-bit file offset across the OVERLAPPED halves: the DWORD casts - // keep the low 32 bits in Offset and the high 32 bits in OffsetHigh. - ov.Offset = static_cast(static_cast(offset)); - ov.OffsetHigh = static_cast(static_cast(offset) >> 32); - DWORD to_write = static_cast(len > 0x7FFFFFFFu ? 0x7FFFFFFFu : len); - DWORD written = 0; - if (!::WriteFile(handle_, data, to_write, &written, &ov)) { - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "FileWriter write failed at offset " + std::to_string(offset) + " (Win32 err " + - std::to_string(::GetLastError()) + ")"); - } - if (written == 0) { - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "FileWriter short write at offset " + std::to_string(offset)); - } - offset += static_cast(written); - data += written; - len -= written; + std::string error; + if (!platform::WriteFileAt(handle_, offset, data, len, error)) { + FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, "FileWriter " + error); } } void FileWriter::Close() { - if (handle_ != nullptr) { - if (!::CloseHandle(handle_)) { - const DWORD err = ::GetLastError(); - logger_.Log(LogLevel::Warning, - "FileWriter: CloseHandle failed (Win32 err " + std::to_string(err) + ")"); + if (handle_ != platform::kInvalidFileHandle) { + std::string error; + if (!platform::CloseFile(handle_, error)) { + logger_.Log(LogLevel::Warning, "FileWriter: " + error); } - handle_ = nullptr; - } -} - -#else // POSIX - -FileWriter::~FileWriter() { Close(); } - -void FileWriter::Open(const fs::path& path, int64_t expected_size) { - EnsureFileExistsAtSize(path, expected_size); - fd_ = ::open(path.c_str(), O_RDWR | O_CLOEXEC); - if (fd_ < 0) { - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "FileWriter open failed for " + path.string() + " (errno " + - std::to_string(errno) + ")"); + handle_ = platform::kInvalidFileHandle; } } -void FileWriter::WriteAt(int64_t offset, const uint8_t* data, size_t len) { - while (len > 0) { - ssize_t n = ::pwrite(fd_, data, len, static_cast(offset)); - if (n < 0) { - if (errno == EINTR) continue; - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "FileWriter pwrite failed at offset " + std::to_string(offset) + " (errno " + - std::to_string(errno) + ")"); - } - if (n == 0) { - FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, - "FileWriter short pwrite at offset " + std::to_string(offset)); - } - offset += n; - data += n; - len -= static_cast(n); - } -} - -void FileWriter::Close() { - if (fd_ >= 0) { - // A failing close() can surface a deferred write error (e.g. EIO, or ENOSPC - // on delayed allocation / a networked filesystem), so the file may be - // incomplete even though every pwrite returned success. Log it for - // diagnosis. Don't retry: on Linux the descriptor is freed even when close() - // returns EINTR, so a retry could close an unrelated, since-reused fd. - if (::close(fd_) != 0) { - const int err = errno; - logger_.Log(LogLevel::Warning, - "FileWriter: close failed (errno " + std::to_string(err) + ")"); - } - fd_ = -1; - } -} - -#endif - } // namespace fl diff --git a/sdk_v2/cpp/src/download/file_writer.h b/sdk_v2/cpp/src/download/file_writer.h index 29540e2aa..2b63feb7f 100644 --- a/sdk_v2/cpp/src/download/file_writer.h +++ b/sdk_v2/cpp/src/download/file_writer.h @@ -15,7 +15,8 @@ class ILogger; /// Workers in a single download claim disjoint chunks, so concurrent `WriteAt` /// calls always target non-overlapping byte ranges. Backed by `pwrite` (POSIX) /// or `WriteFile` + `OVERLAPPED` (Windows): the OS arbitrates concurrent writes -/// to disjoint ranges, so no user-space lock is taken. +/// to disjoint ranges, so no user-space lock is taken. The OS-specific calls +/// live in `src/platform/file_io.*`. class FileWriter { public: explicit FileWriter(ILogger& logger); @@ -38,12 +39,9 @@ class FileWriter { private: ILogger& logger_; -#ifdef _WIN32 - // Win32 HANDLE. Holds a valid handle while open, nullptr otherwise. - void* handle_ = nullptr; -#else - int fd_ = -1; -#endif + // Native file handle (Win32 HANDLE or POSIX fd) as an integer; see + // src/platform/file_io.h. kInvalidFileHandle (-1) when not open. + std::intptr_t handle_ = -1; }; } // namespace fl diff --git a/sdk_v2/cpp/src/platform/file_io.h b/sdk_v2/cpp/src/platform/file_io.h new file mode 100644 index 000000000..49e7d89e8 --- /dev/null +++ b/sdk_v2/cpp/src/platform/file_io.h @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +#pragma once + +#include +#include +#include +#include + +namespace fl::platform { + +/// Native file handle (a Win32 HANDLE or a POSIX file descriptor) carried as an +/// integer so callers don't depend on platform headers. `kInvalidFileHandle` +/// matches both INVALID_HANDLE_VALUE ((HANDLE)-1) and a POSIX fd of -1. +inline constexpr std::intptr_t kInvalidFileHandle = -1; + +/// Open an existing file for positional read/write. Returns a handle that is not +/// `kInvalidFileHandle` on success; on failure returns `kInvalidFileHandle` and +/// sets `error` to a human-readable OS error (e.g. "Win32 err 5" / "errno 13"). +std::intptr_t OpenWritableFile(const std::filesystem::path& path, std::string& error); + +/// Write all `len` bytes from `data` at byte offset `offset`. Safe for concurrent +/// calls on the same handle targeting disjoint ranges. Returns true on success; +/// on failure returns false and sets `error` (e.g. "pwrite failed at offset N +/// (errno M)"). +bool WriteFileAt(std::intptr_t handle, std::int64_t offset, const std::uint8_t* data, std::size_t len, + std::string& error); + +/// Close `handle`. Returns true on success; on failure returns false and sets +/// `error`. A failing close can surface a deferred write error, so the data may +/// be incomplete even when every write succeeded. +bool CloseFile(std::intptr_t handle, std::string& error); + +} // namespace fl::platform diff --git a/sdk_v2/cpp/src/platform/posix/cross_process_file_lock.cc b/sdk_v2/cpp/src/platform/posix/cross_process_file_lock.cc new file mode 100644 index 000000000..443bbc9ee --- /dev/null +++ b/sdk_v2/cpp/src/platform/posix/cross_process_file_lock.cc @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +// POSIX implementation of CrossProcessFileLock acquisition and the +// platform-specific lock handle. Cross-platform orchestration +// (WaitForDirectoryLock) lives in src/download/cross_process_file_lock.cc. +#include "download/cross_process_file_lock.h" +#include "exception.h" +#include "logger.h" + +#include + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace fl { + +namespace { + +constexpr const char* kLockFileName = ".download.lock"; + +/// `PID:,Time:\n` +std::string FormatProcessInfo() { + auto pid = static_cast(getpid()); + auto t = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now()); + std::tm tm{}; + gmtime_r(&t, &tm); + std::ostringstream oss; + oss << "PID:" << pid << ",Time:" << std::put_time(&tm, "%Y-%m-%dT%H:%M:%SZ") << '\n'; + return oss.str(); +} + +} // namespace + +// Platform-specific resource handle. The destructor here is the only thing +// that releases the lock; CrossProcessFileLock's destructor is defaulted. +struct CrossProcessFileLock::State { + int fd; + std::filesystem::path path; + ~State() { + if (fd >= 0) { + // Unlink before close so the file disappears the instant the lock + // releases; a concurrent acquirer simply recreates it. This is the + // classic flock()+unlink() pattern, and it is safe here because every + // acquirer verifies, while holding the flock, that the inode it locked is + // still the one at `path` (see the fstat/stat check in + // TryAcquireForDirectory). An acquirer that raced in on the old inode + // between our unlink and a third party's recreate will see the inode + // mismatch and retry, so two processes never hold "the lock" at once. + // There is also no protected work between this unlink and close. + ::unlink(path.c_str()); + ::close(fd); + } + } +}; + +CrossProcessFileLock::CrossProcessFileLock(std::filesystem::path path, + std::unique_ptr state, + ILogger& logger) + : path_(std::move(path)), state_(std::move(state)), logger_(logger) {} + +CrossProcessFileLock::~CrossProcessFileLock() { + // Release the OS handle first so the "released" log message is accurate. + state_.reset(); + logger_.Log(LogLevel::Debug, "CrossProcessFileLock released: " + path_.string()); +} + +std::unique_ptr CrossProcessFileLock::TryAcquireForDirectory( + const std::filesystem::path& directory, ILogger& logger) { + std::error_code ec; + std::filesystem::create_directories(directory, ec); + // Best-effort: if create_directories failed, the platform open below will + // surface a clearer error message. + + auto lock_path = directory / kLockFileName; + std::unique_ptr state; + + int fd = ::open(lock_path.c_str(), O_CREAT | O_RDWR | O_CLOEXEC, 0644); + if (fd < 0) { + FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, + "open failed for lock '" + lock_path.string() + "' (errno=" + std::to_string(errno) + ")"); + } + if (::flock(fd, LOCK_EX | LOCK_NB) != 0) { + int err = errno; + ::close(fd); + if (err == EWOULDBLOCK || err == EAGAIN) { + return nullptr; + } + FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, + "flock failed for '" + lock_path.string() + "' (errno=" + std::to_string(err) + ")"); + } + + // Robust-flock inode check. We now hold an exclusive flock on whatever inode + // `fd` refers to, but a releaser unlink()s the lock file in its destructor — + // so between our open() and flock() the path may have been unlinked and a + // third process may have recreated it. If so, we are holding a lock on an + // orphaned inode that guards nothing while the live file at `lock_path` is a + // different inode. Confirm the inode we locked is still the one at the path; + // if not, drop it and report contention so the caller retries against the + // live file. This closes the flock()+unlink() orphan-inode race, which is + // what lets two processes never both believe they hold the lock. + struct stat fd_stat {}; + struct stat path_stat {}; + if (::fstat(fd, &fd_stat) != 0 || ::stat(lock_path.c_str(), &path_stat) != 0 || + fd_stat.st_dev != path_stat.st_dev || fd_stat.st_ino != path_stat.st_ino) { + ::close(fd); // releases the flock on the stale / orphaned inode + return nullptr; + } + + auto info = FormatProcessInfo(); + // Best-effort: record this process's identity in the lock file for diagnostics. + // A failure here doesn't affect lock correctness, so it is only logged at Debug. + if (::ftruncate(fd, 0) != 0 || ::write(fd, info.data(), info.size()) < 0) { + logger.Log(LogLevel::Debug, + "CrossProcessFileLock: failed to write diagnostic process info to lock file '" + + lock_path.string() + "' (errno=" + std::to_string(errno) + ")"); + } + + state = std::unique_ptr(new State{fd, lock_path}); + + logger.Log(LogLevel::Debug, "CrossProcessFileLock acquired: " + lock_path.string()); + return std::unique_ptr( + new CrossProcessFileLock(std::move(lock_path), std::move(state), logger)); +} + +} // namespace fl diff --git a/sdk_v2/cpp/src/platform/posix/file_io.cc b/sdk_v2/cpp/src/platform/posix/file_io.cc new file mode 100644 index 000000000..e6c8c2814 --- /dev/null +++ b/sdk_v2/cpp/src/platform/posix/file_io.cc @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +#include "platform/file_io.h" + +#include +#include +#include +#include +#include + +namespace fl::platform { + +std::intptr_t OpenWritableFile(const std::filesystem::path& path, std::string& error) { + int fd = ::open(path.c_str(), O_RDWR | O_CLOEXEC); + if (fd < 0) { + error = "errno " + std::to_string(errno); + return kInvalidFileHandle; + } + return static_cast(fd); +} + +bool WriteFileAt(std::intptr_t handle, std::int64_t offset, const std::uint8_t* data, std::size_t len, + std::string& error) { + int fd = static_cast(handle); + while (len > 0) { + ssize_t n = ::pwrite(fd, data, len, static_cast(offset)); + if (n < 0) { + if (errno == EINTR) continue; + error = "pwrite failed at offset " + std::to_string(offset) + " (errno " + + std::to_string(errno) + ")"; + return false; + } + if (n == 0) { + error = "short pwrite at offset " + std::to_string(offset); + return false; + } + offset += n; + data += n; + len -= static_cast(n); + } + return true; +} + +bool CloseFile(std::intptr_t handle, std::string& error) { + int fd = static_cast(handle); + // A failing close() can surface a deferred write error (e.g. EIO, or ENOSPC on + // delayed allocation / a networked filesystem), so the file may be incomplete + // even though every pwrite returned success. Don't retry: on Linux the + // descriptor is freed even when close() returns EINTR, so a retry could close + // an unrelated, since-reused fd. + if (::close(fd) != 0) { + error = "close failed (errno " + std::to_string(errno) + ")"; + return false; + } + return true; +} + +} // namespace fl::platform diff --git a/sdk_v2/cpp/src/platform/windows/cross_process_file_lock.cc b/sdk_v2/cpp/src/platform/windows/cross_process_file_lock.cc new file mode 100644 index 000000000..c80fcc926 --- /dev/null +++ b/sdk_v2/cpp/src/platform/windows/cross_process_file_lock.cc @@ -0,0 +1,116 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +// Windows implementation of CrossProcessFileLock acquisition and the +// platform-specific lock handle. Cross-platform orchestration +// (WaitForDirectoryLock) lives in src/download/cross_process_file_lock.cc. +#include "download/cross_process_file_lock.h" +#include "exception.h" +#include "logger.h" + +#include + +#include +#include +#include +#include +#include + +#define WIN32_LEAN_AND_MEAN +#define NOMINMAX +#include +#include + +namespace fl { + +namespace { + +constexpr const char* kLockFileName = ".download.lock"; + +/// `PID:,Time:\n` +std::string FormatProcessInfo() { + auto pid = static_cast(_getpid()); + auto t = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now()); + std::tm tm{}; + gmtime_s(&tm, &t); + std::ostringstream oss; + oss << "PID:" << pid << ",Time:" << std::put_time(&tm, "%Y-%m-%dT%H:%M:%SZ") << '\n'; + return oss.str(); +} + +} // namespace + +// Platform-specific resource handle. The destructor here is the only thing +// that releases the lock; CrossProcessFileLock's destructor is defaulted. +struct CrossProcessFileLock::State { + HANDLE handle; + ~State() { + if (handle != INVALID_HANDLE_VALUE) { + // FILE_FLAG_DELETE_ON_CLOSE removes the file when the last handle closes. + CloseHandle(handle); + } + } +}; + +CrossProcessFileLock::CrossProcessFileLock(std::filesystem::path path, + std::unique_ptr state, + ILogger& logger) + : path_(std::move(path)), state_(std::move(state)), logger_(logger) {} + +CrossProcessFileLock::~CrossProcessFileLock() { + // Release the OS handle first so the "released" log message is accurate. + state_.reset(); + logger_.Log(LogLevel::Debug, "CrossProcessFileLock released: " + path_.string()); +} + +std::unique_ptr CrossProcessFileLock::TryAcquireForDirectory( + const std::filesystem::path& directory, ILogger& logger) { + std::error_code ec; + std::filesystem::create_directories(directory, ec); + // Best-effort: if create_directories failed, the platform open below will + // surface a clearer error message. + + auto lock_path = directory / kLockFileName; + std::unique_ptr state; + + // dwShareMode=0 blocks any other open (cross- and in-process) until this + // handle closes. FILE_FLAG_DELETE_ON_CLOSE pairs OPEN_ALWAYS into a + // self-cleaning lock that doesn't require unlink-then-close races. + auto wide = lock_path.wstring(); + HANDLE handle = CreateFileW(wide.c_str(), + GENERIC_READ | GENERIC_WRITE, + 0, + nullptr, + OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, + nullptr); + if (handle == INVALID_HANDLE_VALUE) { + DWORD err = GetLastError(); + if (err == ERROR_SHARING_VIOLATION || err == ERROR_LOCK_VIOLATION || err == ERROR_ACCESS_DENIED) { + // SHARING/LOCK_VIOLATION: another handle already holds the share-none + // lock. ACCESS_DENIED: the holder is mid-release — FILE_FLAG_DELETE_ON_CLOSE + // puts the file into STATUS_DELETE_PENDING during the close window, and a + // concurrent open of a delete-pending file is reported as ACCESS_DENIED. + // All three mean "another process has it"; treat as contention so the + // caller retries. (A genuine permission error also lands here and would + // poll until timeout, but the directory was just created successfully so + // that is improbable.) + return nullptr; + } + FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL, + "CreateFileW failed for lock '" + lock_path.string() + + "' (GetLastError=" + std::to_string(err) + ")"); + } + + auto info = FormatProcessInfo(); + DWORD written = 0; + WriteFile(handle, info.data(), static_cast(info.size()), &written, nullptr); + FlushFileBuffers(handle); + + state = std::unique_ptr(new State{handle}); + + logger.Log(LogLevel::Debug, "CrossProcessFileLock acquired: " + lock_path.string()); + return std::unique_ptr( + new CrossProcessFileLock(std::move(lock_path), std::move(state), logger)); +} + +} // namespace fl diff --git a/sdk_v2/cpp/src/platform/windows/file_io.cc b/sdk_v2/cpp/src/platform/windows/file_io.cc new file mode 100644 index 000000000..597ce6ada --- /dev/null +++ b/sdk_v2/cpp/src/platform/windows/file_io.cc @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +#include "platform/file_io.h" + +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include + +#include + +namespace fl::platform { + +std::intptr_t OpenWritableFile(const std::filesystem::path& path, std::string& error) { + // FILE_SHARE_READ | FILE_SHARE_WRITE so the lock file / other tools can peek + // at the partial file without us erroring; positional WriteFile is safe + // regardless of share mode. + HANDLE h = ::CreateFileW(path.wstring().c_str(), GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, nullptr); + if (h == INVALID_HANDLE_VALUE) { + error = "Win32 err " + std::to_string(::GetLastError()); + return kInvalidFileHandle; + } + return reinterpret_cast(h); +} + +bool WriteFileAt(std::intptr_t handle, std::int64_t offset, const std::uint8_t* data, std::size_t len, + std::string& error) { + HANDLE h = reinterpret_cast(handle); + // Concurrent WriteFile calls with distinct OVERLAPPED offsets on the same + // handle are safe for non-overlapping ranges; the kernel orders them. + while (len > 0) { + OVERLAPPED ov{}; + // Split the 64-bit file offset across the OVERLAPPED halves: the DWORD casts + // keep the low 32 bits in Offset and the high 32 bits in OffsetHigh. + ov.Offset = static_cast(static_cast(offset)); + ov.OffsetHigh = static_cast(static_cast(offset) >> 32); + DWORD to_write = static_cast(len > 0x7FFFFFFFu ? 0x7FFFFFFFu : len); + DWORD written = 0; + if (!::WriteFile(h, data, to_write, &written, &ov)) { + error = "write failed at offset " + std::to_string(offset) + " (Win32 err " + + std::to_string(::GetLastError()) + ")"; + return false; + } + if (written == 0) { + error = "short write at offset " + std::to_string(offset); + return false; + } + offset += static_cast(written); + data += written; + len -= written; + } + return true; +} + +bool CloseFile(std::intptr_t handle, std::string& error) { + HANDLE h = reinterpret_cast(handle); + if (!::CloseHandle(h)) { + error = "CloseHandle failed (Win32 err " + std::to_string(::GetLastError()) + ")"; + return false; + } + return true; +} + +} // namespace fl::platform diff --git a/sdk_v2/cpp/test/internal_api/blob_download_state_test.cc b/sdk_v2/cpp/test/internal_api/blob_download_state_test.cc index f70b99809..19c5152f9 100644 --- a/sdk_v2/cpp/test/internal_api/blob_download_state_test.cc +++ b/sdk_v2/cpp/test/internal_api/blob_download_state_test.cc @@ -17,23 +17,7 @@ using namespace fl; namespace { -class TempDir { - public: - TempDir() { - path_ = fl::test::MakeUniqueTempPath("fl_dlstate_test_"); - fs::create_directories(path_); - } - - ~TempDir() { - std::error_code ec; - fs::remove_all(path_, ec); - } - - const fs::path& path() const { return path_; } - - private: - fs::path path_; -}; +using fl::test::TempDir; constexpr int64_t kBlobSize = 20 * 1024 * 1024; // 20 MiB constexpr int32_t kChunkSize = 2 * 1024 * 1024; // 2 MiB diff --git a/sdk_v2/cpp/test/internal_api/cross_process_file_lock_test.cc b/sdk_v2/cpp/test/internal_api/cross_process_file_lock_test.cc index f32df42dd..f25df86ab 100644 --- a/sdk_v2/cpp/test/internal_api/cross_process_file_lock_test.cc +++ b/sdk_v2/cpp/test/internal_api/cross_process_file_lock_test.cc @@ -27,25 +27,7 @@ using namespace fl; namespace { -/// Per-test temp directory. Auto-cleans on destruction so a flaky test never -/// leaks lock files into the system temp dir. -class TempDir { - public: - TempDir() { - path_ = fl::test::MakeUniqueTempPath("fl_lock_test_"); - fs::create_directories(path_); - } - - ~TempDir() { - std::error_code ec; - fs::remove_all(path_, ec); - } - - const fs::path& path() const { return path_; } - - private: - fs::path path_; -}; +using fl::test::TempDir; } // namespace diff --git a/sdk_v2/cpp/test/internal_api/download_test.cc b/sdk_v2/cpp/test/internal_api/download_test.cc index 96ce4f543..d0f44059a 100644 --- a/sdk_v2/cpp/test/internal_api/download_test.cc +++ b/sdk_v2/cpp/test/internal_api/download_test.cc @@ -47,38 +47,7 @@ using namespace fl; namespace { -/// Create a temporary directory for test isolation. The name comes from MakeUniqueTempPath, so it is -/// unique both across the separate processes CTest launches per test and across multiple TempDirs -/// within one test. create_directory must succeed — the directory must not already exist — so a -/// residual collision (e.g. a directory leaked by an earlier process that reused this pid) advances -/// to the next name and retries instead of silently sharing an existing directory. -class TempDir { - public: - TempDir() { - while (true) { - auto candidate = fl::test::MakeUniqueTempPath("fl_test_"); - std::error_code ec; - if (fs::create_directory(candidate, ec)) { - path_ = std::move(candidate); - return; - } - if (ec) { - throw std::runtime_error("TempDir: failed to create '" + candidate.string() + "': " + - ec.message()); - } - // candidate already existed — try the next name. - } - } - ~TempDir() { - std::error_code ec; - fs::remove_all(path_, ec); - } - const fs::path& path() const { return path_; } - std::string string() const { return path_.string(); } - - private: - fs::path path_; -}; +using fl::test::TempDir; /// Read entire file contents. std::string ReadFile(const fs::path& path) { diff --git a/sdk_v2/cpp/test/internal_api/file_writer_test.cc b/sdk_v2/cpp/test/internal_api/file_writer_test.cc index 4eac37830..bc520b088 100644 --- a/sdk_v2/cpp/test/internal_api/file_writer_test.cc +++ b/sdk_v2/cpp/test/internal_api/file_writer_test.cc @@ -24,21 +24,7 @@ using namespace fl; namespace { -class TempPath { - public: - TempPath() { - path_ = fl::test::MakeUniqueTempPath("file_writer_test_"); - path_ += ".bin"; - } - ~TempPath() { - std::error_code ec; - fs::remove(path_, ec); - } - const fs::path& path() const { return path_; } - - private: - fs::path path_; -}; +using fl::test::TempPath; } // namespace diff --git a/sdk_v2/cpp/test/internal_api/test_helpers.h b/sdk_v2/cpp/test/internal_api/test_helpers.h index de4c41e22..c6bc06fdb 100644 --- a/sdk_v2/cpp/test/internal_api/test_helpers.h +++ b/sdk_v2/cpp/test/internal_api/test_helpers.h @@ -8,42 +8,16 @@ #include "inferencing/model_load_manager.h" #include "logger.h" -#include +#include "utils/temp_dir.h" + #include #include #include #include #include -#ifdef _WIN32 -#include // _getpid -#else -#include // getpid -#endif - namespace fl::test { -/// Current process id. CTest (gtest_discover_tests) launches a separate process per test, so -/// temp paths must include the pid to stay unique across concurrent test processes. process.h -/// is used instead of windows.h so callers that use std::min/std::max aren't broken by its macros. -inline long CurrentPid() { -#ifdef _WIN32 - return ::_getpid(); -#else - return static_cast(::getpid()); -#endif -} - -/// Build a unique path under the system temp directory as `_`. The pid -/// separates concurrent test processes and the per-process atomic counter separates callers -/// within one process, so no two live temp paths collide — no randomness required. -inline std::filesystem::path MakeUniqueTempPath(const std::string& prefix) { - static std::atomic counter{0}; - return std::filesystem::temp_directory_path() / - (prefix + std::to_string(CurrentPid()) + "_" + - std::to_string(counter.fetch_add(1, std::memory_order_relaxed))); -} - /// EP detector that only reports CPU — used by tests that load real models /// without requiring GPU hardware. class CpuOnlyEpDetector : public IEpDetector { diff --git a/sdk_v2/cpp/test/utils/temp_dir.h b/sdk_v2/cpp/test/utils/temp_dir.h new file mode 100644 index 000000000..ed55feb80 --- /dev/null +++ b/sdk_v2/cpp/test/utils/temp_dir.h @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +// Shared temp directory / path helpers for tests. CTest (gtest_discover_tests) +// launches a separate process per test, so temp names embed the pid plus a +// per-process counter and never collide across concurrent test processes. +#pragma once + +#include +#include +#include +#include +#include +#include + +#ifdef _WIN32 +#include // _getpid +#else +#include // getpid +#endif + +namespace fl::test { + +/// Current process id. process.h is used instead of windows.h so callers that use +/// std::min/std::max aren't broken by its macros. +inline long CurrentPid() { +#ifdef _WIN32 + return ::_getpid(); +#else + return static_cast(::getpid()); +#endif +} + +/// Build a unique path under the system temp directory as `_`. The pid +/// separates concurrent test processes and the per-process atomic counter separates callers +/// within one process, so no two live temp paths collide — no randomness required. +inline std::filesystem::path MakeUniqueTempPath(const std::string& prefix) { + static std::atomic counter{0}; + return std::filesystem::temp_directory_path() / + (prefix + std::to_string(CurrentPid()) + "_" + + std::to_string(counter.fetch_add(1, std::memory_order_relaxed))); +} + +/// RAII temporary directory for test isolation. The name comes from MakeUniqueTempPath, so it is +/// unique both across the separate processes CTest launches per test and across multiple TempDirs +/// within one test. create_directory must succeed — the directory must not already exist — so a +/// residual collision (e.g. a directory leaked by an earlier process that reused this pid) advances +/// to the next name and retries instead of silently sharing an existing directory. +class TempDir { + public: + explicit TempDir(const std::string& prefix = "fl_test_") { + while (true) { + auto candidate = MakeUniqueTempPath(prefix); + std::error_code ec; + if (std::filesystem::create_directory(candidate, ec)) { + path_ = std::move(candidate); + return; + } + if (ec) { + throw std::runtime_error("TempDir: failed to create '" + candidate.string() + "': " + + ec.message()); + } + // candidate already existed — try the next name. + } + } + + ~TempDir() { + std::error_code ec; + std::filesystem::remove_all(path_, ec); + } + + TempDir(const TempDir&) = delete; + TempDir& operator=(const TempDir&) = delete; + + const std::filesystem::path& path() const { return path_; } + std::string string() const { return path_.string(); } + + private: + std::filesystem::path path_; +}; + +/// RAII unique temporary file path. The path is not created here — callers create the file — and +/// it is removed on destruction so a flaky test never leaks files into the system temp dir. +class TempPath { + public: + explicit TempPath(const std::string& prefix = "fl_test_") : path_(MakeUniqueTempPath(prefix)) {} + + ~TempPath() { + std::error_code ec; + std::filesystem::remove(path_, ec); + } + + TempPath(const TempPath&) = delete; + TempPath& operator=(const TempPath&) = delete; + + const std::filesystem::path& path() const { return path_; } + + private: + std::filesystem::path path_; +}; + +} // namespace fl::test