Skip to content

Commit

Permalink
Fix for #7556: FB Classic can hang when attempts to attach DB while i…
Browse files Browse the repository at this point in the history 
…t is starting to encrypt/decrypt

(cherry picked from commit 3018b81)
  • Loading branch information
@AlexPeshkoff
AlexPeshkoff committed May 17, 2023
1 parent 596ff4d commit a0cddf8
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 42 deletions.
22 changes: 11 additions & 11 deletions src/jrd/Attachment.h
View file
Expand Up @@ -198,12 +198,10 @@ class StableAttachmentPart : public Firebird::RefCounted, public Firebird::Globa
return totalLocksCounter;
}

#ifdef DEV_BUILD
bool locked() const
{
return threadId == getThreadId();
}
#endif

~Sync()
{
Expand All @@ -225,8 +223,6 @@ class StableAttachmentPart : public Firebird::RefCounted, public Firebird::Globa
int currentLocksCounter;
};

typedef Firebird::RaiiLockGuard<StableAttachmentPart> SyncGuard;

explicit StableAttachmentPart(Attachment* handle)
: att(handle), jAtt(NULL)
{ }
Expand Down Expand Up @@ -619,17 +615,21 @@ class AttachmentsRefHolder
: m_attachments(p)
{}

AttachmentsRefHolder()
: m_attachments(*MemoryPool::getContextPool())
{}

AttachmentsRefHolder& operator=(const AttachmentsRefHolder& other)
{
this->~AttachmentsRefHolder();
clear();

for (FB_SIZE_T i = 0; i < other.m_attachments.getCount(); i++)
add(other.m_attachments[i]);

return *this;
}

~AttachmentsRefHolder()
void clear()
{
while (m_attachments.hasData())
{
Expand All @@ -648,6 +648,11 @@ class AttachmentsRefHolder
return m_attachments.hasData();
}

~AttachmentsRefHolder()
{
clear();
}

void add(StableAttachmentPart* jAtt)
{
if (jAtt)
Expand All @@ -657,11 +662,6 @@ class AttachmentsRefHolder
}
}

void remove(Iterator& iter)
{
iter.remove();
}

private:
AttachmentsRefHolder(const AttachmentsRefHolder&);

Expand Down
106 changes: 77 additions & 29 deletions src/jrd/CryptoManager.cpp
View file
Expand Up @@ -270,6 +270,38 @@ namespace Jrd {
AutoPtr<UCHAR, ArrayDelete> buffer;
};

// Ensures that at least one of attachment's syncs (main or async) is locked
class AttachmentAnySyncHolder : public EnsureUnlock<StableAttachmentPart::Sync, NotRefCounted>
{
public:
AttachmentAnySyncHolder(StableAttachmentPart* sAtt)
: EnsureUnlock(*(sAtt->getSync(true, true)), FB_FUNCTION),
att(sAtt->getHandle())
{
if (!sAtt->getSync()->locked())
enter();
}

bool hasData()
{
return att;
}

Attachment* operator->()
{
return att;
}

operator Attachment*()
{
return att;
}

private:
Attachment* att;
};


CryptoManager::CryptoManager(thread_db* tdbb)
: PermanentStorage(*tdbb->getDatabase()->dbb_permanent),
sync(this),
Expand Down Expand Up @@ -489,7 +521,7 @@ namespace Jrd {
if (!keyPlugin->useOnlyOwnKeys(&st))
{
MutexLockGuard g(holdersMutex, FB_FUNCTION);
keyProviders.push(tdbb->getAttachment());
keyProviders.add(tdbb->getAttachment()->getStable());
}
fLoad = true;
break;
Expand Down Expand Up @@ -625,6 +657,20 @@ namespace Jrd {

try
{
// Create local copy of existing attachments
Sync dSync(&dbb.dbb_sync, FB_FUNCTION);
dSync.lock(SYNC_EXCLUSIVE);

AttachmentsRefHolder existing;
{
MutexLockGuard g(holdersMutex, FB_FUNCTION);
for (Attachment* att = dbb.dbb_attachments; att; att = att->att_next)
existing.add(att->getStable());
}

dSync.unlock();

// Disable cache I/O
BarSync::LockGuard writeGuard(tdbb, sync);

// header scope
Expand Down Expand Up @@ -687,22 +733,16 @@ namespace Jrd {

if (checkFactory)
{
// Create local copy of existing attachments
AttVector existing;
// Loop through attachments
for (AttachmentsRefHolder::Iterator iter(existing); *iter; ++iter)
{
SyncLockGuard dsGuard(&dbb.dbb_sync, SYNC_EXCLUSIVE, FB_FUNCTION);
for (Attachment* att = dbb.dbb_attachments; att; att = att->att_next)
existing.push(att);
AttachmentAnySyncHolder a(*iter);
if (a.hasData())
internalAttach(tdbb, a, true);
}

// Loop through attachments
MutexLockGuard g(holdersMutex, FB_FUNCTION);

for (unsigned n = 0; n < existing.getCount(); ++n)
internalAttach(tdbb, existing[n], true);

// In case of missing providers close consumers
if (keyProviders.getCount() == 0)
if (!keyProviders.hasData())
shutdownConsumers(tdbb);
}
}
Expand Down Expand Up @@ -779,8 +819,12 @@ namespace Jrd {
{
MutexLockGuard g(holdersMutex, FB_FUNCTION);

for (unsigned i = 0; i < keyConsumers.getCount(); ++i)
keyConsumers[i]->signalShutdown();
for (AttachmentsRefHolder::Iterator iter(keyConsumers); *iter; ++iter)
{
AttachmentAnySyncHolder a(*iter);
if (a.hasData())
a->signalShutdown();
}

keyConsumers.clear();
}
Expand Down Expand Up @@ -838,11 +882,12 @@ namespace Jrd {
}

// Apply results
MutexLockGuard g(holdersMutex, FB_FUNCTION);

if (fProvide)
keyProviders.push(att);
keyProviders.add(att->getStable());
else if (consume && !fLoad)
keyConsumers.push(att);
keyConsumers.add(att->getStable());

return fLoad;
}
Expand All @@ -851,14 +896,13 @@ namespace Jrd {
{
if (checkFactory)
{
MutexLockGuard g(holdersMutex, FB_FUNCTION);

if (!internalAttach(tdbb, att, false))
{
if (keyProviders.getCount() == 0)
(Arg::Gds(isc_random) << "Missing correct crypt key").raise();
MutexLockGuard g(holdersMutex, FB_FUNCTION);

keyConsumers.push(att);
if (!keyProviders.hasData())
(Arg::Gds(isc_random) << "Missing correct crypt key").raise();
keyConsumers.add(att->getStable());
}
}

Expand All @@ -871,21 +915,25 @@ namespace Jrd {
return;

MutexLockGuard g(holdersMutex, FB_FUNCTION);
for (unsigned n = 0; n < keyConsumers.getCount(); ++n)
for (AttachmentsRefHolder::Iterator iter(keyConsumers); *iter; ++iter)
{
if (keyConsumers[n] == att)
StableAttachmentPart* const sAtt = *iter;

if (sAtt->getHandle() == att)
{
keyConsumers.remove(n);
iter.remove();
return;
}
}

for (unsigned n = 0; n < keyProviders.getCount(); ++n)
for (AttachmentsRefHolder::Iterator iter(keyProviders); *iter; ++iter)
{
if (keyProviders[n] == att)
StableAttachmentPart* const sAtt = *iter;

if (sAtt->getHandle() == att)
{
keyProviders.remove(n);
if (keyProviders.getCount() == 0)
iter.remove();
if (!keyProviders.hasData())
shutdownConsumers(tdbb);
return;
}
Expand Down
4 changes: 2 additions & 2 deletions src/jrd/CryptoManager.h
View file
Expand Up @@ -36,6 +36,7 @@
#include "../common/classes/objects_array.h"
#include "../common/classes/condition.h"
#include "../common/classes/MetaName.h"
#include "../jrd/Attachment.h"
#include "../common/classes/GetPlugins.h"
#include "../common/ThreadStart.h"
#include "../jrd/ods.h"
Expand Down Expand Up @@ -268,7 +269,6 @@ class CryptoManager FB_FINAL : public Firebird::PermanentStorage, public BarSync
{
public:
typedef Firebird::GetPlugins<Firebird::IDbCryptPlugin> Factory;
typedef Firebird::HalfStaticArray<Attachment*, 16> AttVector;

explicit CryptoManager(thread_db* tdbb);
~CryptoManager();
Expand Down Expand Up @@ -384,7 +384,7 @@ class CryptoManager FB_FINAL : public Firebird::PermanentStorage, public BarSync
Firebird::MetaName keyName;
ULONG currentPage;
Firebird::Mutex pluginLoadMtx, cryptThreadMtx, holdersMutex;
AttVector keyProviders, keyConsumers;
AttachmentsRefHolder keyProviders, keyConsumers;
Firebird::string hash;
Firebird::RefPtr<DbInfo> dbInfo;
Thread::Handle cryptThreadId;
Expand Down

0 comments on commit a0cddf8

Please sign in to comment.