diff --git a/src/common/ThreadStart.cpp b/src/common/ThreadStart.cpp index 309e3da45b..574bc0e040 100644 --- a/src/common/ThreadStart.cpp +++ b/src/common/ThreadStart.cpp @@ -38,6 +38,7 @@ #ifdef WIN_NT #include #include +#include "../common/dllinst.h" #endif #ifdef HAVE_UNISTD_H @@ -319,7 +320,12 @@ void Thread::start(ThreadEntryPoint* routine, void* arg, int priority_arg, Handl void Thread::waitForCompletion(Handle& handle) { - WaitForSingleObject(handle, 500); + // When current DLL is unloading, OS loader holds loader lock. When thread is + // exiting, OS notifies every DLL about it, and acquires loader lock. In such + // scenario waiting on thread handle will never succeed. + if (!Firebird::dDllUnloadTID) { + WaitForSingleObject(handle, 500); + } CloseHandle(handle); handle = 0; } diff --git a/src/common/dllinst.cpp b/src/common/dllinst.cpp index b21121722f..aafd10a749 100644 --- a/src/common/dllinst.cpp +++ b/src/common/dllinst.cpp @@ -35,6 +35,7 @@ namespace Firebird { HINSTANCE hDllInst = 0; bool bDllProcessExiting = false; +DWORD dDllUnloadTID = 0; } // namespace diff --git a/src/common/dllinst.h b/src/common/dllinst.h index 02f9d1aa9d..678b6b47c4 100644 --- a/src/common/dllinst.h +++ b/src/common/dllinst.h @@ -37,6 +37,7 @@ namespace Firebird { extern HINSTANCE hDllInst; extern bool bDllProcessExiting; +extern DWORD dDllUnloadTID; } // namespace diff --git a/src/common/os/win32/mod_loader.cpp b/src/common/os/win32/mod_loader.cpp index 2d5ae12cf2..9c9e4fcbfd 100644 --- a/src/common/os/win32/mod_loader.cpp +++ b/src/common/os/win32/mod_loader.cpp @@ -226,7 +226,9 @@ ModuleLoader::Module* ModuleLoader::loadModule(const PathName& modPath) Win32Module::~Win32Module() { - if (module) + // If we in process of unloading of some DLL, don't unload modules manually + // else we could hang up waiting for OS loader lock. + if (module && !dDllUnloadTID) FreeLibrary(module); } diff --git a/src/jrd/CryptoManager.cpp b/src/jrd/CryptoManager.cpp index 9febe3fc14..fd53c46bdb 100644 --- a/src/jrd/CryptoManager.cpp +++ b/src/jrd/CryptoManager.cpp @@ -332,7 +332,7 @@ namespace Jrd { // ready to go guard.leave(); // release in advance to avoid races with cryptThread() - Thread::start(cryptThreadStatic, (THREAD_ENTRY_PARAM) this, 0, &cryptThreadId); + Thread::start(cryptThreadStatic, (THREAD_ENTRY_PARAM) this, THREAD_medium, &cryptThreadId); } catch (const Firebird::Exception&) { diff --git a/src/jrd/os/win32/ibinitdll.cpp b/src/jrd/os/win32/ibinitdll.cpp index 6b398757d8..7ea8106e93 100644 --- a/src/jrd/os/win32/ibinitdll.cpp +++ b/src/jrd/os/win32/ibinitdll.cpp @@ -47,6 +47,7 @@ BOOL WINAPI DllMain(HINSTANCE h, DWORD reason, LPVOID reserved) case DLL_PROCESS_DETACH: bDllProcessExiting = (reserved != NULL); + dDllUnloadTID = GetCurrentThreadId(); break; } diff --git a/src/jrd/svc.cpp b/src/jrd/svc.cpp index b609027eed..7cb59c6362 100644 --- a/src/jrd/svc.cpp +++ b/src/jrd/svc.cpp @@ -849,7 +849,7 @@ void Service::detach() if (localDoShutdown) { // run in separate thread to avoid blocking in remote - Thread::start(svcShutdownThread, 0, 0); + Thread::start(svcShutdownThread, 0, THREAD_medium); } } diff --git a/src/jrd/trace/TraceConfigStorage.cpp b/src/jrd/trace/TraceConfigStorage.cpp index cc5252824c..499d3cd9c2 100644 --- a/src/jrd/trace/TraceConfigStorage.cpp +++ b/src/jrd/trace/TraceConfigStorage.cpp @@ -136,7 +136,16 @@ ConfigStorage::ConfigStorage() ConfigStorage::~ConfigStorage() { + fb_assert(!m_timer); +} + +void ConfigStorage::shutdown() +{ + if (!m_timer) + return; + m_timer->stop(); + m_timer = NULL; ::close(m_cfg_file); m_cfg_file = -1; @@ -153,6 +162,7 @@ ConfigStorage::~ConfigStorage() m_sharedMemory->removeMapFile(); } } + m_sharedMemory = NULL; } void ConfigStorage::mutexBug(int state, const TEXT* string) diff --git a/src/jrd/trace/TraceConfigStorage.h b/src/jrd/trace/TraceConfigStorage.h index 4a580dac0c..ca973c0567 100644 --- a/src/jrd/trace/TraceConfigStorage.h +++ b/src/jrd/trace/TraceConfigStorage.h @@ -65,6 +65,7 @@ public: void acquire(); void release(); + void shutdown(); private: void mutexBug(int osErrorCode, const char* text); bool initialize(Firebird::SharedMemoryBase*, bool); diff --git a/src/jrd/trace/TraceManager.cpp b/src/jrd/trace/TraceManager.cpp index 9bfc154a2b..e1fc3aa638 100644 --- a/src/jrd/trace/TraceManager.cpp +++ b/src/jrd/trace/TraceManager.cpp @@ -163,6 +163,7 @@ void TraceManager::shutdown() init_factories = false; } } + getStorage()->shutdown(); } diff --git a/src/yvalve/MasterImplementation.cpp b/src/yvalve/MasterImplementation.cpp index 2257800ee1..830e48ddf3 100644 --- a/src/yvalve/MasterImplementation.cpp +++ b/src/yvalve/MasterImplementation.cpp @@ -43,6 +43,7 @@ #include "../common/isc_proto.h" #include "../common/ThreadStart.h" #include "../common/utils_proto.h" +#include "../common/dllinst.h" #include "../jrd/ibase.h" #include "../yvalve/utl_proto.h" @@ -167,6 +168,7 @@ GlobalPtr timerAccess; GlobalPtr timerPause; GlobalPtr timerWakeup; +GlobalPtr timerCleanup; // Should use atomic flag for thread stop to provide correct membar AtomicCounter stopTimerThread(0); @@ -180,7 +182,7 @@ struct TimerEntry static void init() { - Thread::start(timeThread, 0, 0, &timerThreadHandle); + Thread::start(timeThread, 0, THREAD_high, &timerThreadHandle); } static void cleanup(); @@ -199,6 +201,7 @@ void TimerEntry::cleanup() stopTimerThread.setValue(1); timerWakeup->release(); } + timerCleanup->tryEnter(5); Thread::waitForCompletion(timerThreadHandle); while (timerQueue->hasData()) @@ -241,7 +244,25 @@ TimerEntry* getTimer(ITimer* timer) THREAD_ENTRY_DECLARE TimerEntry::timeThread(THREAD_ENTRY_PARAM) { - while (stopTimerThread.value() == 0) +#ifdef WIN_NT + // The timer thread could unload plugins. Plugins almost always linked with + // dispatcher (fbclient.dll) thus, when plugin unloaded it decrement usage + // count of fbclient.dll. If application unload fbclient.dll not calling + // fb_shutdown, then last unloaded plugin will finally unload fbclient.dll + // and the code that is currently running, leading to AV. + // To prevent such scenario we increment usage count of fbclient.dll and + // will decrement it in safe way at the end of the timer thread. + + char buff[MAX_PATH]; + GetModuleFileName(hDllInst, buff, sizeof(buff)); + HMODULE hDll = LoadLibrary(buff); +#endif + + while (stopTimerThread.value() == 0 +#ifdef WIN_NT + && Firebird::dDllUnloadTID == 0 +#endif + ) { ISC_UINT64 microSeconds = 0; @@ -251,7 +272,7 @@ THREAD_ENTRY_DECLARE TimerEntry::timeThread(THREAD_ENTRY_PARAM) const ISC_UINT64 cur = curTime(); - while (timerQueue->getCount() > 0) + if (timerQueue->getCount() > 0) { TimerEntry e(timerQueue->operator[](0)); @@ -264,11 +285,11 @@ THREAD_ENTRY_DECLARE TimerEntry::timeThread(THREAD_ENTRY_PARAM) e.timer->handler(); e.timer->release(); + continue; } else { microSeconds = e.fireTime - cur; - break; } } } @@ -283,6 +304,19 @@ THREAD_ENTRY_DECLARE TimerEntry::timeThread(THREAD_ENTRY_PARAM) } } + timerCleanup->release(); + +#ifdef WIN_NT + if (Firebird::dDllUnloadTID) + // fb_shutdown is called as result of FreeLibrary, not by application. + // Sooner of all we are the last user of fbclient.dll, and code will be + // physically unloaded as result of FreeLibrary() call. + FreeLibraryAndExitThread(hDll, 0); + else + // It is safe to decrement usage count here + FreeLibrary(hDll); +#endif + return 0; } diff --git a/src/yvalve/PluginManager.cpp b/src/yvalve/PluginManager.cpp index 350ac7f04b..249ea8376a 100644 --- a/src/yvalve/PluginManager.cpp +++ b/src/yvalve/PluginManager.cpp @@ -41,6 +41,7 @@ #include "../common/ScanDir.h" #include "../common/classes/GenericMap.h" #include "../common/db_alias.h" +#include "../common/dllinst.h" #include "../yvalve/config/os/config_root.h" @@ -1050,6 +1051,10 @@ void PluginManager::unregisterModule(IPluginModule* cleanup) // and only if it's unloaded not by PluginManager, but by OS. // That means that task is closing unexpectedly - sooner of all // exit() is called by client of embedded server. Shutdown ourselves. +#ifdef WIN_NT + if (!Firebird::dDllUnloadTID) + Firebird::dDllUnloadTID = GetCurrentThreadId(); +#endif fb_shutdown(5000, fb_shutrsn_exit_called); }