fix(shutdown): Reimplement synchronization between shutdown and init routine for GlobalObjectHolder

Author: Artyom Ivanov

src/common/classes/RefCounted.h

@@ -48,6 +48,11 @@ namespace Firebird
 			return refCnt;
 		}
 
+		AtomicCounter::counter_type getRefCount() const noexcept
+		{
+			return m_refCnt.value();
+		}
+
 		void assertNonZero()
 		{
 			fb_assert(m_refCnt.value() > 0);

src/jrd/Database.cpp

@@ -586,21 +586,39 @@ namespace Jrd
 	{
 		MutexLockGuard guard(g_mutex, FB_FUNCTION);
 
-		while (g_shuttingDown)
+		// 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)
 		{
-			// Currently other GlobalObject is in shutdown process but he unlock the mutex for some reason,
-			// so it's better to wait until this process is over.
-			// This is done to eliminate potential race conditions involving global objects, such as shared memory.
-			MutexUnlockGuard unlockGuard(g_mutex, FB_FUNCTION);
-			Thread::yield();
-		}
+			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);
 
-		Database::GlobalObjectHolder::DbId* entry = g_hashTable->lookup(id);
+					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->getRefCount() == 1);
+				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();
@@ -610,16 +628,17 @@ 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);
 
-		{ // scope
-			// We need to unlock mutex to safely shutdown some managers, so set shutdown flag 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.
-			AutoSetRestore shuttingDownGuard(&g_shuttingDown, true);
+		// 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);
+		// 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.
+		MutexLockGuard guard(entry->shutdownMutex, FB_FUNCTION);
 
+		{ // scope
 			// here we cleanup what should not be globally protected
 			MutexUnlockGuard guard(g_mutex, FB_FUNCTION);
 			if (m_replMgr)
@@ -630,7 +649,8 @@ namespace Jrd
 		m_eventMgr = nullptr;
 		m_replMgr = nullptr;
 
-		delete entry;
+		if (!g_hashTable->remove(m_id))
+			fb_assert(false);
 
 		fb_assert(m_tempCacheUsage == 0);
 	}

src/jrd/Database.h

@@ -256,7 +256,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)
@@ -286,6 +286,8 @@ class Database : public pool_alloc<type_dbb>
 
 			const Firebird::string id;
 			GlobalObjectHolder* const holder;
+
+			Firebird::Mutex shutdownMutex;
 		};
 
 		static Firebird::GlobalPtr<DbIdHash> g_hashTable;