Another (and hopefully better) approach to CORE-5197

src/gpre/boot/gpre_meta_boot.cpp

@@ -767,6 +767,9 @@ class DummyMasterImpl : public IMasterImpl<DummyMasterImpl, CheckStatusWrapper>
 	{
 		return false;
 	}
+
+	void backgroundDbProcessing(CheckStatusWrapper*, const char*, unsigned, const UCHAR*, ICryptKeyCallback*)
+	{ }
 };
 
 

src/include/firebird/FirebirdInterface.idl

@@ -102,8 +102,12 @@ interface Master : Versioned
 	Util getUtilInterface();
 	ConfigManager getConfigManager();
 	boolean getProcessExiting();
+
+	void backgroundDbProcessing(Status status, const string dbName, uint dpbLength, const uchar* dpb,
+		CryptKeyCallback cryptCallback);
 }
 
+
 /*
  * Firebird plugins are accessed using methods of PluginLoader interface.
  * For each plugin_module tag found, it constructs a Plugin object, reads the corresponding

src/include/firebird/IdlFbInterfaces.h

@@ -313,6 +313,7 @@ namespace Firebird
 			IUtil* (CLOOP_CARG *getUtilInterface)(IMaster* self) throw();
 			IConfigManager* (CLOOP_CARG *getConfigManager)(IMaster* self) throw();
 			FB_BOOLEAN (CLOOP_CARG *getProcessExiting)(IMaster* self) throw();
+			void (CLOOP_CARG *backgroundDbProcessing)(IMaster* self, IStatus* status, const char* dbName, unsigned dpbLength, const unsigned char* dpb, ICryptKeyCallback* cryptCallback) throw();
 		};
 
 	protected:
@@ -401,6 +402,13 @@ namespace Firebird
 			FB_BOOLEAN ret = static_cast<VTable*>(this->cloopVTable)->getProcessExiting(this);
 			return ret;
 		}
+
+		template <typename StatusType> void backgroundDbProcessing(StatusType* status, const char* dbName, unsigned dpbLength, const unsigned char* dpb, ICryptKeyCallback* cryptCallback)
+		{
+			StatusType::clearException(status);
+			static_cast<VTable*>(this->cloopVTable)->backgroundDbProcessing(this, status, dbName, dpbLength, dpb, cryptCallback);
+			StatusType::checkException(status);
+		}
 	};
 
 	class IPluginBase : public IReferenceCounted
@@ -6695,6 +6703,7 @@ namespace Firebird
 					this->getUtilInterface = &Name::cloopgetUtilInterfaceDispatcher;
 					this->getConfigManager = &Name::cloopgetConfigManagerDispatcher;
 					this->getProcessExiting = &Name::cloopgetProcessExitingDispatcher;
+					this->backgroundDbProcessing = &Name::cloopbackgroundDbProcessingDispatcher;
 				}
 			} vTable;
 
@@ -6858,6 +6867,20 @@ namespace Firebird
 				return static_cast<FB_BOOLEAN>(0);
 			}
 		}
+
+		static void CLOOP_CARG cloopbackgroundDbProcessingDispatcher(IMaster* self, IStatus* status, const char* dbName, unsigned dpbLength, const unsigned char* dpb, ICryptKeyCallback* cryptCallback) throw()
+		{
+			StatusType status2(status);
+
+			try
+			{
+				static_cast<Name*>(self)->Name::backgroundDbProcessing(&status2, dbName, dpbLength, dpb, cryptCallback);
+			}
+			catch (...)
+			{
+				StatusType::catchException(&status2);
+			}
+		}
 	};
 
 	template <typename Name, typename StatusType, typename Base = IVersionedImpl<Name, StatusType, Inherit<IMaster> > >
@@ -6885,6 +6908,7 @@ namespace Firebird
 		virtual IUtil* getUtilInterface() = 0;
 		virtual IConfigManager* getConfigManager() = 0;
 		virtual FB_BOOLEAN getProcessExiting() = 0;
+		virtual void backgroundDbProcessing(StatusType* status, const char* dbName, unsigned dpbLength, const unsigned char* dpb, ICryptKeyCallback* cryptCallback) = 0;
 	};
 
 	template <typename Name, typename StatusType, typename Base>

src/include/gen/Firebird.pas

@@ -228,6 +228,7 @@ 	ISC_TIMESTAMP_TZ_EX = record
 	IMaster_getUtilInterfacePtr = function(this: IMaster): IUtil; cdecl;
 	IMaster_getConfigManagerPtr = function(this: IMaster): IConfigManager; cdecl;
 	IMaster_getProcessExitingPtr = function(this: IMaster): Boolean; cdecl;
+	IMaster_backgroundDbProcessingPtr = procedure(this: IMaster; status: IStatus; dbName: PAnsiChar; dpbLength: Cardinal; dpb: BytePtr; cryptCallback: ICryptKeyCallback); cdecl;
 	IPluginBase_setOwnerPtr = procedure(this: IPluginBase; r: IReferenceCounted); cdecl;
 	IPluginBase_getOwnerPtr = function(this: IPluginBase): IReferenceCounted; cdecl;
 	IPluginSet_getNamePtr = function(this: IPluginSet): PAnsiChar; cdecl;
@@ -806,6 +807,7 @@ 	MasterVTable = class(VersionedVTable)
 		getUtilInterface: IMaster_getUtilInterfacePtr;
 		getConfigManager: IMaster_getConfigManagerPtr;
 		getProcessExiting: IMaster_getProcessExitingPtr;
+		backgroundDbProcessing: IMaster_backgroundDbProcessingPtr;
 	end;
 
 	IMaster = class(IVersioned)
@@ -823,6 +825,7 @@ 	IMaster = class(IVersioned)
 		function getUtilInterface(): IUtil;
 		function getConfigManager(): IConfigManager;
 		function getProcessExiting(): Boolean;
+		procedure backgroundDbProcessing(status: IStatus; dbName: PAnsiChar; dpbLength: Cardinal; dpb: BytePtr; cryptCallback: ICryptKeyCallback);
 	end;
 
 	IMasterImpl = class(IMaster)
@@ -840,6 +843,7 @@ 	IMasterImpl = class(IMaster)
 		function getUtilInterface(): IUtil; virtual; abstract;
 		function getConfigManager(): IConfigManager; virtual; abstract;
 		function getProcessExiting(): Boolean; virtual; abstract;
+		procedure backgroundDbProcessing(status: IStatus; dbName: PAnsiChar; dpbLength: Cardinal; dpb: BytePtr; cryptCallback: ICryptKeyCallback); virtual; abstract;
 	end;
 
 	PluginBaseVTable = class(ReferenceCountedVTable)
@@ -5674,6 +5678,12 @@ function IMaster.getProcessExiting(): Boolean;
 	Result := MasterVTable(vTable).getProcessExiting(Self);
 end;
 
+procedure IMaster.backgroundDbProcessing(status: IStatus; dbName: PAnsiChar; dpbLength: Cardinal; dpb: BytePtr; cryptCallback: ICryptKeyCallback);
+begin
+	MasterVTable(vTable).backgroundDbProcessing(Self, status, dbName, dpbLength, dpb, cryptCallback);
+	FbException.checkException(status);
+end;
+
 procedure IPluginBase.setOwner(r: IReferenceCounted);
 begin
 	PluginBaseVTable(vTable).setOwner(Self, r);
@@ -8546,6 +8556,15 @@ function IMasterImpl_getProcessExitingDispatcher(this: IMaster): Boolean; cdecl;
 	end
 end;
 
+procedure IMasterImpl_backgroundDbProcessingDispatcher(this: IMaster; status: IStatus; dbName: PAnsiChar; dpbLength: Cardinal; dpb: BytePtr; cryptCallback: ICryptKeyCallback); cdecl;
+begin
+	try
+		IMasterImpl(this).backgroundDbProcessing(status, dbName, dpbLength, dpb, cryptCallback);
+	except
+		on e: Exception do FbException.catchException(status, e);
+	end
+end;
+
 var
 	IMasterImpl_vTable: MasterVTable;
 
@@ -14730,6 +14749,7 @@ initialization
 	IMasterImpl_vTable.getUtilInterface := @IMasterImpl_getUtilInterfaceDispatcher;
 	IMasterImpl_vTable.getConfigManager := @IMasterImpl_getConfigManagerDispatcher;
 	IMasterImpl_vTable.getProcessExiting := @IMasterImpl_getProcessExitingDispatcher;
+	IMasterImpl_vTable.backgroundDbProcessing := @IMasterImpl_backgroundDbProcessingDispatcher;
 
 	IPluginBaseImpl_vTable := PluginBaseVTable.create;
 	IPluginBaseImpl_vTable.version := 3;

src/jrd/CryptoManager.cpp

@@ -1347,7 +1347,7 @@ namespace Jrd {
 
 	bool CryptoManager::down() const
 	{
-		return flDown || (dbb.dbb_flags & DBB_closing);
+		return flDown;
 	}
 
 	void CryptoManager::addClumplet(string& signature, ClumpletReader& block, UCHAR tag)

src/jrd/Database.cpp

@@ -224,13 +224,6 @@ namespace Jrd
 			return false;
 		}
 
-		if (dbb_flags & DBB_closing)
-		{
-			SPTHR_DEBUG(fprintf(stderr, "allowSweepThread false, dbb closing\n"));
-			dbb_thread_mutex.leave();
-			return false;
-		}
-
 		while (true)
 		{
 			AtomicCounter::counter_type old = dbb_flags;
@@ -285,7 +278,7 @@ namespace Jrd
 	{
 		SPTHR_DEBUG(fprintf(stderr, "allowSweepRun %p\n", this));
 
-		if (readOnly() || (dbb_flags & DBB_closing))
+		if (readOnly())
 			return false;
 
 		Jrd::Attachment* const attachment = tdbb->getAttachment();

src/jrd/Database.h

@@ -236,7 +236,7 @@ const ULONG DBB_no_fs_cache				= 0x40000L;		// Not using file system cache
 const ULONG DBB_sweep_starting			= 0x80000L;		// Auto-sweep is starting
 const ULONG DBB_creating				= 0x100000L;	// Database creation is in progress
 const ULONG DBB_shared					= 0x200000L;	// Database object is shared among connections
-const ULONG DBB_closing					= 0x400000L;	// Database closing, special backgroud threads should exit
+//const ULONG DBB_closing					= 0x400000L;	// Database closing, special backgroud threads should exit
 
 //
 // dbb_ast_flags
@@ -530,7 +530,6 @@ class Database : public pool_alloc<type_dbb>
 	CryptoManager* dbb_crypto_manager;
 	Firebird::RefPtr<ExistenceRefMutex> dbb_init_fini;
 	Firebird::XThreadMutex dbb_thread_mutex;		// special threads start/stop mutex
-	Thread::Handle dbb_sweep_thread;
 	Firebird::RefPtr<Linger> dbb_linger_timer;
 	unsigned dbb_linger_seconds;
 	time_t dbb_linger_end;
@@ -606,7 +605,6 @@ class Database : public pool_alloc<type_dbb>
 		dbb_creation_date(Firebird::TimeZoneUtil::getCurrentGmtTimeStamp()),
 		dbb_external_file_directory_list(NULL),
 		dbb_init_fini(FB_NEW_POOL(*getDefaultMemoryPool()) ExistenceRefMutex()),
-		dbb_sweep_thread(0),
 		dbb_linger_seconds(0),
 		dbb_linger_end(0),
 		dbb_plugin_config(pConf),

src/jrd/jrd.cpp

@@ -7470,8 +7470,6 @@ void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment)
 		}
 
 		// Notify special threads
-		if (!other)
-			dbb->dbb_flags |= DBB_closing;
 		threadGuard.leave();
 
 		// Sync with special threads
@@ -7482,13 +7480,6 @@ void release_attachment(thread_db* tdbb, Jrd::Attachment* attachment)
 			// crypt thread
 			if (dbb->dbb_crypto_manager)
 				dbb->dbb_crypto_manager->terminateCryptThread(tdbb, true);
-
-			// sweep thread
-			if (dbb->dbb_sweep_thread)
-			{
-				Thread::waitForCompletion(dbb->dbb_sweep_thread);
-				dbb->dbb_sweep_thread = 0;
-			}
 		}
 
 	} // EngineCheckout scope

src/jrd/tra.cpp

@@ -104,7 +104,7 @@ static void release_temp_tables(thread_db*, jrd_tra*);
 static void retain_temp_tables(thread_db*, jrd_tra*, TraNumber);
 static void restart_requests(thread_db*, jrd_tra*);
 static void start_sweeper(thread_db*);
-static THREAD_ENTRY_DECLARE sweep_database(THREAD_ENTRY_PARAM);
+//static THREAD_ENTRY_DECLARE sweep_database(THREAD_ENTRY_PARAM);
 static void transaction_flush(thread_db* tdbb, USHORT flush_flag, TraNumber tra_number);
 static void transaction_options(thread_db*, jrd_tra*, const UCHAR*, USHORT);
 static void transaction_start(thread_db* tdbb, jrd_tra* temp);
@@ -1831,13 +1831,6 @@ void TRA_sweep(thread_db* tdbb)
 
 	try {
 
-		// Avoid races with release_attachment()
-
-		XThreadEnsureUnlock releaseAttGuard(dbb->dbb_thread_mutex, FB_FUNCTION);
-		releaseAttGuard.enter();
-		if (dbb->dbb_flags & DBB_closing)
-			return;
-
 		// Identify ourselves as a sweeper thread. This accomplishes two goals:
 		// 1) Sweep transaction is started "precommitted" and
 		// 2) Execution is throttled in JRD_reschedule() by
@@ -1866,11 +1859,6 @@ void TRA_sweep(thread_db* tdbb)
 
 		attachment->att_flags &= ~ATT_notify_gc;
 
-		// Mark our attachment as special one
-
-		attachment->att_flags |= ATT_from_thread;
-		releaseAttGuard.leave();
-
 		if (VIO_sweep(tdbb, transaction, &traceSweep))
 		{
 			// At this point, we know that no record versions belonging to dead
@@ -2701,60 +2689,28 @@ static void start_sweeper(thread_db* tdbb)
 
 	TRA_update_counters(tdbb, dbb);
 
-	// pass dbb to sweep thread - if allowSweepThread() returned TRUE that is safe
-	try
+	CheckStatusWrapper* status = tdbb->tdbb_status_vector;
+	AutoDispose<IXpbBuilder> dpb(UtilInterfacePtr()->getXpbBuilder(status, IXpbBuilder::DPB, nullptr, 0));
+	check(status);
+	dpb->insertString(status, isc_dpb_user_name, "sweeper");
+	check(status);
+	UCHAR byte = isc_dpb_records;
+	dpb->insertBytes(status, isc_dpb_sweep, &byte, 1);
+	check(status);
+
+	MasterInterfacePtr()->backgroundDbProcessing(status, dbb->dbb_database_name.c_str(),
+		dpb->getBufferLength(status), dpb->getBuffer(status), dbb->dbb_callback);
+	if (status->getState() & IStatus::STATE_ERRORS)
 	{
-		Thread::start(sweep_database, dbb, THREAD_medium, &dbb->dbb_sweep_thread);
-		return;
-	}
-	catch (const Firebird::Exception& ex)
-	{
-		iscLogException("cannot start sweep thread", ex);
+		iscLogStatus("cannot start sweep thread", tdbb->tdbb_status_vector);
 		dbb->clearSweepFlags(tdbb);
 	}
 }
 
 
-static THREAD_ENTRY_DECLARE sweep_database(THREAD_ENTRY_PARAM d)
-{
-/**************************************
- *
- *	s w e e p _ d a t a b a s e
- *
- **************************************
- *
- * Functional description
- *	Sweep database.
- *
- **************************************/
-	// determine database name
-	// taking into an account that thread is started successfully
-	// we should take care about parameters reference counter and DBB flags
-	Database* dbb = (Database*) d;
-	try
-	{
-		ISC_STATUS_ARRAY status_vector = {0};
-		isc_db_handle db_handle = 0;
-
-		Firebird::ClumpletWriter dpb(Firebird::ClumpletReader::dpbList, MAX_DPB_SIZE);
-		dpb.insertByte(isc_dpb_sweep, isc_dpb_records);
-		// use embedded authentication to attach database
-		const char* szAuthenticator = "sweeper";
-		dpb.insertString(isc_dpb_user_name, szAuthenticator, fb_strlen(szAuthenticator));
-
-		isc_attach_database(status_vector, 0, dbb->dbb_database_name.c_str(),
-							&db_handle, dpb.getBufferLength(),
-							reinterpret_cast<const char*>(dpb.getBuffer()));
-		if (db_handle)
-			isc_detach_database(status_vector, &db_handle);
-	}
-	catch (const Exception&)
-	{ }
-
-	dbb->clearSweepStarting();	// actually needed here only for classic,
+/*	dbb->clearSweepStarting();	// actually needed here only for classic,
 								// but do danger calling for super
-	return 0;
-}
+*/
 
 
 static void transaction_flush(thread_db* tdbb, USHORT flush_flag, TraNumber tra_number)

src/jrd/vio.cpp

@@ -3899,13 +3899,6 @@ bool VIO_sweep(thread_db* tdbb, jrd_tra* transaction, TraceSweepEvent* traceSwee
 
 		for (FB_SIZE_T i = 1; (vector = attachment->att_relations) && i < vector->count(); i++)
 		{
-			if (dbb->dbb_flags & DBB_closing)
-			{
-				SPTHR_DEBUG(fprintf(stderr, "VIO_sweep exits\n"));
-				ret = false;
-				break;
-			}
-
 			relation = (*vector)[i];
 			if (relation)
 				relation = MET_lookup_relation_id(tdbb, i, false);
@@ -3936,12 +3929,6 @@ bool VIO_sweep(thread_db* tdbb, jrd_tra* transaction, TraceSweepEvent* traceSwee
 				{
 					CCH_RELEASE(tdbb, &rpb.getWindow(tdbb));
 
-					if (dbb->dbb_flags & DBB_closing)
-					{
-						SPTHR_DEBUG(fprintf(stderr, "VIO_sweep exits after VIO_next_record\n"));
-						break;
-					}
-
 					if (relation->rel_flags & REL_deleting)
 						break;