Fixed CORE-3944: SuperClassic crashes when running script that delete attachments and move database to offline

src/common/classes/PublicHandle.h

@@ -40,6 +40,7 @@ namespace Firebird
 	class ExistenceMutex : public RefMutex
 	{
 	public:
+		Mutex astMutex;
 		bool objectExists;
 
 		ExistenceMutex()

src/jrd/Database.cpp

@@ -190,11 +190,7 @@ namespace Jrd
 
 		try
 		{
-			Database::SyncGuard dsGuard(dbb, true);
+			AstContextHolder tdbb(dbb);
-
-			ThreadContextHolder tdbb;
-			tdbb->setDatabase(dbb);
-			// tdbb->setAttachment(counter->lock->lck_attachment);
 
 			Jrd::ContextPoolHolder context(tdbb, dbb->dbb_permanent);
 

src/jrd/DatabaseSnapshot.cpp

@@ -358,12 +358,7 @@ int DatabaseSnapshot::blockingAst(void* ast_object)
 	try
 	{
 		Lock* const lock = dbb->dbb_monitor_lock;
-
+		AstContextHolder tdbb(dbb, lock->lck_attachment);
-		Database::SyncGuard dsGuard(dbb, true);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(lock->lck_dbb);
-		tdbb->setAttachment(lock->lck_attachment);
 
 		ContextPoolHolder context(tdbb, dbb->dbb_permanent);
 

src/jrd/GlobalRWLock.cpp

@@ -56,10 +56,7 @@ int GlobalRWLock::blocking_ast_cached_lock(void* ast_object)
 			return 0;
 
 		Database* dbb = globalRWLock->cachedLock->lck_dbb;
-		Database::SyncGuard dsGuard(dbb, true);
+		AstContextHolder tdbb(dbb);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
 
 		if (globalRWLock->cachedLock)
 			globalRWLock->blockingAstHandler(tdbb);

src/jrd/cch.cpp

@@ -372,15 +372,11 @@ int CCH_down_grade_dbb(void* ast_object)
 
 	try
 	{
-		Database::SyncGuard dsGuard(dbb, true);
-
 		Lock* const lock = dbb->dbb_lock;
 
 		// Since this routine will be called asynchronously,
 		// we must establish a thread context
-		ThreadContextHolder tdbb;
+		AstContextHolder tdbb(dbb, lock->lck_attachment);
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(lock->lck_attachment);
 
 		dbb->dbb_ast_flags |= DBB_blocking;
 
@@ -2733,12 +2729,9 @@ static int blocking_ast_bdb(void* ast_object)
 	{
 		Database* dbb = bdb->bdb_dbb;
 
-		Database::SyncGuard dsGuard(dbb, true);
-
 		// Since this routine will be called asynchronously,
 		// we must establish a thread context
-		ThreadContextHolder tdbb;
+		AstContextHolder tdbb(dbb);
-		tdbb->setDatabase(dbb);
 
 		// Do some fancy footwork to make sure that pages are
 		// not removed from the btc tree at AST level. Then

src/jrd/idx.cpp

@@ -1535,12 +1535,7 @@ static int index_block_flush(void* ast_object)
 	{
 		Lock* lock = index_block->idb_lock;
 		Database* dbb = lock->lck_dbb;
-
+		AstContextHolder tdbb(dbb, lock->lck_attachment);
-		Database::SyncGuard dsGuard(dbb, true);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(lock->lck_attachment);
 
 		release_index_block(tdbb, index_block);
 	}

src/jrd/intl.cpp

@@ -1306,12 +1306,7 @@ static int blocking_ast_collation(void* ast_object)
 	try
 	{
 		Database* dbb = tt->existenceLock->lck_dbb;
-
+		AstContextHolder tdbb(dbb, tt->existenceLock->lck_attachment);
-		Database::SyncGuard dsGuard(dbb, true);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(tt->existenceLock->lck_attachment);
 
 		Jrd::ContextPoolHolder context(tdbb, 0);
 

src/jrd/jrd.cpp

@@ -2390,6 +2390,7 @@ ISC_STATUS GDS_DROP_DATABASE(ISC_STATUS* user_status, Attachment** handle)
 
 		Attachment* const attachment = *handle;
 		AttachmentHolder attHolder(tdbb, attachment, "GDS_DROP_DATABASE");
+		MutexEnsureUnlock astGuard(attachment->mutex()->astMutex);
 
 		try
 		{
@@ -2456,6 +2457,11 @@ ISC_STATUS GDS_DROP_DATABASE(ISC_STATUS* user_status, Attachment** handle)
 					TraceConnectionImpl conn(attachment);
 					attachment->att_trace_manager->event_detach(&conn, true);
 				}
+			}
+
+			{ // scope - take ast lock here
+				MutexLockGuard astGuard(attachment->mutex()->astMutex);
+				DatabaseContextHolder dbbHolder(tdbb);
 
 				// Unlink attachment from database
 				release_attachment(tdbb, attachment);
@@ -5428,7 +5434,7 @@ static void release_attachment(thread_db* tdbb, Attachment* attachment)
 			break;
 		}
 	}
-	Attachment::destroy(attachment);	// string were re-saved in the beginning of this function,
+	Attachment::destroy(attachment);	// strings were re-saved in the beginning of this function,
 										// keep that in sync please
 	tdbb->setAttachment(NULL);
 }
@@ -5455,12 +5461,13 @@ void Attachment::destroy(Attachment* const attachment)
 		}
 
 		Database* const dbb = attachment->att_database;
+		fb_assert(dbb->locked());
 		MemoryPool* const pool = attachment->att_pool;
 		Firebird::MemoryStats temp_stats;
 		pool->setStatsGroup(temp_stats);
+
 		delete attachment;
 
-		Database::SyncGuard sync(dbb);
 		dbb->deletePool(pool);
 	}
 }
@@ -6238,18 +6245,23 @@ static void purge_attachment(thread_db* tdbb, Attachment* attachment, const bool
 		attachment->att_trace_manager->event_detach(&conn, false);
 	}
 
-	// Unlink attachment from database
+	fb_assert(dbb->locked());
+	Database::Checkout dcoHolder(dbb);
 
+	{ // scope - take ast lock here
+		MutexLockGuard astGuard(attachment->mutex()->astMutex);
+		DatabaseContextHolder dbbHolder(tdbb);
+
+		// Unlink attachment from database
 		release_attachment(tdbb, attachment);
+	}
 
 	// If there are still attachments, do a partial shutdown
 
 	if (dbb->checkHandle())
 	{
-		fb_assert(dbb->locked());
 		if (!dbb->dbb_attachments)
 		{
-			Database::Checkout dcoHolder(dbb);
 			if (unlink_database(dbb))		// remove from linked list
 			{
 				shutdown_database(dbb, true);
@@ -6492,29 +6504,30 @@ static ISC_STATUS unwindAttach(const Exception& ex,
 			}
 		}
 
-		Database::SyncGuard syncGuard(dbb);
-
 		try
 		{
 			ThreadStatusGuard temp_status(tdbb);
 
 			if (attachment)
 			{
+				// no matter that noone has access to this attachment
+				// we need to care about lock ordering in unwind too 
+				// cause we may have locks and therefore AST calls
+				RefPtr<ExistenceMutex> attExistenceMutex(attachment->mutex());
+				MutexLockGuard astGuard(attachment->mutex()->astMutex);
+				Database::SyncGuard syncGuard(dbb);
+
 				release_attachment(tdbb, attachment);
 			}
 
-			if (dbb->checkHandle())
+			if (!dbb->dbb_attachments)	// small SS optimization - full check in unlink_database()
 			{
-				if (!dbb->dbb_attachments)
-				{
-					Database::Checkout dcoHolder(dbb);
 				if (unlink_database(dbb))		// remove from linked list
 				{
 					shutdown_database(dbb, true);
 				}
 			}
 		}
-		}
 		catch (const Exception&)
 		{
 			// no-op
@@ -7299,3 +7312,47 @@ void JRD_shutdown_attachments(const Database* dbb)
 	catch (const Exception&)
 	{} // no-op
 }
+
+
+AstContextHolder::AstContextHolder(Database* dbb, Attachment* attachment)
+	: ThreadContextHolder(),
+	  AstAttachmentHolder(attachment),
+	  Database::SyncGuard(dbb, true)
+{
+	operator thread_db*()->setDatabase(dbb);
+	operator thread_db*()->setAttachment(attachment);
+}
+
+
+AstAttachmentHolder::AstAttachmentHolder(Attachment* attachment)
+	: mtx(attachment->isKnownHandle())
+{
+	if (attachment)
+	{
+		if (mtx)
+		{
+			mtx->astMutex.enter();
+			if (mtx->objectExists)
+			{
+				return;
+			}
+			destroy();
+		}
+
+		Arg::Gds(isc_att_shutdown).raise();
+	}
+}
+
+
+void AstAttachmentHolder::destroy()
+{
+	try
+	{
+		mtx->astMutex.leave();
+	}
+	catch (const Firebird::Exception&)
+	{
+		DtorException::devHalt();
+	}
+	mtx->release();
+}

src/jrd/jrd.h

@@ -857,6 +857,39 @@ private:
 };
 
 
+// AST attachment holder - controls AST mutex in attachment block
+
+class AstAttachmentHolder
+{
+public:
+	AstAttachmentHolder(Attachment* attachment);
+
+	~AstAttachmentHolder()
+	{
+		if (mtx)
+		{
+			destroy();
+		}
+	}
+
+private:
+	Firebird::ExistenceMutex* mtx;
+
+	void destroy();
+};
+
+// AST routines context helper
+
+class AstContextHolder :
+	public ThreadContextHolder,		// creates thread_db block for AST routine
+	public AstAttachmentHolder,		// controls AST mutex in attachment block
+	public Database::SyncGuard		// controls dbb_sync mutex
+{
+public:
+	AstContextHolder(Database* dbb, Attachment* attachment = NULL);
+};
+
+
 // duplicate context of firebird string to store in jrd_nod::nod_arg
 inline char* stringDup(MemoryPool& p, const Firebird::string& s)
 {

src/jrd/met.epp

@@ -4186,12 +4186,7 @@ static int blocking_ast_dsql_cache(void* ast_object)
 	try
 	{
 		Database* dbb = item->lock->lck_dbb;
-
+		AstContextHolder tdbb(dbb, item->lock->lck_attachment);
-		Database::SyncGuard dsGuard(dbb, true);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(item->lock->lck_attachment);
 
 		Jrd::ContextPoolHolder context(tdbb, 0);
 
@@ -4259,17 +4254,12 @@ static int blocking_ast_procedure(void* ast_object)
 
 	try
 	{
+		if (procedure->prc_existence_lock) {
 			Database* dbb = procedure->prc_existence_lock->lck_dbb;
-
+			AstContextHolder tdbb(dbb, procedure->prc_existence_lock->lck_attachment);
-		Database::SyncGuard dsGuard(dbb, true);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(procedure->prc_existence_lock->lck_attachment);
 
 			Jrd::ContextPoolHolder context(tdbb, 0);
 
-		if (procedure->prc_existence_lock) {
 			LCK_release(tdbb, procedure->prc_existence_lock);
 		}
 		procedure->prc_flags |= PRC_obsolete;
@@ -4302,12 +4292,7 @@ static int blocking_ast_relation(void* ast_object)
 	try
 	{
 		Database* dbb = relation->rel_existence_lock->lck_dbb;
-
+		AstContextHolder tdbb(dbb, relation->rel_existence_lock->lck_attachment);
-		Database::SyncGuard dsGuard(dbb, true);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(relation->rel_existence_lock->lck_attachment);
 
 		Jrd::ContextPoolHolder context(tdbb, 0);
 
@@ -4335,12 +4320,7 @@ static int partners_ast_relation(void* ast_object)
 	try
 	{
 		Database* dbb = relation->rel_partners_lock->lck_dbb;
-
+		AstContextHolder tdbb(dbb, relation->rel_partners_lock->lck_attachment);
-		Database::SyncGuard dsGuard(dbb, true);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(relation->rel_partners_lock->lck_attachment);
 
 		Jrd::ContextPoolHolder context(tdbb, 0);
 

src/jrd/pag.cpp

@@ -2038,11 +2038,7 @@ static int blocking_ast_shutdown_attachment(void* ast_object)
 	try
 	{
 		Database* const dbb = attachment->att_database;
-		Database::SyncGuard dsGuard(dbb, true);
+		AstContextHolder tdbb(dbb, attachment);
-
-		ThreadContextHolder tdbb;
-		tdbb->setDatabase(dbb);
-		tdbb->setAttachment(attachment);
 
 		Jrd::ContextPoolHolder context(tdbb, dbb->dbb_permanent);
 

src/jrd/sdw.cpp

@@ -1174,14 +1174,11 @@ static int blocking_ast_shadowing(void* ast_object)
 
 	try
 	{
-		Database::SyncGuard dsGuard(new_dbb, true);
-
-		Lock* lock = new_dbb->dbb_shadow_lock;
-
 		// Since this routine will be called asynchronously,
 		// we must establish a thread context
-		ThreadContextHolder tdbb;
+		AstContextHolder tdbb(new_dbb);
-		tdbb->setDatabase(new_dbb);
+
+		Lock* lock = new_dbb->dbb_shadow_lock;
 
 		new_dbb->dbb_ast_flags |= DBB_get_shadows;
 		if (LCK_read_data(tdbb, lock) & SDW_rollover)