8
0
mirror of https://github.com/FirebirdSQL/firebird.git synced 2025-01-23 04:43:03 +01:00

Attempted to fix CORE-4235: Deadlock is possible while accessing the monitoring tables under concurrent load.

Refactored the list of reference counted attachments to be useful in other cases.
Some minor adjustments in the monitoring code.
This commit is contained in:
dimitr 2013-09-22 16:10:06 +00:00
parent ddffb4861d
commit c235774864
5 changed files with 204 additions and 132 deletions

View File

@ -581,3 +581,8 @@ void Jrd::Attachment::SyncGuard::init(const char* f, bool optional)
}
}
}
void AttachmentsRefHolder::debugHelper(const char* from)
{
RefDeb(DEB_RLS_JATT, from);
}

View File

@ -35,6 +35,8 @@
#include "../common/classes/ByteChunk.h"
#include "../common/classes/GenericMap.h"
#include "../common/classes/SyncObject.h"
#include "../common/classes/array.h"
#include "../common/classes/stack.h"
#include "../common/classes/timestamp.h"
@ -216,6 +218,27 @@ public:
Firebird::Mutex& m_mutex;
};
class CheckoutSyncGuard
{
public:
CheckoutSyncGuard(Attachment* att, Firebird::SyncObject& sync, Firebird::SyncType type, const char* f)
: m_sync(&sync, f)
{
if (!m_sync.lockConditional(type, f))
{
Checkout attCout(att, f);
m_sync.lock(type);
}
}
private:
// copying is prohibited
CheckoutSyncGuard(const CheckoutSyncGuard&);
CheckoutSyncGuard& operator=(const CheckoutSyncGuard&);
Firebird::Sync m_sync;
};
public:
static Attachment* create(Database* dbb);
static void destroy(Attachment* const attachment);
@ -387,6 +410,97 @@ inline void Attachment::setSysTransaction(jrd_tra* trans)
att_sys_transaction = trans;
}
// This class holds references to all attachments it contains
class AttachmentsRefHolder
{
friend class Iterator;
public:
class Iterator
{
public:
explicit Iterator(AttachmentsRefHolder& list)
: m_list(list), m_index(0)
{}
JAttachment* operator*()
{
if (m_index < m_list.m_attachments.getCount())
return m_list.m_attachments[m_index];
return NULL;
}
void operator++()
{
m_index++;
}
void remove()
{
if (m_index < m_list.m_attachments.getCount())
{
AttachmentsRefHolder::debugHelper(FB_FUNCTION);
m_list.m_attachments[m_index]->release();
m_list.m_attachments.remove(m_index);
}
}
private:
// copying is prohibited
Iterator(const Iterator&);
Iterator& operator=(const Iterator&);
AttachmentsRefHolder& m_list;
size_t m_index;
};
explicit AttachmentsRefHolder(MemoryPool& p)
: m_attachments(p)
{}
AttachmentsRefHolder& operator=(const AttachmentsRefHolder& other)
{
this->~AttachmentsRefHolder();
for (size_t i = 0; i < other.m_attachments.getCount(); i++)
add(other.m_attachments[i]);
return *this;
}
~AttachmentsRefHolder()
{
while (m_attachments.hasData())
{
debugHelper(FB_FUNCTION);
m_attachments.pop()->release();
}
}
void add(JAttachment* jAtt)
{
if (jAtt)
{
jAtt->addRef();
m_attachments.add(jAtt);
}
}
void remove(Iterator& iter)
{
iter.remove();
}
private:
AttachmentsRefHolder(const AttachmentsRefHolder&);
static void debugHelper(const char* from);
Firebird::HalfStaticArray<JAttachment*, 128> m_attachments;
};
} // namespace Jrd
#endif // JRD_ATTACHMENT_H

View File

@ -355,7 +355,7 @@ int DatabaseSnapshot::blockingAst(void* ast_object)
if (!(dbb->dbb_ast_flags & DBB_monitor_off))
{
SyncLockGuard monGuard(&dbb->dbb_mon_sync, SYNC_EXCLUSIVE, "DatabaseSnapshot::blockingAst");
SyncLockGuard monGuard(&dbb->dbb_mon_sync, SYNC_EXCLUSIVE, FB_FUNCTION);
if (!(dbb->dbb_ast_flags & DBB_monitor_off))
{
@ -364,7 +364,7 @@ int DatabaseSnapshot::blockingAst(void* ast_object)
// Write the data to the shared memory
try
{
dumpData(tdbb);
dumpData(dbb, backup_state_unknown);
}
catch (const Exception& ex)
{
@ -394,6 +394,9 @@ DatabaseSnapshot::DatabaseSnapshot(thread_db* tdbb, MemoryPool& pool)
Database* const dbb = tdbb->getDatabase();
fb_assert(dbb);
Attachment* const attachment = tdbb->getAttachment();
fb_assert(attachment);
// Initialize record buffers
RecordBuffer* const dbb_buffer = allocBuffer(tdbb, pool, rel_mon_database);
RecordBuffer* const att_buffer = allocBuffer(tdbb, pool, rel_mon_attachments);
@ -405,21 +408,38 @@ DatabaseSnapshot::DatabaseSnapshot(thread_db* tdbb, MemoryPool& pool)
RecordBuffer* const ctx_var_buffer = allocBuffer(tdbb, pool, rel_mon_ctx_vars);
RecordBuffer* const mem_usage_buffer = allocBuffer(tdbb, pool, rel_mon_mem_usage);
{
SyncLockGuard monGuard(&dbb->dbb_mon_sync, SYNC_EXCLUSIVE, "DatabaseSnapshot::DatabaseSnapshot");
// Determine the backup state
int backup_state = backup_state_unknown;
// Release our own lock.
BackupManager* const bm = dbb->dbb_backup_manager;
if (bm && !bm->isShutDown())
{
BackupManager::StateReadGuard holder(tdbb);
switch (bm->getState())
{
case nbak_state_normal:
backup_state = backup_state_normal;
break;
case nbak_state_stalled:
backup_state = backup_state_stalled;
break;
case nbak_state_merge:
backup_state = backup_state_merge;
break;
}
}
{ // scope
Attachment::Checkout cout(attachment, FB_FUNCTION);
SyncLockGuard monGuard(&dbb->dbb_mon_sync, SYNC_EXCLUSIVE, FB_FUNCTION);
// Release our own lock
LCK_release(tdbb, dbb->dbb_monitor_lock);
dbb->dbb_ast_flags &= ~DBB_monitor_off;
{ // scope for the RAII object
// Ensure we'll be dealing with a valid backup state inside the call below.
BackupManager::StateReadGuard holder(tdbb);
// Dump our own data
dumpData(tdbb);
}
// Dump our own data
dumpData(dbb, backup_state);
}
// Signal other processes to dump their data
@ -446,9 +466,6 @@ DatabaseSnapshot::DatabaseSnapshot(thread_db* tdbb, MemoryPool& pool)
Reader reader(dataSize, data);
const Attachment* const attachment = tdbb->getAttachment();
fb_assert(attachment);
string databaseName(dbb->dbb_database_name.c_str());
ISC_systemToUtf8(databaseName);
@ -713,13 +730,8 @@ void DataDump::putField(thread_db* tdbb, Record* record, const DumpField& field,
}
void DatabaseSnapshot::dumpData(thread_db* tdbb)
void DatabaseSnapshot::dumpData(Database* dbb, int backup_state)
{
fb_assert(tdbb);
Database* const dbb = tdbb->getDatabase();
fb_assert(dbb);
MemoryPool& pool = *dbb->dbb_permanent;
if (!dbb->dbb_monitoring_data)
@ -733,54 +745,47 @@ void DatabaseSnapshot::dumpData(thread_db* tdbb)
// Database information
putDatabase(record, dbb, writer, fb_utils::genUniqueId());
putDatabase(record, dbb, writer, fb_utils::genUniqueId(), backup_state);
// Attachment information
Attachment* const self_attachment = tdbb->getAttachment();
AttachmentsRefHolder attachments(pool);
if (self_attachment)
dumpAttachment(record, self_attachment, writer);
try
{
Attachment::Checkout attCout(self_attachment, FB_FUNCTION, true);
{ // scope
SyncLockGuard attGuard(&dbb->dbb_sync, SYNC_SHARED, FB_FUNCTION);
for (Attachment* attachment = dbb->dbb_attachments; attachment;
attachment = attachment->att_next)
{
if (attachment != self_attachment)
{
Attachment::SyncGuard attGuard(attachment, FB_FUNCTION);
dumpAttachment(record, attachment, writer);
}
attachments.add(attachment->att_interface);
}
{ // scope
SyncLockGuard guard(&dbb->dbb_sys_attach, SYNC_SHARED, FB_FUNCTION);
for (Attachment* attachment = dbb->dbb_sys_attachments; attachment;
attachment = attachment->att_next)
{
Attachment::SyncGuard attGuard(attachment, FB_FUNCTION);
dumpAttachment(record, attachment, writer);
}
}
tdbb->setAttachment(self_attachment);
}
catch(const Exception&)
{ // scope
SyncLockGuard sysAttGuard(&dbb->dbb_sys_attach, SYNC_SHARED, FB_FUNCTION);
for (Attachment* attachment = dbb->dbb_sys_attachments; attachment;
attachment = attachment->att_next)
{
attachments.add(attachment->att_interface);
}
}
for (AttachmentsRefHolder::Iterator iter(attachments); *iter; ++iter)
{
tdbb->setAttachment(self_attachment);
throw;
JAttachment* const jAtt = *iter;
Attachment::SyncGuard attGuard(jAtt, FB_FUNCTION);
Attachment* const attachment = jAtt->getHandle();
if (attachment && attachment->att_user)
dumpAttachment(record, attachment, writer);
}
}
void DatabaseSnapshot::dumpAttachment(DumpRecord& record, const Attachment* attachment,
Writer& writer)
{
if (!putAttachment(record, attachment, writer, fb_utils::genUniqueId()))
return;
putAttachment(record, attachment, writer, fb_utils::genUniqueId());
jrd_tra* transaction = NULL;
jrd_req* request = NULL;
@ -837,7 +842,7 @@ SINT64 DatabaseSnapshot::getGlobalId(int value)
void DatabaseSnapshot::putDatabase(DumpRecord& record, const Database* database,
Writer& writer, int stat_id)
Writer& writer, int stat_id, int backup_state)
{
fb_assert(database);
@ -873,21 +878,13 @@ void DatabaseSnapshot::putDatabase(DumpRecord& record, const Database* database,
// shutdown mode
if (database->dbb_ast_flags & DBB_shutdown_full)
{
temp = shut_mode_full;
}
else if (database->dbb_ast_flags & DBB_shutdown_single)
{
temp = shut_mode_single;
}
else if (database->dbb_ast_flags & DBB_shutdown)
{
temp = shut_mode_multi;
}
else
{
temp = shut_mode_online;
}
record.storeInteger(f_mon_db_shut_mode, temp);
// sweep interval
@ -905,25 +902,8 @@ void DatabaseSnapshot::putDatabase(DumpRecord& record, const Database* database,
record.storeTimestamp(f_mon_db_created, database->dbb_creation_date);
// database size
record.storeInteger(f_mon_db_pages, PageSpace::actAlloc(database));
// database state
temp = backup_state_unknown;
if (database->dbb_backup_manager)
{
switch (database->dbb_backup_manager->getState())
{
case nbak_state_normal:
temp = backup_state_normal;
break;
case nbak_state_stalled:
temp = backup_state_stalled;
break;
case nbak_state_merge:
temp = backup_state_merge;
break;
}
}
record.storeInteger(f_mon_db_backup_state, temp);
// database backup state
record.storeInteger(f_mon_db_backup_state, backup_state);
// crypt thread status
if (database->dbb_crypto_manager)
@ -951,13 +931,10 @@ void DatabaseSnapshot::putDatabase(DumpRecord& record, const Database* database,
}
bool DatabaseSnapshot::putAttachment(DumpRecord& record, const Jrd::Attachment* attachment,
void DatabaseSnapshot::putAttachment(DumpRecord& record, const Jrd::Attachment* attachment,
Writer& writer, int stat_id)
{
fb_assert(attachment);
if (!attachment->att_user)
return false;
fb_assert(attachment && attachment->att_user);
record.reset(rel_mon_attachments);
@ -1034,8 +1011,6 @@ bool DatabaseSnapshot::putAttachment(DumpRecord& record, const Jrd::Attachment*
// context vars
putContextVars(record, attachment->att_context_vars, writer, attachment->att_attachment_id, true);
return true;
}

View File

@ -345,13 +345,13 @@ protected:
private:
RecordBuffer* allocBuffer(thread_db*, MemoryPool&, int);
static void dumpData(thread_db*);
static void dumpData(Database*, int);
static void dumpAttachment(DumpRecord&, const Attachment*, Writer&);
static SINT64 getGlobalId(int);
static void putDatabase(DumpRecord&, const Database*, Writer&, int);
static bool putAttachment(DumpRecord&, const Attachment*, Writer&, int);
static void putDatabase(DumpRecord&, const Database*, Writer&, int, int);
static void putAttachment(DumpRecord&, const Attachment*, Writer&, int);
static void putTransaction(DumpRecord&, const jrd_tra*, Writer&, int);
static void putRequest(DumpRecord&, const jrd_req*, Writer&, int);
static void putCall(DumpRecord&, const jrd_req*, Writer&, int);

View File

@ -5130,11 +5130,11 @@ bool JRD_reschedule(thread_db* tdbb, SLONG quantum, bool punt)
*
**************************************/
Database* const dbb = tdbb->getDatabase();
Jrd::Attachment* const att = tdbb->getAttachment();
Jrd::Attachment* const attachment = tdbb->getAttachment();
if (!tdbb->checkCancelState(false))
{
Jrd::Attachment::Checkout cout(att, FB_FUNCTION);
Jrd::Attachment::Checkout cout(attachment, FB_FUNCTION);
THREAD_YIELD();
}
@ -5163,7 +5163,9 @@ bool JRD_reschedule(thread_db* tdbb, SLONG quantum, bool punt)
// Enable signal handler for the monitoring stuff
if (dbb->dbb_ast_flags & DBB_monitor_off)
{
SyncLockGuard monGuard(&dbb->dbb_mon_sync, SYNC_EXCLUSIVE, "JRD_reschedule");
Attachment::CheckoutSyncGuard monGuard(attachment, dbb->dbb_mon_sync,
SYNC_EXCLUSIVE, FB_FUNCTION);
if (dbb->dbb_ast_flags & DBB_monitor_off)
{
dbb->dbb_ast_flags &= ~DBB_monitor_off;
@ -5280,7 +5282,8 @@ static void check_database(thread_db* tdbb, bool async)
if (dbb->dbb_ast_flags & DBB_monitor_off)
{
SyncLockGuard monGuard(&dbb->dbb_mon_sync, SYNC_EXCLUSIVE, "check_database");
Attachment::CheckoutSyncGuard monGuard(attachment, dbb->dbb_mon_sync,
SYNC_EXCLUSIVE, FB_FUNCTION);
if (dbb->dbb_ast_flags & DBB_monitor_off)
{
@ -7091,35 +7094,16 @@ static void unwindAttach(thread_db* tdbb, const Exception& ex, IStatus* userStat
namespace
{
class AttQueue : public HalfStaticArray<JAttachment*, 128>
bool shutdownAttachments(AttachmentsRefHolder* arg)
{
public:
explicit AttQueue(MemoryPool& p)
: HalfStaticArray<JAttachment*, 128>(p)
{ }
~AttQueue()
{
RefDeb(DEB_RLS_JATT, FB_FUNCTION);
// Release interfaces
while (hasData())
{
pop()->release();
}
}
};
bool shutdownAttachments(AttQueue* arg)
{
AutoPtr<AttQueue> queue(arg);
AttQueue& attachments = *arg;
AutoPtr<AttachmentsRefHolder> queue(arg);
AttachmentsRefHolder& attachments = *arg;
bool success = true;
// Set terminate flag for all attachments
unsigned i;
for (i = 0; i < attachments.getCount(); ++i)
for (AttachmentsRefHolder::Iterator iter(attachments); *iter; ++iter)
{
JAttachment* jAtt = attachments[i];
JAttachment* const jAtt = *iter;
MutexLockGuard guard(*(jAtt->getMutex(true)), FB_FUNCTION);
Attachment* attachment = jAtt->getHandle();
@ -7129,9 +7113,9 @@ namespace
}
// Purge all attachments
for (i = 0; i < attachments.getCount(); ++i)
for (AttachmentsRefHolder::Iterator iter(attachments); *iter; ++iter)
{
JAttachment* jAtt = attachments[i];
JAttachment* const jAtt = *iter;
MutexLockGuard guard(*(jAtt->getMutex()), FB_FUNCTION);
Attachment* attachment = jAtt->getHandle();
@ -7175,7 +7159,7 @@ namespace
return 0;
}
shutdownAttachments(static_cast<AttQueue*>(arg));
shutdownAttachments(static_cast<AttachmentsRefHolder*>(arg));
}
catch(const Exception& ex)
{
@ -7205,7 +7189,7 @@ static THREAD_ENTRY_DECLARE shutdown_thread(THREAD_ENTRY_PARAM arg)
bool success = true;
MemoryPool& pool = *getDefaultMemoryPool();
AttQueue* attachments = FB_NEW(pool) AttQueue(pool);
AttachmentsRefHolder* const attachments = FB_NEW(pool) AttachmentsRefHolder(pool);
try
{
@ -7222,14 +7206,8 @@ static THREAD_ENTRY_DECLARE shutdown_thread(THREAD_ENTRY_PARAM arg)
Sync dbbGuard(&dbb->dbb_sync, FB_FUNCTION);
dbbGuard.lock(SYNC_EXCLUSIVE);
for (Attachment* att = dbb->dbb_attachments; att; att = att->att_next)
{
if (att->att_interface)
{
att->att_interface->addRef();
attachments->push(att->att_interface);
}
}
for (const Attachment* att = dbb->dbb_attachments; att; att = att->att_next)
attachments->add(att->att_interface);
}
}
// No need in databases_mutex any more
@ -7778,7 +7756,7 @@ void JRD_shutdown_attachments(Database* dbb)
try
{
MemoryPool& pool = *getDefaultMemoryPool();
AttQueue* queue = FB_NEW(pool) AttQueue(pool);
AttachmentsRefHolder* queue = FB_NEW(pool) AttachmentsRefHolder(pool);
{ // scope
Sync guard(&dbb->dbb_sync, "JRD_shutdown_attachments");