Skip to content

Commit

Permalink
Fixed #7299: gfix hangs on disconnect when transactions trace active
Browse files Browse the repository at this point in the history 
(cherry picked from commit 1ffe59f)
  • Loading branch information
@AlexPeshkoff
AlexPeshkoff committed Sep 13, 2022
1 parent f4f8ece commit 24bc8ca
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 85 deletions.
82 changes: 77 additions & 5 deletions src/jrd/Database.h
View file
Expand Up @@ -336,8 +336,12 @@ class Database : public pool_alloc<type_dbb>
};

public:
class ExRefMutexUnlock;

class ExistenceRefMutex : public Firebird::RefCounted
{
friend class ExRefMutexUnlock;

public:
ExistenceRefMutex()
: exist(true)
Expand All @@ -347,11 +351,6 @@ class Database : public pool_alloc<type_dbb>
{ }

public:
void destroy()
{
exist = false;
}

bool doesExist() const
{
return exist;
Expand All @@ -368,10 +367,83 @@ class Database : public pool_alloc<type_dbb>
}

private:
void destroy()
{
exist = false;
}

Firebird::Mutex mutex;
bool exist;
};

class ExRefMutexUnlock
{
public:
ExRefMutexUnlock()
: entered(false)
{ }

explicit ExRefMutexUnlock(Database::ExistenceRefMutex* p)
: ref(p), entered(false)
{ }

void enter()
{
fb_assert(ref);
ref->enter();
entered = true;
}

void leave()
{
if (entered)
{
ref->leave();
entered = false;
}
}

void linkWith(Database::ExistenceRefMutex* to)
{
if (ref == to)
return;

leave();
ref = to;
}

void unlinkFromMutex()
{
linkWith(nullptr);
}

void destroy()
{
fb_assert(entered);
ref->destroy();
leave();
}

Database::ExistenceRefMutex* operator->()
{
return ref;
}

bool operator!() const
{
return !ref;
}

~ExRefMutexUnlock()
{
leave();
}

private:
Firebird::RefPtr<ExistenceRefMutex> ref;
bool entered;
};

class Linger FB_FINAL :
public Firebird::RefCntIface<Firebird::ITimerImpl<Linger, Firebird::CheckStatusWrapper> >
{
Expand Down
107 changes: 27 additions & 80 deletions src/jrd/jrd.cpp
View file
Expand Up @@ -504,68 +504,6 @@ namespace
// This mutex protects linked list of databases
GlobalPtr<Mutex> databases_mutex;

// Holder for per-database init/fini mutex
class RefMutexUnlock
{
public:
RefMutexUnlock()
: entered(false)
{ }

explicit RefMutexUnlock(Database::ExistenceRefMutex* p)
: ref(p), entered(false)
{ }

void enter()
{
fb_assert(ref);
ref->enter();
entered = true;
}

void leave()
{
if (entered)
{
ref->leave();
entered = false;
}
}

void linkWith(Database::ExistenceRefMutex* to)
{
if (ref == to)
return;

leave();
ref = to;
}

void unlinkFromMutex()
{
linkWith(NULL);
}

Database::ExistenceRefMutex* operator->()
{
return ref;
}

bool operator!() const
{
return !ref;
}

~RefMutexUnlock()
{
leave();
}

private:
RefPtr<Database::ExistenceRefMutex> ref;
bool entered;
};

// We have 2 more related types of mutexes in database and attachment.
// Attachment is using reference counted mutex in JAtt, also making it possible
// to check does object still exist after locking a mutex. This makes great use when
Expand Down Expand Up @@ -1336,7 +1274,7 @@ static VdnResult verifyDatabaseName(const PathName&, FbStatusVector*, bool);

static void unwindAttach(thread_db* tdbb, const Exception& ex, FbStatusVector* userStatus, bool internalFlag);
static JAttachment* initAttachment(thread_db*, const PathName&, const PathName&, RefPtr<const Config>, bool,
const DatabaseOptions&, RefMutexUnlock&, IPluginConfig*, JProvider*);
const DatabaseOptions&, Database::ExRefMutexUnlock&, IPluginConfig*, JProvider*);
static JAttachment* create_attachment(const PathName&, Database*, JProvider* provider, const DatabaseOptions&, bool newDb);
static void prepare_tra(thread_db*, jrd_tra*, USHORT, const UCHAR*);
static void release_attachment(thread_db*, Attachment*, XThreadEnsureUnlock* = nullptr);
Expand Down Expand Up @@ -1741,7 +1679,7 @@ JAttachment* JProvider::internalAttach(CheckStatusWrapper* user_status, const ch
#endif

// Unless we're already attached, do some initialization
RefMutexUnlock initGuard;
Database::ExRefMutexUnlock initGuard;
JAttachment* jAtt = initAttachment(tdbb, expanded_name,
is_alias ? org_filename : expanded_name,
config, true, options, initGuard, pluginConfig, this);
Expand Down Expand Up @@ -2909,7 +2847,7 @@ JAttachment* JProvider::createDatabase(CheckStatusWrapper* user_status, const ch
#endif

// Unless we're already attached, do some initialization
RefMutexUnlock initGuard;
Database::ExRefMutexUnlock initGuard;
JAttachment* jAtt = initAttachment(tdbb, expanded_name,
is_alias ? org_filename : expanded_name,
config, false, options, initGuard, pluginConfig, this);
Expand Down Expand Up @@ -7233,7 +7171,7 @@ void DatabaseOptions::get(const UCHAR* dpb, USHORT dpb_length, bool& invalid_cli

static JAttachment* initAttachment(thread_db* tdbb, const PathName& expanded_name,
const PathName& alias_name, RefPtr<const Config> config, bool attach_flag,
const DatabaseOptions& options, RefMutexUnlock& initGuard, IPluginConfig* pConf,
const DatabaseOptions& options, Database::ExRefMutexUnlock& initGuard, IPluginConfig* pConf,
JProvider* provider)
{
/**************************************
Expand Down Expand Up @@ -7304,6 +7242,13 @@ static JAttachment* initAttachment(thread_db* tdbb, const PathName& expanded_nam
{
if (attach_flag)
{
if (!dbb->dbb_init_fini->doesExist())
{
// database is shutting down
dbb = dbb->dbb_next;
continue;
}

if (dbb->dbb_flags & DBB_bugcheck)
{
status_exception::raise(Arg::Gds(isc_bug_check) << "can't attach after bugcheck");
Expand Down Expand Up @@ -7824,7 +7769,7 @@ bool JRD_shutdown_database(Database* dbb, const unsigned flags)
**************************************/
ThreadContextHolder tdbb(dbb, NULL);

RefMutexUnlock finiGuard;
Database::ExRefMutexUnlock finiGuard;

{ // scope
fb_assert((flags & SHUT_DBB_OVERWRITE_CHECK) || (!databases_mutex->locked()));
Expand Down Expand Up @@ -7888,7 +7833,7 @@ bool JRD_shutdown_database(Database* dbb, const unsigned flags)

// Deactivate dbb_init_fini lock
// Since that moment dbb becomes not reusable
dbb->dbb_init_fini->destroy();
finiGuard.destroy();

fb_assert(!dbb->locked());

Expand Down Expand Up @@ -8008,22 +7953,24 @@ void JRD_enum_attachments(PathNameList* dbList, ULONG& atts, ULONG& dbs, ULONG&
{
SyncLockGuard dbbGuard(&dbb->dbb_sync, SYNC_SHARED, "JRD_enum_attachments");

if (!(dbb->dbb_flags & DBB_bugcheck))
if (dbb->dbb_flags & DBB_bugcheck)
continue;
if (!dbb->dbb_init_fini->doesExist())
continue;

bool found = false; // look for user attachments only
for (const Jrd::Attachment* attach = dbb->dbb_attachments; attach;
attach = attach->att_next)
{
bool found = false; // look for user attachments only
for (const Jrd::Attachment* attach = dbb->dbb_attachments; attach;
attach = attach->att_next)
if (!(attach->att_flags & ATT_security_db))
{
if (!(attach->att_flags & ATT_security_db))
{
atts++;
found = true;
}
atts++;
found = true;
}

if (found && !dbFiles.exist(dbb->dbb_filename))
dbFiles.add(dbb->dbb_filename);
}

if (found && !dbFiles.exist(dbb->dbb_filename))
dbFiles.add(dbb->dbb_filename);
}

dbs = (ULONG) dbFiles.getCount();
Expand Down

0 comments on commit 24bc8ca

Please sign in to comment.