From 1acd6b34275d37f7250955a9e11017f874491e73 Mon Sep 17 00:00:00 2001
From: Artyom Ivanov <artyom.ivanov@red-soft.ru>
Date: Tue, 5 Aug 2025 10:55:01 +0300
Subject: [PATCH] Backport of fix(shutdown): Prevent race condition when
 GlobalObject destruction routine unlocks global mutex

Unlocking global mutex in GlobalObject destruction routine made it possible for a new attachment to slip in, so it will be creating new GlobalObject and using it, while destroying routine still in action. This can lead to an undefined state of the global objects, such as shared memory, where one thread is actively using it while another thread is destroying it.
---
 src/jrd/Database.cpp | 51 ++++++++++++++++++++++++++++++++++++++------
 src/jrd/Database.h   |  6 ++++--
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/jrd/Database.cpp b/src/jrd/Database.cpp
index cbcaa1aa251..a95a706f489 100644
--- a/src/jrd/Database.cpp
+++ b/src/jrd/Database.cpp
@@ -613,12 +613,39 @@ namespace Jrd
 	{
 		MutexLockGuard guard(g_mutex, FB_FUNCTION);
 
-		Database::GlobalObjectHolder::DbId* entry = g_hashTable->lookup(id);
+		// Get entry with incrementing ref counter, so if someone is currently destroying it, the object itself
+		// will remain alive.
+		RefPtr<Database::GlobalObjectHolder::DbId> entry(g_hashTable->lookup(id));
+		if (entry)
+		{
+			auto& shutdownMutex = entry->shutdownMutex;
+			// Check if someone else currently destroying GlobalObject.
+			if (shutdownMutex.tryEnter(FB_FUNCTION))
+			{
+				// No one is destroying GlobalObject, continue init routine.
+				shutdownMutex.leave();
+			}
+			else
+			{
+				// Someone is currently destroying GlobalObject, wait until he finish it to eliminate potential
+				// race conditions.
+				{
+					MutexUnlockGuard unlockGuard(g_mutex, FB_FUNCTION);
+
+					MutexLockGuard guard(shutdownMutex, FB_FUNCTION);
+				}
+				// Now we are the one who owned DbId object.
+				// It also was removed from hash table, so simply delete it and recreate it next.
+				fb_assert(entry->holder == nullptr);
+				entry = nullptr;
+			}
+		}
 		if (!entry)
 		{
 			const auto holder = FB_NEW Database::GlobalObjectHolder(id, filename, config);
-			entry = FB_NEW Database::GlobalObjectHolder::DbId(id, holder);
+			entry = makeRef(FB_NEW Database::GlobalObjectHolder::DbId(id, holder));
 			g_hashTable->add(entry);
+			entry->addRef();
 		}
 
 		entry->holder->addRef();
@@ -628,9 +655,18 @@ namespace Jrd
 	Database::GlobalObjectHolder::~GlobalObjectHolder()
 	{
 		// dtor is executed under g_mutex protection
-		Database::GlobalObjectHolder::DbId* entry = g_hashTable->lookup(m_id);
-		if (!g_hashTable->remove(m_id))
-			fb_assert(false);
+
+		// Stole the object from the hash table without incrementing ref counter, so we will be the one who will delete the object
+		// at the end of this function.
+		RefPtr<Database::GlobalObjectHolder::DbId> entry(REF_NO_INCR, g_hashTable->lookup(m_id));
+		fb_assert(entry);
+		fb_assert(entry->holder == this);
+		// We need to unlock the global mutex to safely shutdown some managers, so lock shutdown mutex to make sure that
+		// other threads will wait until we done our shutdown routine.
+		// This is done to eliminate potential race conditions involving global objects, such as shared memory.
+		//! Be careful with order of mutex locking: first - `g_mutex`, second - `shutdownMutex`, but after `MutexUnlockGuard`
+		//! this order will be reversed, so potentially a deadlock situation may occur here.
+		MutexLockGuard guard(entry->shutdownMutex, FB_FUNCTION);
 
 		{ // scope
 			// here we cleanup what should not be globally protected
@@ -643,7 +679,10 @@ namespace Jrd
 		m_eventMgr = nullptr;
 		m_replMgr = nullptr;
 
-		delete entry;
+		if (!g_hashTable->remove(m_id))
+			fb_assert(false);
+
+		entry->holder = nullptr;
 
 		fb_assert(m_tempCacheUsage == 0);
 	}
diff --git a/src/jrd/Database.h b/src/jrd/Database.h
index fac58c0f6b4..ac815b61034 100644
--- a/src/jrd/Database.h
+++ b/src/jrd/Database.h
@@ -260,7 +260,7 @@ class Database : public pool_alloc<type_dbb>
 		typedef Firebird::HashTable<DbId, Firebird::DEFAULT_HASH_SIZE,
 			Firebird::string, DbId, DbId > DbIdHash;
 
-		struct DbId : public DbIdHash::Entry, public Firebird::GlobalStorage
+		struct DbId : public DbIdHash::Entry, public Firebird::GlobalStorage, public Firebird::RefCounted
 		{
 			DbId(const Firebird::string& x, GlobalObjectHolder* h)
 				: id(getPool(), x), holder(h)
@@ -289,7 +289,9 @@ class Database : public pool_alloc<type_dbb>
 			}
 
 			const Firebird::string id;
-			GlobalObjectHolder* const holder;
+			GlobalObjectHolder* holder;
+			// This mutex is working very tight with `g_mutex`, so use it carefully to avoid possible deadlocks.
+			Firebird::Mutex shutdownMutex;
 		};
 
 		static Firebird::GlobalPtr<DbIdHash> g_hashTable;