From bc10092f591f9126d25a22711e758610f7075ee6 Mon Sep 17 00:00:00 2001 From: Artyom Ivanov Date: Tue, 20 Oct 2026 10:51:42 +0300 Subject: [PATCH 1/6] fix(TipCache): Do not purge TSB while they contain information that can be used --- src/common/os/path_utils.h | 17 ++++-- src/common/os/posix/path_utils.cpp | 93 +++++++++++++++++++++--------- src/common/os/win32/path_utils.cpp | 85 ++++++++++++++++----------- src/jrd/tpc.cpp | 83 ++++++++++++++++++-------- src/jrd/tpc_proto.h | 2 + 5 files changed, 189 insertions(+), 91 deletions(-) diff --git a/src/common/os/path_utils.h b/src/common/os/path_utils.h index fc37d0f4f3d..3c010695b09 100644 --- a/src/common/os/path_utils.h +++ b/src/common/os/path_utils.h @@ -64,15 +64,20 @@ class PathUtils **/ class DirIterator : protected Firebird::AutoStorage { + public: + using Flags = uint32_t; + static constexpr Flags F_NONE = 0; + static constexpr Flags F_RETURN_ONLY_FILENAME = 1 << 0; + public: // The constructor requires a string that is the path of the directory being iterated. // DirIterator may be located on stack, therefore use AutoStorage. - DirIterator(MemoryPool& p, const Firebird::PathName& dir) - : AutoStorage(p), dirPrefix(getPool(), dir) + DirIterator(MemoryPool& p, const Firebird::PathName& dir, const Flags flags) + : AutoStorage(p), dirPrefix(getPool(), dir), flags(flags) {} - DirIterator(const Firebird::PathName& dir) - : AutoStorage(), dirPrefix(getPool(), dir) + DirIterator(const Firebird::PathName& dir, const Flags flags) + : AutoStorage(), dirPrefix(getPool(), dir), flags(flags) {} // Destructor provided for memory cleanup @@ -103,6 +108,7 @@ class PathUtils protected: // Stores the path to the directory as given in the constructor const Firebird::PathName dirPrefix; + const Flags flags = F_NONE; }; /** isRelative returns true if the given path is relative, and false if not. @@ -171,7 +177,8 @@ class PathUtils All errors result in either exceptions being thrown, or a valid empty DirIterator being returned. **/ - static DirIterator* newDirIterator(MemoryPool&, const Firebird::PathName&); + static DirIterator* newDirIterator(MemoryPool&, const Firebird::PathName&, + const DirIterator::Flags flags = DirIterator::F_NONE); /** makeDir creates directory passed as parameter. return value is 0 on success or error code on error. diff --git a/src/common/os/posix/path_utils.cpp b/src/common/os/posix/path_utils.cpp index ea2253b8e43..8aec9d7a3eb 100644 --- a/src/common/os/posix/path_utils.cpp +++ b/src/common/os/posix/path_utils.cpp @@ -32,7 +32,10 @@ #include #include #include +#include + #include +#include using namespace Firebird; @@ -48,14 +51,15 @@ const size_t PathUtils::up_dir_link_len = std::char_traits::length(up_dir_ class PosixDirIterator : public PathUtils::DirIterator { public: - PosixDirIterator(MemoryPool& p, const PathName& path) - : DirIterator(p, path), dir(nullptr), ent(nullptr), file(p), done(false) + PosixDirIterator(MemoryPool& p, const PathName& path, const PathUtils::DirIterator::Flags flags) + : DirIterator(p, path, flags), + m_file(p), m_tmp_buffer(p) { init(); } - PosixDirIterator(const PathName& path) - : DirIterator(path), dir(0), done(false) + PosixDirIterator(const PathName& path, const PathUtils::DirIterator::Flags flags) + : DirIterator(path, flags) { init(); } @@ -63,65 +67,98 @@ class PosixDirIterator : public PathUtils::DirIterator ~PosixDirIterator(); const PosixDirIterator& operator++() override; - const PathName& operator*() noexcept override { return file; } - operator bool() noexcept override { return !done; } + const PathName& operator*() noexcept override { return m_file; } + operator bool() noexcept override { return !m_done; } private: - DIR* dir; - dirent* ent; - PathName file; - bool done; - void init(); + +private: + DIR* m_dir = nullptr; + int m_dir_fd = -1; + dirent* m_ent = nullptr; + + PathName m_file; + // Used to minimize reallocation on every iteration. + PathName m_tmp_buffer; + + bool m_done = false; }; void PosixDirIterator::init() { - dir = opendir(dirPrefix.c_str()); + m_dir = opendir(dirPrefix.c_str()); - if (!dir) + if (!m_dir) system_call_failed::raise("opendir"); + m_dir_fd = dirfd(m_dir); + ++(*this); } PosixDirIterator::~PosixDirIterator() { - if (dir) + if (m_dir) { - closedir(dir); - dir = nullptr; + closedir(m_dir); + m_dir = nullptr; } - done = true; + m_done = true; } const PosixDirIterator& PosixDirIterator::operator++() { - if (!done) + if (!m_done) { - while ( (ent = os_utils::readdir(dir)) ) + while ( (m_ent = os_utils::readdir(m_dir)) ) { - PathUtils::concatPath(file, dirPrefix, ent->d_name); + std::optional is_reg_file; + +#ifdef _DIRENT_HAVE_D_TYPE + // Fast path of checking is file a regular file. + if (m_ent->d_type == DT_REG) + is_reg_file = true; + else if (m_ent->d_type != DT_UNKNOWN) + is_reg_file = false; +#endif + if (!is_reg_file.has_value()) + { + // Slow path, should be taken rarely because most of the FS support and fill `d_type` with actual value. + struct stat stat_buf; + if (fstatat(m_dir_fd, m_ent->d_name, &stat_buf, AT_SYMLINK_NOFOLLOW) != 0) + continue; + is_reg_file = S_ISREG(stat_buf.st_mode); + } + + if (is_reg_file.value() == false) + continue; + + if (flags & PathUtils::DirIterator::F_RETURN_ONLY_FILENAME) + m_file = m_ent->d_name; + else + { + m_tmp_buffer = m_ent->d_name; + PathUtils::concatPath(m_file, dirPrefix, m_tmp_buffer); + } - struct stat stats; - if (!stat(file.c_str(), &stats) && S_ISREG(stats.st_mode)) - break; + break; } - if (!ent) + if (!m_ent) { - done = true; - file.clear(); + m_done = true; + m_file.clear(); } } return *this; } -PathUtils::DirIterator* PathUtils::newDirIterator(MemoryPool& p, const PathName& path) +PathUtils::DirIterator* PathUtils::newDirIterator(MemoryPool& p, const PathName& path, const DirIterator::Flags flags) { - return FB_NEW_POOL(p) PosixDirIterator(p, path); + return FB_NEW_POOL(p) PosixDirIterator(p, path, flags); } void PathUtils::splitLastComponent(PathName& path, PathName& file, diff --git a/src/common/os/win32/path_utils.cpp b/src/common/os/win32/path_utils.cpp index ecd2e37907c..afb5ffff26b 100644 --- a/src/common/os/win32/path_utils.cpp +++ b/src/common/os/win32/path_utils.cpp @@ -19,14 +19,14 @@ const size_t PathUtils::up_dir_link_len = std::char_traits::length(up_dir_ class Win32DirIterator final : public PathUtils::DirIterator { public: - Win32DirIterator(MemoryPool& p, const PathName& path) - : DirIterator(p, path), dir(0), file(getPool()), done(false) + Win32DirIterator(MemoryPool& p, const PathName& path, const PathUtils::DirIterator::Flags flags) + : DirIterator(p, path, flags), file(getPool()), tmp_buffer(getPool()) { init(); } - Win32DirIterator(const PathName& path) - : DirIterator(path), dir(0), file(getPool()), done(false) + Win32DirIterator(const PathName& path, const PathUtils::DirIterator::Flags flags) + : DirIterator(path, flags), file(getPool()), tmp_buffer(getPool()) { init(); } @@ -38,43 +38,65 @@ class Win32DirIterator final : public PathUtils::DirIterator operator bool() noexcept override { return !done; } private: - HANDLE dir; + void init(); + void setCurrentFile(); + + bool isRegularFile() const + { + return !(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY); + } + +private: + HANDLE dir = INVALID_HANDLE_VALUE; WIN32_FIND_DATA fd; + PathName file; - bool done; + // Used to minimize reallocation on every iteration. + PathName tmp_buffer; - void init(); + bool done = false; }; void Win32DirIterator::init() { PathName mask(dirPrefix); PathUtils::ensureSeparator(mask); - mask += "*.*"; + mask += '*'; + + // `FindExInfoBasic` skips the short name lookup (legacy stuff) and `FIND_FIRST_EX_LARGE_FETCH` + // makes the kernel return entries in larger batches, so fewer system calls are made. + // Both are rejected with `ERROR_INVALID_PARAMETER` before Windows 7, fall back then. + dir = FindFirstFileEx(mask.c_str(), FindExInfoBasic, &fd, FindExSearchNameMatch, + NULL, FIND_FIRST_EX_LARGE_FETCH); - dir = FindFirstFile(mask.c_str(), &fd); + if (dir == INVALID_HANDLE_VALUE && GetLastError() == ERROR_INVALID_PARAMETER) + { + dir = FindFirstFileEx(mask.c_str(), FindExInfoStandard, &fd, FindExSearchNameMatch, + NULL, 0); + } if (dir == INVALID_HANDLE_VALUE) { - auto const last_error = GetLastError(); - if (last_error != ERROR_FILE_NOT_FOUND && last_error != ERROR_PATH_NOT_FOUND) - system_call_failed::raise("FindFirstFile"); + const DWORD lastError = GetLastError(); + if (lastError != ERROR_FILE_NOT_FOUND && lastError != ERROR_PATH_NOT_FOUND) + system_call_failed::raise("FindFirstFileEx", lastError); - dir = 0; done = true; + return; } - else if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) - ++(*this); + + if (isRegularFile()) + setCurrentFile(); else - PathUtils::concatPath(file, dirPrefix, fd.cFileName); + ++(*this); } Win32DirIterator::~Win32DirIterator() { - if (dir) + if (dir != INVALID_HANDLE_VALUE) { FindClose(dir); - dir = 0; + dir = INVALID_HANDLE_VALUE; } done = true; @@ -82,29 +104,26 @@ Win32DirIterator::~Win32DirIterator() const PathUtils::DirIterator& Win32DirIterator::operator++() { - if (!done) + if (done) + return *this; + + while (FindNextFile(dir, &fd)) { - while (true) - { - if (!FindNextFile(dir, &fd)) - { - done = true; - break; - } - else if (!(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) - break; - } + if (!isRegularFile()) + continue; - if (!done) - PathUtils::concatPath(file, dirPrefix, fd.cFileName); + setCurrentFile(); + return *this; } + done = true; return *this; } -PathUtils::DirIterator* PathUtils::newDirIterator(MemoryPool& p, const PathName& path) +PathUtils::DirIterator* PathUtils::newDirIterator(MemoryPool& p, const PathName& path, + const DirIterator::Flags flags) { - return FB_NEW_POOL(p) Win32DirIterator(p, path); + return FB_NEW_POOL(p) Win32DirIterator(p, path, flags); } void PathUtils::splitLastComponent(PathName& path, PathName& file, diff --git a/src/jrd/tpc.cpp b/src/jrd/tpc.cpp index 011b298a808..ec55deb3439 100644 --- a/src/jrd/tpc.cpp +++ b/src/jrd/tpc.cpp @@ -33,6 +33,7 @@ #include "../jrd/tra_proto.h" #include "../jrd/replication/Publisher.h" #include "../common/isc_proto.h" +#include "../common/os/path_utils.h" #include @@ -62,7 +63,11 @@ bool TipCache::GlobalTpcInitializer::initialize(SharedMemoryBase* sm, bool initF } thread_db* tdbb = JRD_get_thread_data(); - const Database* dbb = tdbb->getDatabase(); + Database* dbb = tdbb->getDatabase(); + + // Purge every existing TSB for this database, they can contain garbage values after the crash + // or after unfortunate shutdown. + m_cache->purgeTransactionStatusBlock(*dbb); // Initialize the shared data header initHeader(header); @@ -103,10 +108,13 @@ bool TipCache::MemBlockInitializer::initialize(SharedMemoryBase* sm, bool initFl TransactionStatusBlock* header = static_cast(sm->sh_mem_header); - // Initialize the shared data header - initHeader(header); + if (!checkHeader(header, false)) + { + // Initialize the shared data header + initHeader(header); - memset(header->data, 0, sm->sh_mem_length_mapped - offsetof(TransactionStatusBlock, data[0])); + memset(header->data, 0, sm->sh_mem_length_mapped - offsetof(TransactionStatusBlock, data[0])); + } fb_assert(header->data->is_lock_free()); @@ -167,6 +175,14 @@ void TipCache::finalizeTpc(thread_db* tdbb) if (m_tpcHeader) { + if (LCK_convert(tdbb, m_lock, LCK_EX, LCK_NO_WAIT)) + { + purgeTransactionStatusBlock(*tdbb->getDatabase()); + LCK_convert(tdbb, m_lock, LCK_SW, LCK_WAIT); + } + else + tdbb->tdbb_status_vector->init(); + nmHdr = m_tpcHeader->getMapFileName(); delete m_tpcHeader; m_tpcHeader = NULL; @@ -175,9 +191,9 @@ void TipCache::finalizeTpc(thread_db* tdbb) m_blocks_memory.clear(); m_transactionsPerBlock = 0; - if (nmSnap.hasData() || nmHdr.hasData()) - { - if (LCK_lock(tdbb, m_lock, LCK_EX, LCK_NO_WAIT)) + if (nmSnap.hasData() || nmHdr.hasData()) + { + if (LCK_lock(tdbb, m_lock, LCK_EX, LCK_NO_WAIT)) { if (nmSnap.hasData()) SharedMemoryBase::unlinkFile(nmSnap.c_str()); @@ -195,6 +211,41 @@ void TipCache::finalizeTpc(thread_db* tdbb) m_lock.reset(); } +// We are the last or first one who holds the `GlobalTpcHeader`, so remove all related TSB from disk. +void TipCache::purgeTransactionStatusBlock(Database& dbb) +{ + PathName filenameTemplate = StatusBlockData::makeSharedMemoryFileName(&dbb, 0, false); + // Drop block number from the filename. + filenameTemplate.erase(filenameTemplate.length() - 1, 1); + + TEXT tmpDirBuffer[MAXPATHLEN]; + gds__prefix_lock(tmpDirBuffer, ""); + + const PathName tmpDir = tmpDirBuffer; + PathName fullTmpPathname; + try + { + for (AutoPtr iter = PathUtils::newDirIterator(*getDefaultMemoryPool(), tmpDir, PathUtils::DirIterator::F_RETURN_ONLY_FILENAME); + *iter; ++(*iter)) + { + const auto& tmpFilename = **iter; + if (filenameTemplate.length() >= tmpFilename.length()) + continue; + if (filenameTemplate.compare(tmpFilename.data(), filenameTemplate.length())) + continue; + + PathUtils::concatPath(fullTmpPathname, tmpDir, tmpFilename); + + SharedMemoryBase::unlinkFile(fullTmpPathname.data()); + } + } + catch (...) + { + gds__log("An error occurred while clearing the TSB during the TipCache initialization or shutdown procedure, " + "which may result in some `tpc` files remaining in the tmp folder."); + } +} + CommitNumber TipCache::cacheState(TraNumber number) { fb_assert(m_tpcHeader); @@ -426,7 +477,6 @@ TipCache::StatusBlockData::~StatusBlockData() void TipCache::StatusBlockData::clear(thread_db* tdbb) { // memory could be already released at tpc_block_blocking_ast - PathName fName; if (memory) { // wait for all initializing processes (PR) @@ -455,27 +505,10 @@ void TipCache::StatusBlockData::clear(thread_db* tdbb) ERR_bugcheck_msg("Unable to convert TPC lock (SW)"); } - fName = memory->getMapFileName(); delete memory; memory = NULL; } - if (fName.hasData()) - { - // Here file is removed from SharedMemory created with skipLock parameter - // set to true. That means internal file lock is turned off. - // Appropriate locking is performed by existenceLock using LM. - // This should be in sync with SharedMemory constructor called - // in TipCache::StatusBlockData constructor. - if (LCK_lock(tdbb, &existenceLock, LCK_EX, LCK_NO_WAIT)) - SharedMemoryBase::unlinkFile(fName.c_str()); - else - { - tdbb->tdbb_status_vector->init(); - return; - } - } - LCK_release(tdbb, &existenceLock); } diff --git a/src/jrd/tpc_proto.h b/src/jrd/tpc_proto.h index fede1f57517..7995e78a33a 100644 --- a/src/jrd/tpc_proto.h +++ b/src/jrd/tpc_proto.h @@ -376,6 +376,8 @@ class TipCache SnapshotHandle allocateSnapshotSlot(); void deallocateSnapshotSlot(SnapshotHandle slotNumber); void remapSnapshots(bool sync); + + void purgeTransactionStatusBlock(Database& dbb); }; From eca83893f4329dbbbc944cdca9166767c6b06a1b Mon Sep 17 00:00:00 2001 From: Artyom Ivanov Date: Tue, 20 Oct 2026 17:35:28 +0300 Subject: [PATCH 2/6] Add missing function impl on Windows --- src/common/os/win32/path_utils.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/common/os/win32/path_utils.cpp b/src/common/os/win32/path_utils.cpp index afb5ffff26b..742c74f9dca 100644 --- a/src/common/os/win32/path_utils.cpp +++ b/src/common/os/win32/path_utils.cpp @@ -41,7 +41,7 @@ class Win32DirIterator final : public PathUtils::DirIterator void init(); void setCurrentFile(); - bool isRegularFile() const + bool isRegularFile() const noexcept { return !(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY); } @@ -91,6 +91,17 @@ void Win32DirIterator::init() ++(*this); } +void Win32DirIterator::setCurrentFile() +{ + if (flags & PathUtils::DirIterator::F_RETURN_ONLY_FILENAME) + file = fd.cFileName; + else + { + tmp_buffer = fd.cFileName; + PathUtils::concatPath(file, dirPrefix, tmp_buffer); + } +} + Win32DirIterator::~Win32DirIterator() { if (dir != INVALID_HANDLE_VALUE) From d529e76db887c10973f02b6ea2de62c9f14d8eb5 Mon Sep 17 00:00:00 2001 From: Artyom Ivanov Date: Wed, 21 Oct 2026 09:34:47 +0300 Subject: [PATCH 3/6] Handle fail of `dirfd` call --- src/common/os/posix/path_utils.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/common/os/posix/path_utils.cpp b/src/common/os/posix/path_utils.cpp index 8aec9d7a3eb..56ae64644d7 100644 --- a/src/common/os/posix/path_utils.cpp +++ b/src/common/os/posix/path_utils.cpp @@ -127,8 +127,21 @@ const PosixDirIterator& PosixDirIterator::operator++() { // Slow path, should be taken rarely because most of the FS support and fill `d_type` with actual value. struct stat stat_buf; - if (fstatat(m_dir_fd, m_ent->d_name, &stat_buf, AT_SYMLINK_NOFOLLOW) != 0) - continue; + if (m_dir_fd >= 0) + { + if (fstatat(m_dir_fd, m_ent->d_name, &stat_buf, AT_SYMLINK_NOFOLLOW) != 0) + continue; + } + else + { + // Very slow path, `dirfd` has failed, so the full path must be resolved by the kernel. + // Shouldn't happen on modern systems, but POSIX allows it. + m_tmp_buffer = m_ent->d_name; + PathUtils::concatPath(m_file, dirPrefix, m_tmp_buffer); + if (lstat(m_file.c_str(), &stat_buf) != 0) + continue; + } + is_reg_file = S_ISREG(stat_buf.st_mode); } From 3523dc963629980ab6f99646387f6a44198dbdc4 Mon Sep 17 00:00:00 2001 From: Artyom Ivanov Date: Wed, 21 Oct 2026 09:38:28 +0300 Subject: [PATCH 4/6] Do not memset to zero mapped SharedMemory on Windows - It is responsibility of the caller to memset new mapped region; - Bring this in line with Linux logic; --- src/common/isc_sync.cpp | 107 +++++++++++++++++++++++----------------- 1 file changed, 63 insertions(+), 44 deletions(-) diff --git a/src/common/isc_sync.cpp b/src/common/isc_sync.cpp index 9998406e93f..7ec513c664c 100644 --- a/src/common/isc_sync.cpp +++ b/src/common/isc_sync.cpp @@ -1703,6 +1703,32 @@ SharedMemoryBase::SharedMemoryBase(const TEXT* filename, ULONG length, IpcObject } } + if (init_flag) + { + // We created the event, so no process in our IPC namespace uses this file. + // If somebody still has it open, it's another engine instance (e.g. in another + // Windows session with its own object namespace), so don't touch the file. + HANDLE probe_handle = CreateFile(expanded_filename, + GENERIC_READ, + 0, // no sharing: fails if anyone has the file open + NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + + if (probe_handle != INVALID_HANDLE_VALUE) + CloseHandle(probe_handle); + else if (GetLastError() == ERROR_SHARING_VIOLATION) + { + if (retry_count < 50) // 0.5 sec + goto retry; + + CloseHandle(event_handle); + Arg::Gds(isc_instance_conflict).raise(); + } + // ERROR_FILE_NOT_FOUND is the normal case, other errors are reported by CreateFile below. + } + HANDLE file_handle = CreateFile(expanded_filename, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, @@ -1730,44 +1756,20 @@ SharedMemoryBase::SharedMemoryBase(const TEXT* filename, ULONG length, IpcObject if (init_flag) { - if (err == ERROR_ALREADY_EXISTS) - { - const DWORD file_size = GetFileSize(file_handle, NULL); - if (file_size == INVALID_FILE_SIZE) - { - err = GetLastError(); - CloseHandle(event_handle); - CloseHandle(file_handle); - system_call_failed::raise("GetFileSize", err); - } + LARGE_INTEGER file_size; + file_size.QuadPart = 0; - if (length == 0) - { - length = file_size; - } - else if (file_size && - SetFilePointer(file_handle, 0, NULL, FILE_BEGIN) == INVALID_SET_FILE_POINTER || - !SetEndOfFile(file_handle) || !FlushFileBuffers(file_handle)) - { - err = GetLastError(); - CloseHandle(file_handle); - - if (err == ERROR_USER_MAPPED_FILE) - { - if (retry_count < 50) // 0.5 sec - goto retry; - - CloseHandle(event_handle); - Arg::Gds(isc_instance_conflict).raise(); - } - else - { - CloseHandle(event_handle); - system_call_failed::raise("SetFilePointer", err); - } - } + if (err == ERROR_ALREADY_EXISTS && !GetFileSizeEx(file_handle, &file_size)) + { + err = GetLastError(); + CloseHandle(event_handle); + CloseHandle(file_handle); + system_call_failed::raise("GetFileSizeEx", err); } + if (length == 0) + length = static_cast(file_size.QuadPart); + if (length == 0) { CloseHandle(event_handle); @@ -1782,16 +1784,33 @@ SharedMemoryBase::SharedMemoryBase(const TEXT* filename, ULONG length, IpcObject (Arg::Gds(isc_random) << Arg::Str("File for memory mapping is empty.")).raise(); } - LARGE_INTEGER offset; - offset.QuadPart = length; - - if (SetFilePointer(file_handle, offset.LowPart, &offset.HighPart, FILE_BEGIN) == INVALID_SET_FILE_POINTER || - !SetEndOfFile(file_handle) || !FlushFileBuffers(file_handle)) + // Only adjust the size: initializing the content is the job of + // IpcObject::initialize(), the same as with ftruncate() on POSIX. + if (file_size.QuadPart != static_cast(length)) { - err = GetLastError(); - CloseHandle(event_handle); - CloseHandle(file_handle); - system_call_failed::raise("SetFilePointer", err); + LARGE_INTEGER offset; + offset.QuadPart = length; + + if (!SetFilePointerEx(file_handle, offset, NULL, FILE_BEGIN) || + !SetEndOfFile(file_handle) || !FlushFileBuffers(file_handle)) + { + err = GetLastError(); + CloseHandle(file_handle); + + // The file is still mapped by a killed process that hasn't released + // its mappings yet, wait a bit. + if (err == ERROR_USER_MAPPED_FILE) + { + if (retry_count < 50) // 0.5 sec + goto retry; + + CloseHandle(event_handle); + Arg::Gds(isc_instance_conflict).raise(); + } + + CloseHandle(event_handle); + system_call_failed::raise("SetEndOfFile", err); + } } } From 81c46bb4933e8addb94f202939bf48b223dd6298 Mon Sep 17 00:00:00 2001 From: Artyom Ivanov Date: Wed, 21 Oct 2026 10:45:52 +0300 Subject: [PATCH 5/6] Fix style --- src/common/os/posix/path_utils.cpp | 36 ++++++++++----------- src/common/os/win32/path_utils.cpp | 50 +++++++++++++++--------------- src/jrd/tpc.cpp | 6 ++-- src/jrd/tpc_proto.h | 2 +- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/common/os/posix/path_utils.cpp b/src/common/os/posix/path_utils.cpp index 56ae64644d7..34b0daf2a3c 100644 --- a/src/common/os/posix/path_utils.cpp +++ b/src/common/os/posix/path_utils.cpp @@ -53,7 +53,7 @@ class PosixDirIterator : public PathUtils::DirIterator public: PosixDirIterator(MemoryPool& p, const PathName& path, const PathUtils::DirIterator::Flags flags) : DirIterator(p, path, flags), - m_file(p), m_tmp_buffer(p) + m_file(p), m_tmpBuffer(p) { init(); } @@ -75,12 +75,12 @@ class PosixDirIterator : public PathUtils::DirIterator private: DIR* m_dir = nullptr; - int m_dir_fd = -1; + int m_dirFd = -1; dirent* m_ent = nullptr; PathName m_file; // Used to minimize reallocation on every iteration. - PathName m_tmp_buffer; + PathName m_tmpBuffer; bool m_done = false; }; @@ -92,7 +92,7 @@ void PosixDirIterator::init() if (!m_dir) system_call_failed::raise("opendir"); - m_dir_fd = dirfd(m_dir); + m_dirFd = dirfd(m_dir); ++(*this); } @@ -114,46 +114,46 @@ const PosixDirIterator& PosixDirIterator::operator++() { while ( (m_ent = os_utils::readdir(m_dir)) ) { - std::optional is_reg_file; + std::optional isRegFile; #ifdef _DIRENT_HAVE_D_TYPE // Fast path of checking is file a regular file. if (m_ent->d_type == DT_REG) - is_reg_file = true; + isRegFile = true; else if (m_ent->d_type != DT_UNKNOWN) - is_reg_file = false; + isRegFile = false; #endif - if (!is_reg_file.has_value()) + if (!isRegFile.has_value()) { // Slow path, should be taken rarely because most of the FS support and fill `d_type` with actual value. - struct stat stat_buf; - if (m_dir_fd >= 0) + struct stat statBuf; + if (m_dirFd >= 0) { - if (fstatat(m_dir_fd, m_ent->d_name, &stat_buf, AT_SYMLINK_NOFOLLOW) != 0) + if (fstatat(m_dirFd, m_ent->d_name, &statBuf, AT_SYMLINK_NOFOLLOW) != 0) continue; } else { // Very slow path, `dirfd` has failed, so the full path must be resolved by the kernel. // Shouldn't happen on modern systems, but POSIX allows it. - m_tmp_buffer = m_ent->d_name; - PathUtils::concatPath(m_file, dirPrefix, m_tmp_buffer); - if (lstat(m_file.c_str(), &stat_buf) != 0) + m_tmpBuffer = m_ent->d_name; + PathUtils::concatPath(m_file, dirPrefix, m_tmpBuffer); + if (lstat(m_file.c_str(), &statBuf) != 0) continue; } - is_reg_file = S_ISREG(stat_buf.st_mode); + isRegFile = S_ISREG(statBuf.st_mode); } - if (is_reg_file.value() == false) + if (isRegFile.value() == false) continue; if (flags & PathUtils::DirIterator::F_RETURN_ONLY_FILENAME) m_file = m_ent->d_name; else { - m_tmp_buffer = m_ent->d_name; - PathUtils::concatPath(m_file, dirPrefix, m_tmp_buffer); + m_tmpBuffer = m_ent->d_name; + PathUtils::concatPath(m_file, dirPrefix, m_tmpBuffer); } break; diff --git a/src/common/os/win32/path_utils.cpp b/src/common/os/win32/path_utils.cpp index 742c74f9dca..b538d7aa557 100644 --- a/src/common/os/win32/path_utils.cpp +++ b/src/common/os/win32/path_utils.cpp @@ -20,13 +20,13 @@ class Win32DirIterator final : public PathUtils::DirIterator { public: Win32DirIterator(MemoryPool& p, const PathName& path, const PathUtils::DirIterator::Flags flags) - : DirIterator(p, path, flags), file(getPool()), tmp_buffer(getPool()) + : DirIterator(p, path, flags), m_file(getPool()), m_tmpBuffer(getPool()) { init(); } Win32DirIterator(const PathName& path, const PathUtils::DirIterator::Flags flags) - : DirIterator(path, flags), file(getPool()), tmp_buffer(getPool()) + : DirIterator(path, flags), m_file(getPool()), m_tmpBuffer(getPool()) { init(); } @@ -34,8 +34,8 @@ class Win32DirIterator final : public PathUtils::DirIterator ~Win32DirIterator(); const PathUtils::DirIterator& operator++() override; - const PathName& operator*() noexcept override { return file; } - operator bool() noexcept override { return !done; } + const PathName& operator*() noexcept override { return m_file; } + operator bool() noexcept override { return !m_done; } private: void init(); @@ -43,18 +43,18 @@ class Win32DirIterator final : public PathUtils::DirIterator bool isRegularFile() const noexcept { - return !(fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY); + return !(m_fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY); } private: - HANDLE dir = INVALID_HANDLE_VALUE; - WIN32_FIND_DATA fd; + HANDLE m_dir = INVALID_HANDLE_VALUE; + WIN32_FIND_DATA m_fd; - PathName file; + PathName m_file; // Used to minimize reallocation on every iteration. - PathName tmp_buffer; + PathName m_tmpBuffer; - bool done = false; + bool m_done = false; }; void Win32DirIterator::init() @@ -66,22 +66,22 @@ void Win32DirIterator::init() // `FindExInfoBasic` skips the short name lookup (legacy stuff) and `FIND_FIRST_EX_LARGE_FETCH` // makes the kernel return entries in larger batches, so fewer system calls are made. // Both are rejected with `ERROR_INVALID_PARAMETER` before Windows 7, fall back then. - dir = FindFirstFileEx(mask.c_str(), FindExInfoBasic, &fd, FindExSearchNameMatch, + m_dir = FindFirstFileEx(mask.c_str(), FindExInfoBasic, &m_fd, FindExSearchNameMatch, NULL, FIND_FIRST_EX_LARGE_FETCH); - if (dir == INVALID_HANDLE_VALUE && GetLastError() == ERROR_INVALID_PARAMETER) + if (m_dir == INVALID_HANDLE_VALUE && GetLastError() == ERROR_INVALID_PARAMETER) { - dir = FindFirstFileEx(mask.c_str(), FindExInfoStandard, &fd, FindExSearchNameMatch, + m_dir = FindFirstFileEx(mask.c_str(), FindExInfoStandard, &m_fd, FindExSearchNameMatch, NULL, 0); } - if (dir == INVALID_HANDLE_VALUE) + if (m_dir == INVALID_HANDLE_VALUE) { const DWORD lastError = GetLastError(); if (lastError != ERROR_FILE_NOT_FOUND && lastError != ERROR_PATH_NOT_FOUND) system_call_failed::raise("FindFirstFileEx", lastError); - done = true; + m_done = true; return; } @@ -94,31 +94,31 @@ void Win32DirIterator::init() void Win32DirIterator::setCurrentFile() { if (flags & PathUtils::DirIterator::F_RETURN_ONLY_FILENAME) - file = fd.cFileName; + m_file = m_fd.cFileName; else { - tmp_buffer = fd.cFileName; - PathUtils::concatPath(file, dirPrefix, tmp_buffer); + m_tmpBuffer = m_fd.cFileName; + PathUtils::concatPath(m_file, dirPrefix, m_tmpBuffer); } } Win32DirIterator::~Win32DirIterator() { - if (dir != INVALID_HANDLE_VALUE) + if (m_dir != INVALID_HANDLE_VALUE) { - FindClose(dir); - dir = INVALID_HANDLE_VALUE; + FindClose(m_dir); + m_dir = INVALID_HANDLE_VALUE; } - done = true; + m_done = true; } const PathUtils::DirIterator& Win32DirIterator::operator++() { - if (done) + if (m_done) return *this; - while (FindNextFile(dir, &fd)) + while (FindNextFile(m_dir, &m_fd)) { if (!isRegularFile()) continue; @@ -127,7 +127,7 @@ const PathUtils::DirIterator& Win32DirIterator::operator++() return *this; } - done = true; + m_done = true; return *this; } diff --git a/src/jrd/tpc.cpp b/src/jrd/tpc.cpp index ec55deb3439..127458201e6 100644 --- a/src/jrd/tpc.cpp +++ b/src/jrd/tpc.cpp @@ -67,7 +67,7 @@ bool TipCache::GlobalTpcInitializer::initialize(SharedMemoryBase* sm, bool initF // Purge every existing TSB for this database, they can contain garbage values after the crash // or after unfortunate shutdown. - m_cache->purgeTransactionStatusBlock(*dbb); + m_cache->purgeTransactionStatusBlocks(*dbb); // Initialize the shared data header initHeader(header); @@ -177,7 +177,7 @@ void TipCache::finalizeTpc(thread_db* tdbb) { if (LCK_convert(tdbb, m_lock, LCK_EX, LCK_NO_WAIT)) { - purgeTransactionStatusBlock(*tdbb->getDatabase()); + purgeTransactionStatusBlocks(*tdbb->getDatabase()); LCK_convert(tdbb, m_lock, LCK_SW, LCK_WAIT); } else @@ -212,7 +212,7 @@ void TipCache::finalizeTpc(thread_db* tdbb) } // We are the last or first one who holds the `GlobalTpcHeader`, so remove all related TSB from disk. -void TipCache::purgeTransactionStatusBlock(Database& dbb) +void TipCache::purgeTransactionStatusBlocks(Database& dbb) { PathName filenameTemplate = StatusBlockData::makeSharedMemoryFileName(&dbb, 0, false); // Drop block number from the filename. diff --git a/src/jrd/tpc_proto.h b/src/jrd/tpc_proto.h index 7995e78a33a..f4e24b8537e 100644 --- a/src/jrd/tpc_proto.h +++ b/src/jrd/tpc_proto.h @@ -377,7 +377,7 @@ class TipCache void deallocateSnapshotSlot(SnapshotHandle slotNumber); void remapSnapshots(bool sync); - void purgeTransactionStatusBlock(Database& dbb); + void purgeTransactionStatusBlocks(Database& dbb); }; From 3d453bb00c64f36266bf4b37abdee2cdaf3c976c Mon Sep 17 00:00:00 2001 From: Artyom Ivanov Date: Wed, 21 Oct 2026 11:07:10 +0300 Subject: [PATCH 6/6] Add more comments explaining why we do things this way --- src/jrd/tpc.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/jrd/tpc.cpp b/src/jrd/tpc.cpp index 127458201e6..e65cb71ffdc 100644 --- a/src/jrd/tpc.cpp +++ b/src/jrd/tpc.cpp @@ -108,6 +108,8 @@ bool TipCache::MemBlockInitializer::initialize(SharedMemoryBase* sm, bool initFl TransactionStatusBlock* header = static_cast(sm->sh_mem_header); + // See if TSB is already has been initialized, if it is, do not memset it's content + // because it contains valid information about CN. if (!checkHeader(header, false)) { // Initialize the shared data header @@ -175,6 +177,8 @@ void TipCache::finalizeTpc(thread_db* tdbb) if (m_tpcHeader) { + // Check if we are the last one who holds the TipCache. If so, it's time + // to cleanup all related TSBs. if (LCK_convert(tdbb, m_lock, LCK_EX, LCK_NO_WAIT)) { purgeTransactionStatusBlocks(*tdbb->getDatabase());