From 6f17277a958128bfda18723efaecd0e9320a02b1 Mon Sep 17 00:00:00 2001 From: Vlad Khorsun Date: Fri, 17 Jan 2025 11:24:12 +0200 Subject: [PATCH] Fixed bug #8390 : Deadlock might happens when database is shutting down with internal worker attachments exists. --- src/jrd/WorkerAttachment.cpp | 57 ++++++++++++++++++++++++++++-------- src/jrd/WorkerAttachment.h | 28 +++++++++++++++--- 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/src/jrd/WorkerAttachment.cpp b/src/jrd/WorkerAttachment.cpp index 65798c4277..969f54a38e 100644 --- a/src/jrd/WorkerAttachment.cpp +++ b/src/jrd/WorkerAttachment.cpp @@ -49,8 +49,10 @@ const unsigned WORKER_IDLE_TIMEOUT = 60; // 1 minute /// class WorkerStableAttachment -WorkerStableAttachment::WorkerStableAttachment(FbStatusVector* status, Jrd::Attachment* attachment) : - SysStableAttachment(attachment) +WorkerStableAttachment::WorkerStableAttachment(FbStatusVector* status, Jrd::Attachment* attachment, + WorkerAttachment* workers) : + SysStableAttachment(attachment), + m_workers(workers) { UserId user; user.setUserName(""); @@ -69,6 +71,7 @@ WorkerStableAttachment::WorkerStableAttachment(FbStatusVector* status, Jrd::Atta Monitoring::publishAttachment(tdbb); initDone(); + m_workers->incWorkers(); } WorkerStableAttachment::~WorkerStableAttachment() @@ -76,7 +79,8 @@ WorkerStableAttachment::~WorkerStableAttachment() fini(); } -WorkerStableAttachment* WorkerStableAttachment::create(FbStatusVector* status, Database* dbb, JProvider* provider) +WorkerStableAttachment* WorkerStableAttachment::create(FbStatusVector* status, Database* dbb, + JProvider* provider, WorkerAttachment* workers) { Attachment* attachment = NULL; try @@ -85,7 +89,7 @@ WorkerStableAttachment* WorkerStableAttachment::create(FbStatusVector* status, D attachment->att_filename = dbb->dbb_filename; attachment->att_flags |= ATT_worker; - WorkerStableAttachment* sAtt = FB_NEW WorkerStableAttachment(status, attachment); + WorkerStableAttachment* sAtt = FB_NEW WorkerStableAttachment(status, attachment, workers); return sAtt; } catch (const Exception& ex) @@ -129,6 +133,8 @@ void WorkerStableAttachment::fini() } destroy(attachment); + + m_workers->decWorkers(); } /// class WorkerAttachment @@ -139,8 +145,7 @@ bool WorkerAttachment::m_shutdown = false; WorkerAttachment::WorkerAttachment() : m_idleAtts(*getDefaultMemoryPool()), - m_activeAtts(*getDefaultMemoryPool()), - m_cntUserAtts(0) + m_activeAtts(*getDefaultMemoryPool()) { } @@ -177,6 +182,30 @@ void WorkerAttachment::decUserAtts(const PathName& dbname) } } +void WorkerAttachment::incWorkers() +{ + fb_assert(Config::getServerMode() == MODE_SUPER); + + MutexLockGuard guard(m_mutex, FB_FUNCTION); + ++m_cntWorkers; +} + +void WorkerAttachment::decWorkers() +{ + fb_assert(Config::getServerMode() == MODE_SUPER); + + MutexLockGuard guard(m_mutex, FB_FUNCTION); + if (--m_cntWorkers == 0) + m_noWorkers.notifyAll(); +} + +void WorkerAttachment::waitForWorkers() +{ + MutexLockGuard guard(m_mutex, FB_FUNCTION); + while (m_cntWorkers != 0) + m_noWorkers.wait(m_mutex); +} + WorkerAttachment* WorkerAttachment::getByName(const PathName& dbname) { if (m_shutdown) @@ -230,13 +259,17 @@ void WorkerAttachment::shutdownDbb(Database* dbb) if (Config::getServerMode() != MODE_SUPER) return; - MutexLockGuard guard(m_mapMutex, FB_FUNCTION); - WorkerAttachment* item = NULL; - if (!m_map->get(dbb->dbb_filename, item)) - return; + + { + MutexLockGuard guard(m_mapMutex, FB_FUNCTION); + + if (!m_map->get(dbb->dbb_filename, item)) + return; + } item->clear(false); + item->waitForWorkers(); } StableAttachmentPart* WorkerAttachment::getAttachment(FbStatusVector* status, Database* dbb) @@ -301,7 +334,7 @@ StableAttachmentPart* WorkerAttachment::getAttachment(FbStatusVector* status, Da MutexUnlockGuard unlock(item->m_mutex, FB_FUNCTION); status->init(); - sAtt = doAttach(status, dbb); + sAtt = item->doAttach(status, dbb); if (!sAtt) { // log error ? @@ -439,7 +472,7 @@ StableAttachmentPart* WorkerAttachment::doAttach(FbStatusVector* status, Databas //jInstance->setDbCryptCallback(&status, tdbb->getAttachment()->att_crypt_callback); if (Config::getServerMode() == MODE_SUPER) - sAtt = WorkerStableAttachment::create(status, dbb, jInstance); + sAtt = WorkerStableAttachment::create(status, dbb, jInstance, this); else { ClumpletWriter dpb(ClumpletReader::Tagged, MAX_DPB_SIZE, isc_dpb_version1); diff --git a/src/jrd/WorkerAttachment.h b/src/jrd/WorkerAttachment.h index dd366573f1..3111cd322b 100644 --- a/src/jrd/WorkerAttachment.h +++ b/src/jrd/WorkerAttachment.h @@ -31,6 +31,7 @@ #include "firebird.h" #include "../common/classes/alloc.h" #include "../common/classes/array.h" +#include "../common/classes/condition.h" #include "../common/classes/fb_string.h" #include "../common/classes/GenericMap.h" #include "../common/classes/init.h" @@ -43,10 +44,13 @@ namespace Jrd { +class WorkerAttachment; + class WorkerStableAttachment : public SysStableAttachment { public: - static WorkerStableAttachment* create(FbStatusVector* status, Database* dbb, JProvider* provider); + static WorkerStableAttachment* create(FbStatusVector* status, Database* dbb, JProvider* provider, + WorkerAttachment* workers); void fini(); @@ -54,8 +58,10 @@ protected: virtual void doOnIdleTimer(Firebird::TimerImpl* timer); private: - explicit WorkerStableAttachment(FbStatusVector* status, Jrd::Attachment* att); + explicit WorkerStableAttachment(FbStatusVector* status, Jrd::Attachment* att, WorkerAttachment* workers); virtual ~WorkerStableAttachment(); + + WorkerAttachment* m_workers; }; @@ -77,6 +83,8 @@ private: class WorkerAttachment { + friend class WorkerStableAttachment; + public: explicit WorkerAttachment(); @@ -93,10 +101,14 @@ public: private: static WorkerAttachment* getByName(const Firebird::PathName& dbname); - static Jrd::StableAttachmentPart* doAttach(FbStatusVector* status, Jrd::Database* dbb); + Jrd::StableAttachmentPart* doAttach(FbStatusVector* status, Jrd::Database* dbb); static void doDetach(FbStatusVector* status, Jrd::StableAttachmentPart* sAtt); void clear(bool checkRefs); + void incWorkers(); + void decWorkers(); + void waitForWorkers(); + typedef Firebird::GenericMap > > MapDbIdToWorkAtts; @@ -109,7 +121,15 @@ private: Firebird::HalfStaticArray m_idleAtts; Firebird::SortedArray > m_activeAtts; - int m_cntUserAtts; + + // count of regular user attachments, used with non-shared Database + int m_cntUserAtts = 0; + + // count of internal worker attachments, used with shared Database + int m_cntWorkers = 0; + + // used to wait for "no internal workers" condition + Firebird::Condition m_noWorkers; }; } // namespace Jrd