Skip to content

Commit

Permalink
Fixed CORE-5197: Segfault when process exits with active sweep thread
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlexPeshkoff
AlexPeshkoff committed Dec 8, 2020
1 parent 41e3471 commit 7a67126
Show file tree
Hide file tree
Showing 9 changed files with 259 additions and 94 deletions.
15 changes: 9 additions & 6 deletions src/common/classes/locks.h
View file
Expand Up @@ -321,16 +321,17 @@ typedef Mutex Spinlock;


// RAII holder
class MutexLockGuard
template <typename M>
class RaiiLockGuard
{
public:
MutexLockGuard(Mutex& aLock, const char* aReason)
RaiiLockGuard(M& aLock, const char* aReason)
: lock(&aLock)
{
lock->enter(aReason);
}

~MutexLockGuard()
~RaiiLockGuard()
{
try
{
Expand All @@ -354,12 +355,14 @@ class MutexLockGuard

private:
// Forbid copying
MutexLockGuard(const MutexLockGuard&);
MutexLockGuard& operator=(const MutexLockGuard&);
RaiiLockGuard(const RaiiLockGuard&);
RaiiLockGuard& operator=(const RaiiLockGuard&);

Mutex* lock;
M* lock;
};

typedef RaiiLockGuard<Mutex> MutexLockGuard;

class MutexUnlockGuard
{
public:
Expand Down
2 changes: 1 addition & 1 deletion src/jrd/Attachment.h
View file
Expand Up @@ -159,7 +159,7 @@ const ULONG ATT_creator = 0x08000L; // This attachment created the DB
const ULONG ATT_monitor_done = 0x10000L; // Monitoring data is refreshed
const ULONG ATT_security_db = 0x20000L; // Attachment used for security purposes
const ULONG ATT_mapping = 0x40000L; // Attachment used for mapping auth block
const ULONG ATT_crypt_thread = 0x80000L; // Attachment from crypt thread
const ULONG ATT_from_thread = 0x80000L; // Attachment from internal special thread (sweep, crypt)
const ULONG ATT_monitor_init = 0x100000L; // Attachment is registered in monitoring
const ULONG ATT_repl_reset = 0x200000L; // Replication set has been reset
const ULONG ATT_replicating = 0x400000L; // Replication is active
Expand Down
21 changes: 13 additions & 8 deletions src/jrd/CryptoManager.cpp
View file
Expand Up @@ -278,7 +278,7 @@ namespace Jrd {
slowIO(0),
crypt(false),
process(false),
down(false),
flDown(false),
run(false)
{
stateLock = FB_NEW_RPT(getPool(), 0)
Expand Down Expand Up @@ -817,7 +817,7 @@ namespace Jrd {

void CryptoManager::terminateCryptThread(thread_db*, bool wait)
{
down = true;
flDown = true;
if (wait && cryptThreadId)
{
Thread::waitForCompletion(cryptThreadId);
Expand Down Expand Up @@ -956,10 +956,10 @@ namespace Jrd {
writer.insertByte(isc_dpb_no_db_triggers, TRUE);

// Avoid races with release_attachment() in jrd.cpp
MutexEnsureUnlock releaseGuard(cryptAttMutex, FB_FUNCTION);
XThreadEnsureUnlock releaseGuard(dbb.dbb_thread_mutex, FB_FUNCTION);
releaseGuard.enter();

if (!down)
if (!down())
{
AutoPlugin<JProvider> jInstance(JProvider::getInstance());
jInstance->setDbCryptCallback(&status_vector, dbb.dbb_callback);
Expand All @@ -973,7 +973,7 @@ namespace Jrd {
Attachment* att = jAtt->getHandle();
if (!att)
Arg::Gds(isc_att_shutdown).raise();
att->att_flags |= ATT_crypt_thread;
att->att_flags |= ATT_from_thread;
releaseGuard.leave();

ThreadContextHolder tdbb(att->att_database, att, &status_vector);
Expand Down Expand Up @@ -1008,7 +1008,7 @@ namespace Jrd {
while (currentPage < lastPage)
{
// forced terminate
if (down)
if (down())
{
break;
}
Expand Down Expand Up @@ -1050,7 +1050,7 @@ namespace Jrd {
}

// forced terminate
if (down)
if (down())
{
break;
}
Expand All @@ -1062,7 +1062,7 @@ namespace Jrd {
} while (currentPage < lastPage);

// Finalize crypt
if (!down)
if (!down())
{
writeDbHeader(tdbb, 0);
}
Expand Down Expand Up @@ -1345,6 +1345,11 @@ namespace Jrd {
return pluginName.c_str();
}

bool CryptoManager::down() const
{
return flDown || (dbb.dbb_flags & DBB_closing);
}

void CryptoManager::addClumplet(string& signature, ClumpletReader& block, UCHAR tag)
{
if (block.find(tag))
Expand Down
7 changes: 2 additions & 5 deletions src/jrd/CryptoManager.h
View file
Expand Up @@ -42,8 +42,6 @@
#include "../jrd/status.h"
#include "firebird/Interface.h"

#define CRYPT_DEBUG(A)

// forward

namespace Ods {
Expand Down Expand Up @@ -409,10 +407,9 @@ class CryptoManager FB_FINAL : public Firebird::PermanentStorage, public BarSync
// normal operation.

SINT64 slowIO;
bool crypt, process, down, run;
bool crypt, process, flDown, run;

public:
Firebird::Mutex cryptAttMutex;
bool down() const;
};

} // namespace Jrd
Expand Down
81 changes: 73 additions & 8 deletions src/jrd/Database.cpp
View file
Expand Up @@ -174,10 +174,21 @@ namespace Jrd
Database* dbb = static_cast<Database*>(ast_object);
AsyncContextHolder tdbb(dbb, FB_FUNCTION);

if ((dbb->dbb_flags & DBB_sweep_starting) && !(dbb->dbb_flags & DBB_sweep_in_progress))
while (true)
{
dbb->dbb_flags &= ~DBB_sweep_starting;
LCK_release(tdbb, dbb->dbb_sweep_lock);
AtomicCounter::counter_type old = dbb->dbb_flags;
if ((old & DBB_sweep_in_progress) || !(old & DBB_sweep_starting))
{
SPTHR_DEBUG(fprintf(stderr, "blocking_ast_sweep %p false wrong flags %lx\n", dbb, old));
break;
}

if (dbb->dbb_flags.compareExchange(old, old & ~DBB_sweep_starting))
{
SPTHR_DEBUG(fprintf(stderr, "blocking_ast_sweep true %p\n", dbb));
dbb->dbb_thread_mutex.leave();
break;
}
}
}
catch (const Exception&)
Expand All @@ -199,39 +210,82 @@ namespace Jrd

bool Database::allowSweepThread(thread_db* tdbb)
{
SPTHR_DEBUG(fprintf(stderr, "allowSweepThread %p\n", this));
if (readOnly())
return false;

Jrd::Attachment* const attachment = tdbb->getAttachment();
if (attachment->att_flags & ATT_no_cleanup)
return false;

if (!dbb_thread_mutex.tryEnter(FB_FUNCTION))
{
SPTHR_DEBUG(fprintf(stderr, "allowSweepThread %p false, dbb_thread_mutex busy\n", this));
return false;
}

if (dbb_flags & DBB_closing)
{
SPTHR_DEBUG(fprintf(stderr, "allowSweepThread false, dbb closing\n"));
dbb_thread_mutex.leave();
return false;
}

while (true)
{
AtomicCounter::counter_type old = dbb_flags;
if ((old & (DBB_sweep_in_progress | DBB_sweep_starting)) || (dbb_ast_flags & DBB_shutdown))
{
dbb_thread_mutex.leave();
return false;
}

if (dbb_flags.compareExchange(old, old | DBB_sweep_starting))
break;
}

SPTHR_DEBUG(fprintf(stderr, "allowSweepThread - set DBB_sweep_starting\n"));

createSweepLock(tdbb);
if (!LCK_lock(tdbb, dbb_sweep_lock, LCK_EX, LCK_NO_WAIT))
{
// clear lock error from status vector
fb_utils::init_status(tdbb->tdbb_status_vector);

dbb_flags &= ~DBB_sweep_starting;
clearSweepStarting();
SPTHR_DEBUG(fprintf(stderr, "allowSweepThread - !LCK_lock\n"));
return false;
}

SPTHR_DEBUG(fprintf(stderr, "allowSweepThread - TRUE\n"));
return true;
}

bool Database::clearSweepStarting()
{
while (true)
{
AtomicCounter::counter_type old = dbb_flags;
if (!(old & DBB_sweep_starting))
{
SPTHR_DEBUG(fprintf(stderr, "clearSweepStarting false %p\n", this));
return false;
}

if (dbb_flags.compareExchange(old, old & ~DBB_sweep_starting))
{
SPTHR_DEBUG(fprintf(stderr, "clearSweepStarting true %p\n", this));
dbb_thread_mutex.leave();
return true;
}
}
}

bool Database::allowSweepRun(thread_db* tdbb)
{
if (readOnly())
SPTHR_DEBUG(fprintf(stderr, "allowSweepRun %p\n", this));

if (readOnly() || (dbb_flags & DBB_closing))
return false;

Jrd::Attachment* const attachment = tdbb->getAttachment();
Expand All @@ -242,14 +296,21 @@ namespace Jrd
{
AtomicCounter::counter_type old = dbb_flags;
if (old & DBB_sweep_in_progress)
{
clearSweepStarting();
return false;
}

if (dbb_flags.compareExchange(old, old | DBB_sweep_in_progress))
break;
}

SPTHR_DEBUG(fprintf(stderr, "allowSweepRun - set DBB_sweep_in_progress\n"));

if (!(dbb_flags & DBB_sweep_starting))
{
SPTHR_DEBUG(fprintf(stderr, "allowSweepRun - createSweepLock\n"));

createSweepLock(tdbb);
if (!LCK_lock(tdbb, dbb_sweep_lock, LCK_EX, -1))
{
Expand All @@ -261,20 +322,24 @@ namespace Jrd
}
}
else
dbb_flags &= ~DBB_sweep_starting;
{
SPTHR_DEBUG(fprintf(stderr, "allowSweepRun - clearSweepStarting\n"));
attachment->att_flags |= ATT_from_thread;
clearSweepStarting();
}

return true;
}

void Database::clearSweepFlags(thread_db* tdbb)
{
if (!(dbb_flags & (DBB_sweep_starting | DBB_sweep_in_progress)))
if (!(dbb_flags & DBB_sweep_in_progress))
return;

if (dbb_sweep_lock)
LCK_release(tdbb, dbb_sweep_lock);

dbb_flags &= ~(DBB_sweep_in_progress | DBB_sweep_starting);
dbb_flags &= ~DBB_sweep_in_progress;
}

void Database::registerModule(Module& module)
Expand Down

0 comments on commit 7a67126

Please sign in to comment.