From e7801a5a27ac4f54111c25fce431c471a20e135a Mon Sep 17 00:00:00 2001
From: MochalovAlexey <alexeymochalov@icloud.com>
Date: Tue, 28 Jan 2025 08:43:39 +0300
Subject: [PATCH] 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 <aleksey.mochalov@red-soft.ru>
---
 src/common/ThreadStart.cpp | 28 ++++++++++++++++++++++++++--
 src/common/ThreadStart.h   |  2 ++
 src/jrd/CryptoManager.cpp  | 14 +++++++-------
 src/jrd/CryptoManager.h    |  6 +++++-
 src/jrd/jrd.cpp            | 12 +++++++++++-
 5 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/src/common/ThreadStart.cpp b/src/common/ThreadStart.cpp
index 18c8615eb4f..2fe2b6defc1 100644
--- a/src/common/ThreadStart.cpp
+++ b/src/common/ThreadStart.cpp
@@ -219,9 +219,19 @@ ThreadId Thread::getId()
 #endif
 }
 
+ThreadId Thread::getIdFromHandle(Handle threadHandle)
+{
+	return threadHandle;
+}
+
+bool Thread::isCurrent(InternalId iid)
+{
+	return pthread_equal(iid, pthread_self());
+}
+
 bool Thread::isCurrent()
 {
-	return pthread_equal(internalId, pthread_self());
+	return isCurrent(internalId);
 }
 
 void Thread::sleep(unsigned milliseconds)
@@ -363,9 +373,19 @@ ThreadId Thread::getId()
 	return GetCurrentThreadId();
 }
 
+ThreadId Thread::getIdFromHandle(Handle threadHandle)
+{
+	return GetThreadId(threadHandle);
+}
+
+bool Thread::isCurrent(InternalId iid)
+{
+	return GetCurrentThreadId() == iid;
+}
+
 bool Thread::isCurrent()
 {
-	return GetCurrentThreadId() == internalId;
+	return isCurrent(internalId);
 }
 
 void Thread::sleep(unsigned milliseconds)
@@ -410,6 +430,10 @@ Thread::Handle Thread::getId()
 {
 }
 
+Thread::Handle Thread::getIdFromHandle(Handle)
+{
+}
+
 void Thread::sleep(unsigned milliseconds)
 {
 }
diff --git a/src/common/ThreadStart.h b/src/common/ThreadStart.h
index bfefcc81b39..7bea8899c05 100644
--- a/src/common/ThreadStart.h
+++ b/src/common/ThreadStart.h
@@ -83,10 +83,12 @@ class Thread
 	static void kill(Handle& handle);
 
 	static ThreadId getId();
+	static ThreadId getIdFromHandle(Handle threadHandle);
 
 	static void sleep(unsigned milliseconds);
 	static void yield();
 
+	static bool isCurrent(InternalId iid);
 	bool isCurrent();
 
 	Thread()
diff --git a/src/jrd/CryptoManager.cpp b/src/jrd/CryptoManager.cpp
index 26410db9c83..fe342e7f791 100644
--- a/src/jrd/CryptoManager.cpp
+++ b/src/jrd/CryptoManager.cpp
@@ -328,7 +328,7 @@ namespace Jrd {
 		  keyConsumers(getPool()),
 		  hash(getPool()),
 		  dbInfo(FB_NEW DbInfo(this)),
-		  cryptThreadId(0),
+		  cryptThreadHandle(0),
 		  cryptPlugin(NULL),
 		  checkFactory(NULL),
 		  dbb(*tdbb->getDatabase()),
@@ -346,8 +346,8 @@ namespace Jrd {
 
 	CryptoManager::~CryptoManager()
 	{
-		if (cryptThreadId)
-			Thread::waitForCompletion(cryptThreadId);
+		if (cryptThreadHandle)
+			Thread::waitForCompletion(cryptThreadHandle);
 
 		delete stateLock;
 		delete threadLock;
@@ -933,10 +933,10 @@ namespace Jrd {
 	void CryptoManager::terminateCryptThread(thread_db*, bool wait)
 	{
 		flDown = true;
-		if (wait && cryptThreadId)
+		if (wait && cryptThreadHandle)
 		{
-			Thread::waitForCompletion(cryptThreadId);
-			cryptThreadId = 0;
+			Thread::waitForCompletion(cryptThreadHandle);
+			cryptThreadHandle = 0;
 		}
 	}
 
@@ -995,7 +995,7 @@ namespace Jrd {
 
 			// ready to go
 			guard.leave();		// release in advance to avoid races with cryptThread()
-			Thread::start(cryptThreadStatic, (THREAD_ENTRY_PARAM) this, THREAD_medium, &cryptThreadId);
+			Thread::start(cryptThreadStatic, (THREAD_ENTRY_PARAM) this, THREAD_medium, &cryptThreadHandle);
 		}
 		catch (const Firebird::Exception&)
 		{
diff --git a/src/jrd/CryptoManager.h b/src/jrd/CryptoManager.h
index 0f7e5999608..730462ee388 100644
--- a/src/jrd/CryptoManager.h
+++ b/src/jrd/CryptoManager.h
@@ -302,6 +302,10 @@ class CryptoManager FB_FINAL : public Firebird::PermanentStorage, public BarSync
 	ULONG getCurrentPage(thread_db* tdbb) const;
 	UCHAR getCurrentState(thread_db* tdbb) const;
 	const char* getKeyName() const;
+	Thread::Handle getCryptThreadHandle() const
+	{
+		return cryptThreadHandle;
+	}
 
 private:
 	enum IoResult {SUCCESS_ALL, FAILED_CRYPT, FAILED_IO};
@@ -388,7 +392,7 @@ class CryptoManager FB_FINAL : public Firebird::PermanentStorage, public BarSync
 	AttachmentsRefHolder keyProviders, keyConsumers;
 	Firebird::string hash;
 	Firebird::RefPtr<DbInfo> dbInfo;
-	Thread::Handle cryptThreadId;
+	Thread::Handle cryptThreadHandle;
 	Firebird::IDbCryptPlugin* cryptPlugin;
 	Factory* checkFactory;
 	Database& dbb;
diff --git a/src/jrd/jrd.cpp b/src/jrd/jrd.cpp
index de2649fae09..eced5e54d71 100644
--- a/src/jrd/jrd.cpp
+++ b/src/jrd/jrd.cpp
@@ -6433,9 +6433,19 @@ static void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment)
 
 	Sync sync(&dbb->dbb_sync, "jrd.cpp: release_attachment");
 
+	// dummy mutex is used to avoid races with crypto thread
+	XThreadMutex dummy_mutex;
+	XThreadEnsureUnlock dummyGuard(dummy_mutex, FB_FUNCTION);
+
 	// avoid races with special threads
 	XThreadEnsureUnlock threadGuard(dbb->dbb_thread_mutex, FB_FUNCTION);
-	threadGuard.enter();
+	XThreadEnsureUnlock* activeThreadGuard = &threadGuard;
+	if (dbb->dbb_crypto_manager
+		&& Thread::isCurrent(Thread::getIdFromHandle(dbb->dbb_crypto_manager->getCryptThreadHandle())))
+	{
+		activeThreadGuard = &dummyGuard;
+	}
+	activeThreadGuard->enter();
 
 	sync.lock(SYNC_EXCLUSIVE);