diff --git a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.cpp b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.cpp index f53b89adcc..2742a5cf5f 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.cpp @@ -22,7 +22,7 @@ DbgHelpGuard::DbgHelpGuard() - : m_hasLoaded(false) + : m_needsUnload(false) { activate(); } @@ -34,26 +34,16 @@ DbgHelpGuard::~DbgHelpGuard() void DbgHelpGuard::activate() { - if (DbgHelpLoader::isLoadedFromSystem()) - { - // This is ok. Do nothing. - } - else if (DbgHelpLoader::isLoaded()) - { - // This is maybe not ok. But do nothing until this becomes a user facing problem. - } - else - { - // Front load the DLL now to prevent other code from loading the potentially wrong DLL. - m_hasLoaded = DbgHelpLoader::load(); - } + // Front load the DLL now to prevent other code from loading the potentially wrong DLL. + DbgHelpLoader::load(); + m_needsUnload = true; } void DbgHelpGuard::deactivate() { - if (m_hasLoaded) + if (m_needsUnload) { DbgHelpLoader::unload(); - m_hasLoaded = false; + m_needsUnload = false; } } diff --git a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.h b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.h index 1a3e8931a4..c4ff06b280 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.h +++ b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpGuard.h @@ -40,5 +40,5 @@ class DbgHelpGuard private: - bool m_hasLoaded; + bool m_needsUnload; }; diff --git a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp index b5bcef62f4..96898d5097 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp @@ -20,6 +20,7 @@ DbgHelpLoader* DbgHelpLoader::Inst = NULL; +CriticalSectionClass DbgHelpLoader::CriticalSection; DbgHelpLoader::DbgHelpLoader() : m_symInitialize(NULL) @@ -33,6 +34,7 @@ DbgHelpLoader::DbgHelpLoader() , m_symFunctionTableAccess(NULL) , m_stackWalk(NULL) , m_dllModule(HMODULE(0)) + , m_referenceCount(0) , m_failed(false) , m_loadedFromSystem(false) { @@ -44,16 +46,29 @@ DbgHelpLoader::~DbgHelpLoader() bool DbgHelpLoader::isLoaded() { + CriticalSectionClass::LockClass lock(CriticalSection); + return Inst != NULL && Inst->m_dllModule != HMODULE(0); } bool DbgHelpLoader::isLoadedFromSystem() { - return Inst != NULL && Inst->m_loadedFromSystem; + CriticalSectionClass::LockClass lock(CriticalSection); + + return isLoaded() && Inst->m_loadedFromSystem; +} + +bool DbgHelpLoader::isFailed() +{ + CriticalSectionClass::LockClass lock(CriticalSection); + + return Inst != NULL && Inst->m_failed; } bool DbgHelpLoader::load() { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst == NULL) { // Cannot use new/delete here when this is loaded during game memory initialization. @@ -61,10 +76,17 @@ bool DbgHelpLoader::load() Inst = new (p) DbgHelpLoader(); } + // Always increment the reference count. + ++Inst->m_referenceCount; + // Optimization: return early if it failed before. if (Inst->m_failed) return false; + // Return early if someone else already loaded it. + if (Inst->m_referenceCount > 1) + return true; + // Try load dbghelp.dll from the system directory first. char dllFilename[MAX_PATH]; ::GetSystemDirectoryA(dllFilename, ARRAY_SIZE(dllFilename)); @@ -99,7 +121,7 @@ bool DbgHelpLoader::load() if (Inst->m_symInitialize == NULL || Inst->m_symCleanup == NULL) { - unload(); + freeResources(); Inst->m_failed = true; return false; } @@ -107,17 +129,27 @@ bool DbgHelpLoader::load() return true; } -bool DbgHelpLoader::reload() -{ - unload(); - return load(); -} - void DbgHelpLoader::unload() { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst == NULL) return; + if (--Inst->m_referenceCount != 0) + return; + + freeResources(); + + Inst->~DbgHelpLoader(); + GlobalFree(Inst); + Inst = NULL; +} + +void DbgHelpLoader::freeResources() +{ + // Is private. Needs no locking. + while (!Inst->m_initializedProcesses.empty()) { symCleanup(*Inst->m_initializedProcesses.begin()); @@ -129,9 +161,18 @@ void DbgHelpLoader::unload() Inst->m_dllModule = HMODULE(0); } - Inst->~DbgHelpLoader(); - GlobalFree(Inst); - Inst = NULL; + Inst->m_symInitialize = NULL; + Inst->m_symCleanup = NULL; + Inst->m_symLoadModule = NULL; + Inst->m_symUnloadModule = NULL; + Inst->m_symGetModuleBase = NULL; + Inst->m_symGetSymFromAddr = NULL; + Inst->m_symGetLineFromAddr = NULL; + Inst->m_symSetOptions = NULL; + Inst->m_symFunctionTableAccess = NULL; + Inst->m_stackWalk = NULL; + + Inst->m_loadedFromSystem = false; } BOOL DbgHelpLoader::symInitialize( @@ -139,6 +180,8 @@ BOOL DbgHelpLoader::symInitialize( LPSTR UserSearchPath, BOOL fInvadeProcess) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst == NULL) return FALSE; @@ -164,6 +207,8 @@ BOOL DbgHelpLoader::symInitialize( BOOL DbgHelpLoader::symCleanup( HANDLE hProcess) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst == NULL) return FALSE; @@ -186,6 +231,8 @@ BOOL DbgHelpLoader::symLoadModule( DWORD BaseOfDll, DWORD SizeOfDll) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_symLoadModule) return Inst->m_symLoadModule(hProcess, hFile, ImageName, ModuleName, BaseOfDll, SizeOfDll); @@ -196,6 +243,8 @@ DWORD DbgHelpLoader::symGetModuleBase( HANDLE hProcess, DWORD dwAddr) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_symGetModuleBase) return Inst->m_symGetModuleBase(hProcess, dwAddr); @@ -206,6 +255,8 @@ BOOL DbgHelpLoader::symUnloadModule( HANDLE hProcess, DWORD BaseOfDll) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_symUnloadModule) return Inst->m_symUnloadModule(hProcess, BaseOfDll); @@ -218,6 +269,8 @@ BOOL DbgHelpLoader::symGetSymFromAddr( LPDWORD Displacement, PIMAGEHLP_SYMBOL Symbol) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_symGetSymFromAddr) return Inst->m_symGetSymFromAddr(hProcess, Address, Displacement, Symbol); @@ -230,6 +283,8 @@ BOOL DbgHelpLoader::symGetLineFromAddr( PDWORD pdwDisplacement, PIMAGEHLP_LINE Line) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_symGetLineFromAddr) return Inst->m_symGetLineFromAddr(hProcess, dwAddr, pdwDisplacement, Line); @@ -239,6 +294,8 @@ BOOL DbgHelpLoader::symGetLineFromAddr( DWORD DbgHelpLoader::symSetOptions( DWORD SymOptions) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_symSetOptions) return Inst->m_symSetOptions(SymOptions); @@ -249,6 +306,8 @@ LPVOID DbgHelpLoader::symFunctionTableAccess( HANDLE hProcess, DWORD AddrBase) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_symFunctionTableAccess) return Inst->m_symFunctionTableAccess(hProcess, AddrBase); @@ -266,6 +325,8 @@ BOOL DbgHelpLoader::stackWalk( PGET_MODULE_BASE_ROUTINE GetModuleBaseRoutine, PTRANSLATE_ADDRESS_ROUTINE TranslateAddress) { + CriticalSectionClass::LockClass lock(CriticalSection); + if (Inst != NULL && Inst->m_stackWalk) return Inst->m_stackWalk(MachineType, hProcess, hThread, StackFrame, ContextRecord, ReadMemoryRoutine, FunctionTableAccessRoutine, GetModuleBaseRoutine, TranslateAddress); diff --git a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h index 7d83da31e5..c2167db075 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h +++ b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h @@ -24,9 +24,10 @@ #include // Must be included after Windows.h #include +#include "mutex.h" #include "SystemAllocator.h" -// This static class can load and unload dbghelp.dll +// This static class can load, unload and use dbghelp.dll. Is thread-safe. // Internally it must not use new and delete because it can be created during game memory initialization. class DbgHelpLoader @@ -34,6 +35,7 @@ class DbgHelpLoader private: static DbgHelpLoader* Inst; // Is singleton class + static CriticalSectionClass CriticalSection; // Required because dbg help is not thread safe for the most part DbgHelpLoader(); ~DbgHelpLoader(); @@ -46,8 +48,11 @@ class DbgHelpLoader // Returns whether dbghelp.dll is loaded from the system directory static bool isLoadedFromSystem(); + // Returns whether dbghelp.dll was attempted to be loaded but failed + static bool isFailed(); + + // Every call to load needs a paired call to unload, no matter if the load was successful static bool load(); - static bool reload(); static void unload(); static BOOL WINAPI symInitialize( @@ -106,6 +111,8 @@ class DbgHelpLoader private: + static void freeResources(); + typedef BOOL (WINAPI *SymInitialize_t) ( HANDLE hProcess, LPSTR UserSearchPath, @@ -175,6 +182,8 @@ class DbgHelpLoader Processes m_initializedProcesses; HMODULE m_dllModule; + int m_referenceCount; + CriticalSectionClass m_criticalSection; bool m_failed; bool m_loadedFromSystem; }; diff --git a/Core/Libraries/Source/WWVegas/WWLib/WWCommon.h b/Core/Libraries/Source/WWVegas/WWLib/WWCommon.h index 253c500bb8..9c41702c57 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/WWCommon.h +++ b/Core/Libraries/Source/WWVegas/WWLib/WWCommon.h @@ -38,6 +38,8 @@ enum #if defined(_MSC_VER) && _MSC_VER < 1300 typedef unsigned MemValueType; +typedef long Interlocked32; // To use with Interlocked functions #else typedef unsigned long long MemValueType; +typedef volatile long Interlocked32; // To use with Interlocked functions #endif diff --git a/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp b/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp index 38a8e7544d..a1c06976e6 100644 --- a/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp @@ -37,7 +37,6 @@ // Prototypes //***************************************************************************** BOOL InitSymbolInfo(void); -void UninitSymbolInfo(void); void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*)); void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* linenumber, unsigned int* address); void WriteStackLine(void*address, void (*callback)(const char*)); @@ -109,10 +108,14 @@ BOOL InitSymbolInfo() if (DbgHelpLoader::isLoaded()) return TRUE; - if (!DbgHelpLoader::load()) + if (DbgHelpLoader::isFailed()) return FALSE; - atexit(UninitSymbolInfo); + if (!DbgHelpLoader::load()) + { + atexit(DbgHelpLoader::unload); + return FALSE; + } char pathname[_MAX_PATH+1]; char drive[10]; @@ -140,24 +143,16 @@ BOOL InitSymbolInfo() if(DbgHelpLoader::symLoadModule(process, NULL, pathname, NULL, 0, 0)) { //Load any other relevant modules (ie dlls) here + atexit(DbgHelpLoader::unload); return TRUE; } } DbgHelpLoader::unload(); - return(FALSE); + return FALSE; } -//***************************************************************************** -//***************************************************************************** -void UninitSymbolInfo(void) -{ - DbgHelpLoader::unload(); -} - - - //***************************************************************************** //***************************************************************************** void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*)) diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp index 5dcb35ce1e..b6a48d4737 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp @@ -37,7 +37,6 @@ // Prototypes //***************************************************************************** BOOL InitSymbolInfo(void); -void UninitSymbolInfo(void); void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*)); void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* linenumber, unsigned int* address); void WriteStackLine(void*address, void (*callback)(const char*)); @@ -109,10 +108,14 @@ BOOL InitSymbolInfo() if (DbgHelpLoader::isLoaded()) return TRUE; - if (!DbgHelpLoader::load()) + if (DbgHelpLoader::isFailed()) return FALSE; - atexit(UninitSymbolInfo); + if (!DbgHelpLoader::load()) + { + atexit(DbgHelpLoader::unload); + return FALSE; + } char pathname[_MAX_PATH+1]; char drive[10]; @@ -140,24 +143,16 @@ BOOL InitSymbolInfo() if(DbgHelpLoader::symLoadModule(process, NULL, pathname, NULL, 0, 0)) { //Load any other relevant modules (ie dlls) here + atexit(DbgHelpLoader::unload); return TRUE; } } DbgHelpLoader::unload(); - return(FALSE); + return FALSE; } -//***************************************************************************** -//***************************************************************************** -void UninitSymbolInfo(void) -{ - DbgHelpLoader::unload(); -} - - - //***************************************************************************** //***************************************************************************** void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*))