fix(TipCache): Committed transactions becoming invisible after TransactionStatusBlock is re-mapped

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<ULONG>(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<LONGLONG>(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);
+			}
 		}
 	}
 

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.

src/common/os/posix/path_utils.cpp

@@ -32,7 +32,10 @@
 #include <sys/stat.h>
 #include <dirent.h>
 #include <unistd.h>
+#include <fcntl.h>
+
 #include <string>
+#include <optional>
 
 using namespace Firebird;
 
@@ -48,80 +51,127 @@ const size_t PathUtils::up_dir_link_len = std::char_traits<char>::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_tmpBuffer(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();
 	}
 
 	~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_dirFd = -1;
+	dirent* m_ent = nullptr;
+
+	PathName m_file;
+	// Used to minimize reallocation on every iteration.
+	PathName m_tmpBuffer;
+
+	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_dirFd = 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<bool> isRegFile;
+
+#ifdef _DIRENT_HAVE_D_TYPE
+			// Fast path of checking is file a regular file.
+			if (m_ent->d_type == DT_REG)
+				isRegFile = true;
+			else if (m_ent->d_type != DT_UNKNOWN)
+				isRegFile = false;
+#endif
+			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 statBuf;
+				if (m_dirFd >= 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_tmpBuffer = m_ent->d_name;
+					PathUtils::concatPath(m_file, dirPrefix, m_tmpBuffer);
+					if (lstat(m_file.c_str(), &statBuf) != 0)
+						continue;
+				}
+
+				isRegFile = S_ISREG(statBuf.st_mode);
+			}
+
+			if (isRegFile.value() == false)
+				continue;
+
+			if (flags & PathUtils::DirIterator::F_RETURN_ONLY_FILENAME)
+				m_file = m_ent->d_name;
+			else
+			{
+				m_tmpBuffer = m_ent->d_name;
+				PathUtils::concatPath(m_file, dirPrefix, m_tmpBuffer);
+			}
 
-			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,