From e1ffc620633312039a03dcb9d1d550ba8eaaba48 Mon Sep 17 00:00:00 2001 From: AlexPeshkoff Date: Fri, 18 Dec 2020 21:13:17 +0300 Subject: [PATCH] Another (and hopefully better) approach to CORE-5197 --- src/gpre/boot/gpre_meta_boot.cpp | 3 + src/include/firebird/FirebirdInterface.idl | 4 + src/include/firebird/IdlFbInterfaces.h | 24 ++++++ src/include/gen/Firebird.pas | 20 +++++ src/jrd/CryptoManager.cpp | 2 +- src/jrd/Database.cpp | 9 +-- src/jrd/Database.h | 4 +- src/jrd/jrd.cpp | 9 --- src/jrd/tra.cpp | 76 ++++--------------- src/jrd/vio.cpp | 13 ---- src/yvalve/MasterImplementation.cpp | 88 ++++++++++++++++++++++ src/yvalve/MasterImplementation.h | 2 + 12 files changed, 160 insertions(+), 94 deletions(-) diff --git a/src/gpre/boot/gpre_meta_boot.cpp b/src/gpre/boot/gpre_meta_boot.cpp index a153cc6d740..7149aa2cb31 100644 --- a/src/gpre/boot/gpre_meta_boot.cpp +++ b/src/gpre/boot/gpre_meta_boot.cpp @@ -767,6 +767,9 @@ class DummyMasterImpl : public IMasterImpl { return false; } + + void backgroundDbProcessing(CheckStatusWrapper*, const char*, unsigned, const UCHAR*, ICryptKeyCallback*) + { } }; diff --git a/src/include/firebird/FirebirdInterface.idl b/src/include/firebird/FirebirdInterface.idl index 4b81314090c..5c7438fd939 100644 --- a/src/include/firebird/FirebirdInterface.idl +++ b/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 diff --git a/src/include/firebird/IdlFbInterfaces.h b/src/include/firebird/IdlFbInterfaces.h index 236e9835b81..ecbe71fa47f 100644 --- a/src/include/firebird/IdlFbInterfaces.h +++ b/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(this->cloopVTable)->getProcessExiting(this); return ret; } + + template void backgroundDbProcessing(StatusType* status, const char* dbName, unsigned dpbLength, const unsigned char* dpb, ICryptKeyCallback* cryptCallback) + { + StatusType::clearException(status); + static_cast(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(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(self)->Name::backgroundDbProcessing(&status2, dbName, dpbLength, dpb, cryptCallback); + } + catch (...) + { + StatusType::catchException(&status2); + } + } }; template > > @@ -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 diff --git a/src/include/gen/Firebird.pas b/src/include/gen/Firebird.pas index 1014d699e1e..632b34df4dc 100644 --- a/src/include/gen/Firebird.pas +++ b/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; diff --git a/src/jrd/CryptoManager.cpp b/src/jrd/CryptoManager.cpp index 51fcb2b0d1e..b9f78ce06f0 100644 --- a/src/jrd/CryptoManager.cpp +++ b/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) diff --git a/src/jrd/Database.cpp b/src/jrd/Database.cpp index 49516c7afc1..add4ac8a3f0 100644 --- a/src/jrd/Database.cpp +++ b/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(); diff --git a/src/jrd/Database.h b/src/jrd/Database.h index 4cb03358e77..0aef9a6bba2 100644 --- a/src/jrd/Database.h +++ b/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 CryptoManager* dbb_crypto_manager; Firebird::RefPtr dbb_init_fini; Firebird::XThreadMutex dbb_thread_mutex; // special threads start/stop mutex - Thread::Handle dbb_sweep_thread; Firebird::RefPtr dbb_linger_timer; unsigned dbb_linger_seconds; time_t dbb_linger_end; @@ -606,7 +605,6 @@ class Database : public pool_alloc 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), diff --git a/src/jrd/jrd.cpp b/src/jrd/jrd.cpp index d9e444fbd79..5b08cc17171 100644 --- a/src/jrd/jrd.cpp +++ b/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 diff --git a/src/jrd/tra.cpp b/src/jrd/tra.cpp index da8188e78d2..17724218e1f 100644 --- a/src/jrd/tra.cpp +++ b/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 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(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) diff --git a/src/jrd/vio.cpp b/src/jrd/vio.cpp index c646a83d7c2..3fff0aa3c87 100644 --- a/src/jrd/vio.cpp +++ b/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; diff --git a/src/yvalve/MasterImplementation.cpp b/src/yvalve/MasterImplementation.cpp index 74fd7b29f2d..356fd299de7 100644 --- a/src/yvalve/MasterImplementation.cpp +++ b/src/yvalve/MasterImplementation.cpp @@ -44,6 +44,7 @@ #include "../common/ThreadStart.h" #include "../common/utils_proto.h" #include "../common/dllinst.h" +#include "../common/status.h" #include "ibase.h" #include "../yvalve/utl_proto.h" @@ -121,6 +122,93 @@ int MasterImplementation::serverMode(int mode) return currentMode; } + +class AttachParams : public GlobalStorage +{ +public: + AttachParams(const PathName& db, unsigned len, const UCHAR* d, ICryptKeyCallback*) + : dbName(getPool(), db), dpb(getPool()), cryptCallback(nullptr) + { + dpb.assign(d, len); + } + + AttachParams(const AttachParams& p) + : dbName(getPool(), p.dbName), dpb(getPool(), p.dpb), cryptCallback(p.cryptCallback) + { } + + AttachParams& operator=(const AttachParams& p) + { + dbName = p.dbName; + dpb = p.dpb; + cryptCallback = p.cryptCallback; + + return *this; + } + + static THREAD_ENTRY_DECLARE runTask(THREAD_ENTRY_PARAM p) + { + try + { + AttachParams* par = (AttachParams*) p; + AttachParams params(*par); + par->sem.release(); + +#ifdef NEVERDEF + // small delay to debug fbclient unload at problematic moment + long long x = 0x10000000; + while (--x > 0); +#endif + + FbLocalStatus status; + DispatcherPtr prov; + if (params.cryptCallback) + { + prov->setDbCryptCallback(&status, params.cryptCallback); + status.check(); + } + + RefPtr att(REF_NO_INCR, + prov->attachDatabase(&status, params.dbName.c_str(), params.dpb.getCount(), params.dpb.begin())); + status.check(); + } + catch(const Exception& ex) + { + iscLogException("Automatic sweep error", ex); + } + + return 0; + } + + void wait() + { + sem.enter(); + } + +private: + Semaphore sem; + PathName dbName; + UCharBuffer dpb; + ICryptKeyCallback* cryptCallback; +}; + +void MasterImplementation::backgroundDbProcessing(CheckStatusWrapper* status, const char* dbName, + unsigned dpbLength, const UCHAR* d, ICryptKeyCallback* cryptCallback) +{ + AutoDispose dpb(UtilInterfacePtr()->getXpbBuilder(status, IXpbBuilder::DPB, d, dpbLength)); + check(status); + bool hasLogin = dpb->findFirst(status, isc_dpb_user_name); + check(status); + if (!hasLogin) + { + dpb->insertString(status, isc_dpb_user_name, "SYSDBA"); + check(status); + } + + AttachParams par(dbName, dpb->getBufferLength(status), dpb->getBuffer(status), cryptCallback); + Thread::start(AttachParams::runTask, &par, THREAD_medium); + par.wait(); +} + } // namespace Why // diff --git a/src/yvalve/MasterImplementation.h b/src/yvalve/MasterImplementation.h index cdf90099970..b77352f7570 100644 --- a/src/yvalve/MasterImplementation.h +++ b/src/yvalve/MasterImplementation.h @@ -64,6 +64,8 @@ namespace Why Firebird::IUtil* getUtilInterface(); Firebird::IConfigManager* getConfigManager(); FB_BOOLEAN getProcessExiting(); + void backgroundDbProcessing(Firebird::CheckStatusWrapper* status, const char* dbName, + unsigned dpbLength, const UCHAR* dpb, Firebird::ICryptKeyCallback* cryptCallback); }; void shutdownTimers();