From 151657a9f292a2269fba71f4786bcb988d64e47d Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Tue, 17 Apr 2012 15:59:14 -0400 Subject: [PATCH 1/3] Fix zombie plCrashHandler issues Win32 HACK: We wait on both the pfCrashCli handle and the crashed semaphore. This way, we can proceed to exit pfCrashSrv when the client process exits insanely. --- Sources/Plasma/CoreLib/hsThread.h | 14 +++++++++++++- .../FeatureLib/pfCrashHandler/plCrashSrv.cpp | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Sources/Plasma/CoreLib/hsThread.h b/Sources/Plasma/CoreLib/hsThread.h index e36d131b..c27e2118 100644 --- a/Sources/Plasma/CoreLib/hsThread.h +++ b/Sources/Plasma/CoreLib/hsThread.h @@ -115,7 +115,11 @@ class hsMutex { public: hsMutex(); virtual ~hsMutex(); - + +#ifdef HS_BUILD_FOR_WIN32 + HANDLE GetHandle() const { return fMutexH; } +#endif + void Lock(); hsBool TryLock(); void Unlock(); @@ -157,6 +161,10 @@ public: hsSemaphore(int initialValue=0, const char* name=nil); ~hsSemaphore(); +#ifdef HS_BUILD_FOR_WIN32 + HANDLE GetHandle() const { return fSemaH; } +#endif + hsBool TryWait(); hsBool Wait(hsMilliseconds timeToWait = kPosInfinity32); void Signal(); @@ -183,6 +191,10 @@ public: hsEvent(); ~hsEvent(); +#ifdef HS_BUILD_FOR_WIN32 + HANDLE GetHandle() const { return fEvent; } +#endif + hsBool Wait(hsMilliseconds timeToWait = kPosInfinity32); void Signal(); }; diff --git a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp index 0efae16b..4b35a6c3 100644 --- a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp +++ b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp @@ -108,10 +108,21 @@ void plCrashSrv::IHandleCrash() void plCrashSrv::HandleCrash() { - fCrashed->Wait(); // Wait for a crash +#ifdef HS_BUILD_FOR_WIN32 + if (!fLink) + FATAL("plCrashMemLink is nil!"); + + // In Win32 land we have to hackily handle the client process exiting, so we'll wait on both + // the crashed semaphore and the client process... + HANDLE hack[2] = { fLink->fClientProcess, fCrashed->GetHandle() }; + DWORD result = WaitForMultipleObjects(arrsize(hack), hack, FALSE, INFINITE); + hsAssert(result != WAIT_FAILED, "WaitForMultipleObjects failed"); +#else + fCrashed->Wait(); if (!fLink) FATAL("plCrashMemLink is nil!"); - else if (fLink->fCrashed) +#endif + if (fLink->fCrashed) IHandleCrash(); fHandled->Signal(); // Tell CrashCli we handled it } From 389d3c1802f541566a1997c0a4e118a6aab06705 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Tue, 17 Apr 2012 16:03:41 -0400 Subject: [PATCH 2/3] Fix deadlock if plCrashHandler is not running Only wait for the crash server to do its job if it's attached. Silent failures are evil, but it's better than hanging forever. --- Sources/Plasma/FeatureLib/pfCrashHandler/plCrashCli.cpp | 4 +++- Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp | 5 ++--- Sources/Plasma/FeatureLib/pfCrashHandler/plCrash_Private.h | 1 + 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashCli.cpp b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashCli.cpp index d5412d3b..881c761c 100644 --- a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashCli.cpp +++ b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashCli.cpp @@ -134,5 +134,7 @@ void plCrashCli::ReportCrash(PEXCEPTION_POINTERS e) void plCrashCli::WaitForHandle() { - fHandled->Wait(); + // Don't deadlock... Only wait if the CrashSrv is attached + if (fLink && fLink->fSrvReady) + fHandled->Wait(); } diff --git a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp index 4b35a6c3..5d5926e2 100644 --- a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp +++ b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrashSrv.cpp @@ -108,10 +108,11 @@ void plCrashSrv::IHandleCrash() void plCrashSrv::HandleCrash() { -#ifdef HS_BUILD_FOR_WIN32 if (!fLink) FATAL("plCrashMemLink is nil!"); + fLink->fSrvReady = true; // mark us as ready to receive crashes +#ifdef HS_BUILD_FOR_WIN32 // In Win32 land we have to hackily handle the client process exiting, so we'll wait on both // the crashed semaphore and the client process... HANDLE hack[2] = { fLink->fClientProcess, fCrashed->GetHandle() }; @@ -119,8 +120,6 @@ void plCrashSrv::HandleCrash() hsAssert(result != WAIT_FAILED, "WaitForMultipleObjects failed"); #else fCrashed->Wait(); - if (!fLink) - FATAL("plCrashMemLink is nil!"); #endif if (fLink->fCrashed) IHandleCrash(); diff --git a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrash_Private.h b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrash_Private.h index e96f8dc9..70e9ec33 100644 --- a/Sources/Plasma/FeatureLib/pfCrashHandler/plCrash_Private.h +++ b/Sources/Plasma/FeatureLib/pfCrashHandler/plCrash_Private.h @@ -59,6 +59,7 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com struct plCrashMemLink { bool fCrashed; + bool fSrvReady; #ifdef HS_BUILD_FOR_WIN32 HANDLE fClientProcess; uint32_t fClientProcessID; From 55a816f376af4adc8e15fbaade82c630a331b39e Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Tue, 17 Apr 2012 16:05:00 -0400 Subject: [PATCH 3/3] Remove stale stack dump reports --- Sources/Plasma/Apps/plClient/winmain.cpp | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Sources/Plasma/Apps/plClient/winmain.cpp b/Sources/Plasma/Apps/plClient/winmain.cpp index b8af4143..68aaddc5 100644 --- a/Sources/Plasma/Apps/plClient/winmain.cpp +++ b/Sources/Plasma/Apps/plClient/winmain.cpp @@ -1629,24 +1629,6 @@ int WINAPI WinMain(HINSTANCE hInst, HINSTANCE hPrevInst, LPSTR lpCmdLine, int nC } } - // log stackdump.log text if the log exists - char stackDumpText[1024]; - wchar_t stackDumpTextW[1024]; - memset(stackDumpText, 0, arrsize(stackDumpText)); - memset(stackDumpTextW, 0, arrsize(stackDumpTextW) * sizeof(wchar_t)); - wchar_t fileAndPath[MAX_PATH]; - PathGetLogDirectory(fileAndPath, arrsize(fileAndPath)); - PathAddFilename(fileAndPath, fileAndPath, L"stackDump.log", arrsize(fileAndPath)); - FILE *stackDumpLog = _wfopen(fileAndPath, L"r"); - if(stackDumpLog) - { - fread(stackDumpText, 1, arrsize(stackDumpText) - 1, stackDumpLog); - StrToUnicode(stackDumpTextW, stackDumpText, arrsize(stackDumpText)); - NetCliAuthLogStackDump (stackDumpTextW); - fclose(stackDumpLog); - plFileUtils::RemoveFile(fileAndPath); - } - for (;;) { // Create Window if (!WinInit(hInst, nCmdShow) || gClient->GetDone())