Skip to content

Commit e7801a5

Browse files
Fix potential deadlock when starting the encryption thread (#8403) B3_0_Release backport (#8412)
add a check to verify thread matching between the encryption thread and the thread where we release the attachment. If they match, use a dummy mutex instead of the actual dbb_thread_mutex to avoid a deadlock Co-authored-by: aleksey.mochalov <[email protected]>
1 parent f20aba8 commit e7801a5

File tree

5 files changed

+51
-11
lines changed

5 files changed

+51
-11
lines changed

src/common/ThreadStart.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,19 @@ ThreadId Thread::getId()
219219
#endif
220220
}
221221

222+
ThreadId Thread::getIdFromHandle(Handle threadHandle)
223+
{
224+
return threadHandle;
225+
}
226+
227+
bool Thread::isCurrent(InternalId iid)
228+
{
229+
return pthread_equal(iid, pthread_self());
230+
}
231+
222232
bool Thread::isCurrent()
223233
{
224-
return pthread_equal(internalId, pthread_self());
234+
return isCurrent(internalId);
225235
}
226236

227237
void Thread::sleep(unsigned milliseconds)
@@ -363,9 +373,19 @@ ThreadId Thread::getId()
363373
return GetCurrentThreadId();
364374
}
365375

376+
ThreadId Thread::getIdFromHandle(Handle threadHandle)
377+
{
378+
return GetThreadId(threadHandle);
379+
}
380+
381+
bool Thread::isCurrent(InternalId iid)
382+
{
383+
return GetCurrentThreadId() == iid;
384+
}
385+
366386
bool Thread::isCurrent()
367387
{
368-
return GetCurrentThreadId() == internalId;
388+
return isCurrent(internalId);
369389
}
370390

371391
void Thread::sleep(unsigned milliseconds)
@@ -410,6 +430,10 @@ Thread::Handle Thread::getId()
410430
{
411431
}
412432

433+
Thread::Handle Thread::getIdFromHandle(Handle)
434+
{
435+
}
436+
413437
void Thread::sleep(unsigned milliseconds)
414438
{
415439
}

src/common/ThreadStart.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,12 @@ class Thread
8383
static void kill(Handle& handle);
8484

8585
static ThreadId getId();
86+
static ThreadId getIdFromHandle(Handle threadHandle);
8687

8788
static void sleep(unsigned milliseconds);
8889
static void yield();
8990

91+
static bool isCurrent(InternalId iid);
9092
bool isCurrent();
9193

9294
Thread()

src/jrd/CryptoManager.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ namespace Jrd {
328328
keyConsumers(getPool()),
329329
hash(getPool()),
330330
dbInfo(FB_NEW DbInfo(this)),
331-
cryptThreadId(0),
331+
cryptThreadHandle(0),
332332
cryptPlugin(NULL),
333333
checkFactory(NULL),
334334
dbb(*tdbb->getDatabase()),
@@ -346,8 +346,8 @@ namespace Jrd {
346346

347347
CryptoManager::~CryptoManager()
348348
{
349-
if (cryptThreadId)
350-
Thread::waitForCompletion(cryptThreadId);
349+
if (cryptThreadHandle)
350+
Thread::waitForCompletion(cryptThreadHandle);
351351

352352
delete stateLock;
353353
delete threadLock;
@@ -933,10 +933,10 @@ namespace Jrd {
933933
void CryptoManager::terminateCryptThread(thread_db*, bool wait)
934934
{
935935
flDown = true;
936-
if (wait && cryptThreadId)
936+
if (wait && cryptThreadHandle)
937937
{
938-
Thread::waitForCompletion(cryptThreadId);
939-
cryptThreadId = 0;
938+
Thread::waitForCompletion(cryptThreadHandle);
939+
cryptThreadHandle = 0;
940940
}
941941
}
942942

@@ -995,7 +995,7 @@ namespace Jrd {
995995

996996
// ready to go
997997
guard.leave(); // release in advance to avoid races with cryptThread()
998-
Thread::start(cryptThreadStatic, (THREAD_ENTRY_PARAM) this, THREAD_medium, &cryptThreadId);
998+
Thread::start(cryptThreadStatic, (THREAD_ENTRY_PARAM) this, THREAD_medium, &cryptThreadHandle);
999999
}
10001000
catch (const Firebird::Exception&)
10011001
{

src/jrd/CryptoManager.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,10 @@ class CryptoManager FB_FINAL : public Firebird::PermanentStorage, public BarSync
302302
ULONG getCurrentPage(thread_db* tdbb) const;
303303
UCHAR getCurrentState(thread_db* tdbb) const;
304304
const char* getKeyName() const;
305+
Thread::Handle getCryptThreadHandle() const
306+
{
307+
return cryptThreadHandle;
308+
}
305309

306310
private:
307311
enum IoResult {SUCCESS_ALL, FAILED_CRYPT, FAILED_IO};
@@ -388,7 +392,7 @@ class CryptoManager FB_FINAL : public Firebird::PermanentStorage, public BarSync
388392
AttachmentsRefHolder keyProviders, keyConsumers;
389393
Firebird::string hash;
390394
Firebird::RefPtr<DbInfo> dbInfo;
391-
Thread::Handle cryptThreadId;
395+
Thread::Handle cryptThreadHandle;
392396
Firebird::IDbCryptPlugin* cryptPlugin;
393397
Factory* checkFactory;
394398
Database& dbb;

src/jrd/jrd.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6433,9 +6433,19 @@ static void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment)
64336433

64346434
Sync sync(&dbb->dbb_sync, "jrd.cpp: release_attachment");
64356435

6436+
// dummy mutex is used to avoid races with crypto thread
6437+
XThreadMutex dummy_mutex;
6438+
XThreadEnsureUnlock dummyGuard(dummy_mutex, FB_FUNCTION);
6439+
64366440
// avoid races with special threads
64376441
XThreadEnsureUnlock threadGuard(dbb->dbb_thread_mutex, FB_FUNCTION);
6438-
threadGuard.enter();
6442+
XThreadEnsureUnlock* activeThreadGuard = &threadGuard;
6443+
if (dbb->dbb_crypto_manager
6444+
&& Thread::isCurrent(Thread::getIdFromHandle(dbb->dbb_crypto_manager->getCryptThreadHandle())))
6445+
{
6446+
activeThreadGuard = &dummyGuard;
6447+
}
6448+
activeThreadGuard->enter();
64396449

64406450
sync.lock(SYNC_EXCLUSIVE);
64416451

0 commit comments

Comments
 (0)