Skip to content

Commit 18e55c4

Browse files
author
Artyom Ivanov
committed
fix(shutdown): Prevent race condition when ~GlobalObjectHolder() 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.
1 parent 581f0ea commit 18e55c4

File tree

2 files changed

+16
-0
lines changed

2 files changed

+16
-0
lines changed

src/jrd/Database.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,15 @@ namespace Jrd
586586
{
587587
MutexLockGuard guard(g_mutex, FB_FUNCTION);
588588

589+
while (g_shuttingDown)
590+
{
591+
// Currently other GlobalObject is in shutdown process but he unlock the mutex for some reason,
592+
// so it's better to wait until this process is over.
593+
// This is done to eliminate potential race conditions involving global objects, such as shared memory.
594+
MutexUnlockGuard unlockGuard(g_mutex, FB_FUNCTION);
595+
Thread::yield();
596+
}
597+
589598
Database::GlobalObjectHolder::DbId* entry = g_hashTable->lookup(id);
590599
if (!entry)
591600
{
@@ -606,6 +615,11 @@ namespace Jrd
606615
fb_assert(false);
607616

608617
{ // scope
618+
// We need to unlock mutex to safely shutdown some managers, so set shutdown flag to make sure that
619+
// other threads will wait until we done our shutdown routine.
620+
// This is done to eliminate potential race conditions involving global objects, such as shared memory.
621+
AutoSetRestore shuttingDownGuard(&g_shuttingDown, true);
622+
609623
// here we cleanup what should not be globally protected
610624
MutexUnlockGuard guard(g_mutex, FB_FUNCTION);
611625
if (m_replMgr)

src/jrd/Database.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,8 @@ class Database : public pool_alloc<type_dbb>
290290

291291
static Firebird::GlobalPtr<DbIdHash> g_hashTable;
292292
static Firebird::GlobalPtr<Firebird::Mutex> g_mutex;
293+
// It doesn't need to be atomic because all accesses is done under the mutex
294+
static inline bool g_shuttingDown = false;
293295

294296
public:
295297
static GlobalObjectHolder* init(const Firebird::string& id,

0 commit comments

Comments
 (0)