Fixed CORE-5326: encrypting a database crashes host process when background encryption gets ready while no connection is established to that database

src/jrd/Attachment.h

@@ -403,6 +403,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_NO_CLEANUP			= (ATT_no_cleanup | ATT_notify_gc);
 

src/jrd/CryptoManager.cpp

@@ -616,9 +616,14 @@ namespace Jrd {
 		lockAndReadHeader(tdbb, CRYPT_HDR_INIT);
 	}
 
-	void CryptoManager::terminateCryptThread(thread_db*)
+	void CryptoManager::terminateCryptThread(thread_db*, bool wait)
 	{
 		down = true;
+		if (wait && cryptThreadId)
+		{
+			Thread::waitForCompletion(cryptThreadId);
+			cryptThreadId = 0;
+		}
 	}
 
 	void CryptoManager::stopThreadUsing(thread_db* tdbb, Attachment* att)
@@ -749,6 +754,11 @@ namespace Jrd {
 				writer.insertString(isc_dpb_user_name, "SYSDBA");
 				writer.insertByte(isc_dpb_no_db_triggers, TRUE);
 
+				// Avoid races with release_attachment() in jrd.cpp
+				MutexEnsureUnlock releaseGuard(cryptAttMutex, FB_FUNCTION);
+				releaseGuard.enter();
+				if (!down)
+				{
 				RefPtr<JAttachment> jAtt(REF_NO_INCR, dbb.dbb_provider->attachDatabase(&status_vector,
 					dbb.dbb_filename.c_str(), writer.getBufferLength(), writer.getBuffer()));
 				check(&status_vector);
@@ -757,6 +767,9 @@ namespace Jrd {
 				Attachment* att = jAtt->getHandle();
 				if (!att)
 					Arg::Gds(isc_att_shutdown).raise();
+				att->att_flags |= ATT_crypt_thread;
+				releaseGuard.leave();
+
 				ThreadContextHolder tdbb(att->att_database, att, &status_vector);
 				tdbb->tdbb_quantum = SWEEP_QUANTUM;
 
@@ -848,6 +861,7 @@ namespace Jrd {
 				{
 					writeDbHeader(tdbb, 0);
 				}
+				}
 
 				// Release exclusive lock on StartCryptThread
 				lckRelease = true;

src/jrd/CryptoManager.h

@@ -41,6 +41,8 @@
 #include "../jrd/status.h"
 #include "firebird/Interface.h"
 
+#define CRYPT_DEBUG(A)
+
 // forward
 
 class Config;
@@ -278,7 +280,7 @@ public:
 	void detach(thread_db* tdbb, Attachment* att);
 
 	void startCryptThread(thread_db* tdbb);
-	void terminateCryptThread(thread_db* tdbb);
+	void terminateCryptThread(thread_db* tdbb, bool wait = false);
 	void stopThreadUsing(thread_db* tdbb, Attachment* att);
 
 	class IOCallback
@@ -411,6 +413,9 @@ private:
 
 	SINT64 slowIO;
 	bool crypt, process, down, run;
+
+public:
+	Firebird::Mutex cryptAttMutex;
 };
 
 } // namespace Jrd

src/jrd/jrd.cpp

@@ -6359,10 +6359,47 @@ static void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment)
 
 	attachment->mergeStats();
 
-	// remove the attachment block from the dbb linked list
+	// avoid races with crypt thread
+	MutexEnsureUnlock cryptGuard(dbb->dbb_crypto_manager->cryptAttMutex, FB_FUNCTION);
+	cryptGuard.enter();
+
 	Sync sync(&dbb->dbb_sync, "jrd.cpp: release_attachment");
 	sync.lock(SYNC_EXCLUSIVE);
 
+	// stop the crypt thread if we release last regular attachment
+	Jrd::Attachment* crypt_att = NULL;
+	CRYPT_DEBUG(fprintf(stderr, "\nrelease attachment=%p\n", attachment));
+	for (Jrd::Attachment* att = dbb->dbb_attachments; att; att = att->att_next)
+	{
+		CRYPT_DEBUG(fprintf(stderr, "att=%p crypt_att=%p F=%c ", att, crypt_att, att->att_flags & ATT_crypt_thread ? '1' : '0'));
+		if (att == attachment)
+		{
+			CRYPT_DEBUG(fprintf(stderr, "self\n"));
+			continue;
+		}
+		if (att->att_flags & ATT_crypt_thread)
+		{
+			crypt_att = att;
+			CRYPT_DEBUG(fprintf(stderr, "found crypt_att=%p\n", crypt_att));
+			continue;
+		}
+		crypt_att = NULL;
+		CRYPT_DEBUG(fprintf(stderr, "other\n"));
+		break;
+	}
+	cryptGuard.leave();
+
+	if (crypt_att)
+	{
+		sync.unlock();
+
+		CRYPT_DEBUG(fprintf(stderr, "crypt_att=%p terminateCryptThread\n", crypt_att));
+		dbb->dbb_crypto_manager->terminateCryptThread(tdbb, true);
+
+		sync.lock(SYNC_EXCLUSIVE);
+	}
+
+	// remove the attachment block from the dbb linked list
 	for (Jrd::Attachment** ptr = &dbb->dbb_attachments; *ptr; ptr = &(*ptr)->att_next)
 	{
 		if (*ptr == attachment)
@@ -6386,8 +6423,7 @@ static void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment)
 	}
 
 	tdbb->setAttachment(NULL);
-	Jrd::Attachment::destroy(attachment);	// string were re-saved in the beginning of this function,
+	Jrd::Attachment::destroy(attachment);
-											// keep that in sync please
 }