From 475f87368205ecee6d4fdcd389a2d9042b100b95 Mon Sep 17 00:00:00 2001 From: dimitr Date: Mon, 8 Nov 2010 10:10:29 +0000 Subject: [PATCH] Fixed CORE-3217: Server crashes inside the lock manager when multiple connections attaching/detaching simultaneously. --- src/jrd/Database.cpp | 3 ++- src/jrd/Database.h | 4 ++-- src/jrd/event.cpp | 41 ++++++++++++++++++++++++++--------------- src/jrd/event_proto.h | 3 ++- src/lock/lock.cpp | 39 +++++++++++++++++++++++++-------------- src/lock/lock_proto.h | 3 ++- 6 files changed, 59 insertions(+), 34 deletions(-) diff --git a/src/jrd/Database.cpp b/src/jrd/Database.cpp index 8f100d14c1..5f7428a6dd 100644 --- a/src/jrd/Database.cpp +++ b/src/jrd/Database.cpp @@ -93,7 +93,8 @@ namespace Jrd Checkout dcoHolder(this); // This line decrements the usage counter and may cause the destructor to be called. // It should happen with the dbb_sync unlocked. - dbb_lock_mgr = NULL; + LockManager::destroy(dbb_lock_mgr); + EventManager::destroy(dbb_event_mgr); } void Database::deletePool(MemoryPool* pool) diff --git a/src/jrd/Database.h b/src/jrd/Database.h index 0d7b2a7391..0aa6045c7c 100644 --- a/src/jrd/Database.h +++ b/src/jrd/Database.h @@ -330,8 +330,8 @@ public: mutable Firebird::RefPtr dbb_sync; // Database sync primitive - Firebird::RefPtr dbb_lock_mgr; - Firebird::RefPtr dbb_event_mgr; + LockManager* dbb_lock_mgr; + EventManager* dbb_event_mgr; Database* dbb_next; // Next database block in system Attachment* dbb_attachments; // Active attachments diff --git a/src/jrd/event.cpp b/src/jrd/event.cpp index 17a22a8f9f..1b53f22913 100644 --- a/src/jrd/event.cpp +++ b/src/jrd/event.cpp @@ -76,7 +76,7 @@ Firebird::GlobalPtr EventManager::g_mapMutex; void EventManager::init(Attachment* attachment) { - Database* dbb = attachment->att_database; + Database* const dbb = attachment->att_database; EventManager* eventMgr = dbb->dbb_event_mgr; if (!eventMgr) { @@ -87,10 +87,16 @@ void EventManager::init(Attachment* attachment) if (!g_emMap->get(id, eventMgr)) { eventMgr = new EventManager(id); + + if (g_emMap->put(id, eventMgr)) + { + fb_assert(false); + } } fb_assert(eventMgr); + eventMgr->addRef(); dbb->dbb_event_mgr = eventMgr; } @@ -101,6 +107,25 @@ void EventManager::init(Attachment* attachment) } +void EventManager::destroy(EventManager* eventMgr) +{ + if (eventMgr) + { + const Firebird::string id = eventMgr->m_dbId; + + Firebird::MutexLockGuard guard(g_mapMutex); + + if (!eventMgr->release()) + { + if (!g_emMap->remove(id)) + { + fb_assert(false); + } + } + } +} + + EventManager::EventManager(const Firebird::string& id) : PID(getpid()), m_header(NULL), @@ -111,13 +136,6 @@ EventManager::EventManager(const Firebird::string& id) m_exiting(false) { attach_shared_file(); - - Firebird::MutexLockGuard guard(g_mapMutex); - - if (g_emMap->put(m_dbId, this)) - { - fb_assert(false); - } } @@ -157,13 +175,6 @@ EventManager::~EventManager() release_shmem(); detach_shared_file(); - - Firebird::MutexLockGuard guard(g_mapMutex); - - if (!g_emMap->remove(m_dbId)) - { - fb_assert(false); - } } diff --git a/src/jrd/event_proto.h b/src/jrd/event_proto.h index 266f60a893..38e3db39d8 100644 --- a/src/jrd/event_proto.h +++ b/src/jrd/event_proto.h @@ -35,7 +35,7 @@ namespace Jrd { class Attachment; -class EventManager : public Firebird::RefCounted, public Firebird::GlobalStorage +class EventManager : private Firebird::RefCounted, public Firebird::GlobalStorage { typedef Firebird::GenericMap > > DbEventMgrMap; @@ -46,6 +46,7 @@ class EventManager : public Firebird::RefCounted, public Firebird::GlobalStorage public: static void init(Attachment*); + static void destroy(EventManager*); explicit EventManager(const Firebird::string&); ~EventManager(); diff --git a/src/lock/lock.cpp b/src/lock/lock.cpp index e430f0edf2..ba9753ef29 100644 --- a/src/lock/lock.cpp +++ b/src/lock/lock.cpp @@ -176,14 +176,39 @@ LockManager* LockManager::create(const Firebird::string& id) if (!g_lmMap->get(id, lockMgr)) { lockMgr = new LockManager(id); + + if (g_lmMap->put(id, lockMgr)) + { + fb_assert(false); + } } fb_assert(lockMgr); + lockMgr->addRef(); return lockMgr; } +void LockManager::destroy(LockManager* lockMgr) +{ + if (lockMgr) + { + const Firebird::string id = lockMgr->m_dbId; + + Firebird::MutexLockGuard guard(g_mapMutex); + + if (!lockMgr->release()) + { + if (!g_lmMap->remove(id)) + { + fb_assert(false); + } + } + } +} + + LockManager::LockManager(const Firebird::string& id) : PID(getpid()), m_bugcheck(false), @@ -203,13 +228,6 @@ LockManager::LockManager(const Firebird::string& id) { Firebird::status_exception::raise(local_status); } - - Firebird::MutexLockGuard guard(g_mapMutex); - - if (g_lmMap->put(m_dbId, this)) - { - fb_assert(false); - } } @@ -271,13 +289,6 @@ LockManager::~LockManager() ISC_unmap_file(local_status, &m_extents[i].sh_data); } #endif //USE_SHMEM_EXT - - Firebird::MutexLockGuard guard(g_mapMutex); - - if (!g_lmMap->remove(m_dbId)) - { - fb_assert(false); - } } diff --git a/src/lock/lock_proto.h b/src/lock/lock_proto.h index fd977895f0..22dff59610 100644 --- a/src/lock/lock_proto.h +++ b/src/lock/lock_proto.h @@ -310,7 +310,7 @@ namespace Jrd { class thread_db; -class LockManager : public Firebird::RefCounted, public Firebird::GlobalStorage +class LockManager : private Firebird::RefCounted, public Firebird::GlobalStorage { typedef Firebird::GenericMap > > DbLockMgrMap; @@ -321,6 +321,7 @@ class LockManager : public Firebird::RefCounted, public Firebird::GlobalStorage public: static LockManager* create(const Firebird::string&); + static void destroy(LockManager*); bool initializeOwner(thread_db*, LOCK_OWNER_T, UCHAR, SRQ_PTR*); void shutdownOwner(thread_db*, SRQ_PTR*);