Fix potential deadlock when starting the encryption thread (#8403)

* 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

* fix Windows build by introducing a method to retrieve thread id from crypto thread handle

* rename the crypt process thread id variable to its actual purpose as handle

---------

Co-authored-by: aleksey.mochalov <[email protected]>

Author: MochalovAlexey

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)
 {
 }

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()

src/jrd/CryptoManager.cpp

@@ -319,7 +319,7 @@ namespace Jrd {
 		  keyConsumers(getPool()),
 		  hash(getPool()),
 		  dbInfo(FB_NEW DbInfo(this)),
-		  cryptThreadId(0),
+		  cryptThreadHandle(0),
 		  cryptPlugin(NULL),
 		  checkFactory(NULL),
 		  dbb(*tdbb->getDatabase()),
@@ -337,8 +337,8 @@ namespace Jrd {
 
 	CryptoManager::~CryptoManager()
 	{
-		if (cryptThreadId)
-			Thread::waitForCompletion(cryptThreadId);
+		if (cryptThreadHandle)
+			Thread::waitForCompletion(cryptThreadHandle);
 
 		delete stateLock;
 		delete threadLock;
@@ -925,10 +925,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;
 		}
 	}
 
@@ -987,7 +987,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&)
 		{

src/jrd/CryptoManager.h

@@ -301,6 +301,10 @@ class CryptoManager final : public Firebird::PermanentStorage, public BarSync::I
 	UCHAR getCurrentState(thread_db* tdbb) const;
 	const char* getKeyName() const;
 	const char* getPluginName() const;
+	Thread::Handle getCryptThreadHandle() const
+	{
+		return cryptThreadHandle;
+	}
 
 private:
 	enum IoResult {SUCCESS_ALL, FAILED_CRYPT, FAILED_IO};
@@ -378,7 +382,7 @@ class CryptoManager final : public Firebird::PermanentStorage, public BarSync::I
 	AttachmentsRefHolder keyProviders, keyConsumers;
 	Firebird::string hash;
 	Firebird::RefPtr<DbInfo> dbInfo;
-	Thread::Handle cryptThreadId;
+	Thread::Handle cryptThreadHandle;
 	Firebird::IDbCryptPlugin* cryptPlugin;
 	Factory* checkFactory;
 	Database& dbb;

src/jrd/jrd.cpp

@@ -7763,14 +7763,26 @@ void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment, XThreadEns
 
 	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
 	// take into an account lock earlier taken in DROP DATABASE
 	XThreadEnsureUnlock threadGuard(dbb->dbb_thread_mutex, FB_FUNCTION);
 	XThreadEnsureUnlock* activeThreadGuard = dropGuard;
 	if (!activeThreadGuard)
 	{
-		threadGuard.enter();
-		activeThreadGuard = &threadGuard;
+		if (dbb->dbb_crypto_manager
+			&& Thread::isCurrent(Thread::getIdFromHandle(dbb->dbb_crypto_manager->getCryptThreadHandle())))
+		{
+			activeThreadGuard = &dummyGuard;
+		}
+		else
+		{
+			activeThreadGuard = &threadGuard;
+		}
+		activeThreadGuard->enter();
 	}
 
 	sync.lock(SYNC_EXCLUSIVE);