From c746a8abb70a21b73eb9bf0fec934f43337a41ca Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sat, 18 Dec 2021 21:34:15 +0000 Subject: [PATCH 1/7] Attempt to catch freezes on Windows --- .../crashcatcher/windows_crashcatcher.cpp | 3 +- .../crashcatcher/windows_crashmonitor.cpp | 56 +++++++++++++++++++ .../crashcatcher/windows_crashmonitor.hpp | 4 ++ components/crashcatcher/windows_crashshm.hpp | 1 + 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/components/crashcatcher/windows_crashcatcher.cpp b/components/crashcatcher/windows_crashcatcher.cpp index 39ac86d7b8..eea6ac40ad 100644 --- a/components/crashcatcher/windows_crashcatcher.cpp +++ b/components/crashcatcher/windows_crashcatcher.cpp @@ -144,6 +144,7 @@ namespace Crash mShm->mEvent = CrashSHM::Event::Startup; mShm->mStartup.mShmMutex = duplicateHandle(mShmMutex); mShm->mStartup.mAppProcessHandle = duplicateHandle(GetCurrentProcess()); + mShm->mStartup.mAppMainThreadId = GetThreadId(GetCurrentThread()); mShm->mStartup.mSignalApp = duplicateHandle(mSignalAppEvent); mShm->mStartup.mSignalMonitor = duplicateHandle(mSignalMonitorEvent); @@ -196,7 +197,7 @@ namespace Crash // must remain until monitor has finished waitMonitor(); - std::string message = "OpenMW has encountered a fatal error.\nCrash log saved to '" + std::string(mShm->mStartup.mLogFilePath) + "'.\n Please report this to https://gitlab.com/OpenMW/openmw/issues !"; + std::string message = "OpenMW has encountered a fatal error.\nCrash log saved to '" + std::string(mShm->mStartup.mLogFilePath) + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr); } diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index 8976deb2ea..26ebc0bad3 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -9,6 +9,8 @@ #include #include +#include + #include "windows_crashcatcher.hpp" #include "windows_crashmonitor.hpp" #include "windows_crashshm.hpp" @@ -28,6 +30,7 @@ namespace Crash mShmMutex = mShm->mStartup.mShmMutex; mAppProcessHandle = mShm->mStartup.mAppProcessHandle; + mAppMainThreadId = mShm->mStartup.mAppMainThreadId; mSignalAppEvent = mShm->mStartup.mSignalApp; mSignalMonitorEvent = mShm->mStartup.mSignalMonitor; } @@ -80,6 +83,44 @@ namespace Crash return code == STILL_ACTIVE; } + bool CrashMonitor::isAppFrozen() + { + if (!mAppWindowHandle) + { + EnumWindows([](HWND handle, LPARAM param) -> BOOL { + CrashMonitor& crashMonitor = *(CrashMonitor*)param; + DWORD processId; + if (GetWindowThreadProcessId(handle, &processId) == crashMonitor.mAppMainThreadId && processId == GetProcessId(crashMonitor.mAppProcessHandle)) + { + if (GetWindow(handle, GW_OWNER) == 0) + { + crashMonitor.mAppWindowHandle = handle; + return false; + } + } + return true; + }, (LPARAM)this); + if (mAppWindowHandle) + { + // TODO: use https://devblogs.microsoft.com/oldnewthing/20111026-00/?p=9263 to monitor for the window being destroyed + } + else + return false; + } + if (IsHungAppWindow) + return IsHungAppWindow(mAppWindowHandle); + else + { + BOOL debuggerPresent; + + if (CheckRemoteDebuggerPresent(mAppProcessHandle, &debuggerPresent) && debuggerPresent) + return false; + if (SendMessageTimeoutA(mAppWindowHandle, WM_NULL, 0, 0, 0, 5000, nullptr) == 0) + return GetLastError() == ERROR_TIMEOUT; + } + return false; + } + void CrashMonitor::run() { try @@ -88,8 +129,16 @@ namespace Crash signalApp(); bool running = true; + bool frozen = false; while (isAppAlive() && running) { + if (isAppFrozen()) + { + frozen = true; + handleCrash(); + running = false; + break; + } if (waitApp()) { shmLock(); @@ -113,6 +162,13 @@ namespace Crash } } + if (frozen) + { + TerminateProcess(mAppProcessHandle, -1); + std::string message = "OpenMW appears to have frozen.\nCrash log saved to '" + std::string(mShm->mStartup.mLogFilePath) + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; + SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr); + } + } catch (...) { diff --git a/components/crashcatcher/windows_crashmonitor.hpp b/components/crashcatcher/windows_crashmonitor.hpp index 678d38435c..01acd34bdd 100644 --- a/components/crashcatcher/windows_crashmonitor.hpp +++ b/components/crashcatcher/windows_crashmonitor.hpp @@ -21,6 +21,8 @@ public: private: HANDLE mAppProcessHandle = nullptr; + DWORD mAppMainThreadId = 0; + HWND mAppWindowHandle = nullptr; // triggered when the monitor process wants to wake the parent process (received via SHM) HANDLE mSignalAppEvent = nullptr; @@ -37,6 +39,8 @@ private: bool isAppAlive() const; + bool isAppFrozen(); + void shmLock(); void shmUnlock(); diff --git a/components/crashcatcher/windows_crashshm.hpp b/components/crashcatcher/windows_crashshm.hpp index 47929a45fe..a474600f94 100644 --- a/components/crashcatcher/windows_crashshm.hpp +++ b/components/crashcatcher/windows_crashshm.hpp @@ -26,6 +26,7 @@ namespace Crash struct Startup { HANDLE mAppProcessHandle; + DWORD mAppMainThreadId; HANDLE mSignalApp; HANDLE mSignalMonitor; HANDLE mShmMutex; From fe1523d16d203ec1fe21a077c4adde0bacef196e Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 20 Dec 2021 22:13:11 +0000 Subject: [PATCH 2/7] Fix signed/unsigned mismatch --- components/crashcatcher/windows_crashmonitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index 26ebc0bad3..5774f5ee30 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -164,7 +164,7 @@ namespace Crash if (frozen) { - TerminateProcess(mAppProcessHandle, -1); + TerminateProcess(mAppProcessHandle, 0xDEAD); std::string message = "OpenMW appears to have frozen.\nCrash log saved to '" + std::string(mShm->mStartup.mLogFilePath) + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr); } From 97396da74c51def4ca84f86107d418cec43deb69 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 20 Dec 2021 22:23:44 +0000 Subject: [PATCH 3/7] Get rid of break It might look confusing with the breaks in the switch below --- components/crashcatcher/windows_crashmonitor.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index 5774f5ee30..2c942e00b7 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -137,9 +137,8 @@ namespace Crash frozen = true; handleCrash(); running = false; - break; } - if (waitApp()) + if (!frozen && waitApp()) { shmLock(); From d15c2922a96f7c414229508767765acfb6c42cf6 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 20 Dec 2021 22:24:47 +0000 Subject: [PATCH 4/7] Stop monitoring closed windows If it gets repalced, the new one will be watched instead --- .../crashcatcher/windows_crashmonitor.cpp | 20 ++++++++++++++++++- .../crashcatcher/windows_crashmonitor.hpp | 2 ++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index 2c942e00b7..4627380356 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -18,6 +18,7 @@ namespace Crash { + std::unordered_map CrashMonitor::smEventHookOwners{}; CrashMonitor::CrashMonitor(HANDLE shmHandle) : mShmHandle(shmHandle) @@ -85,6 +86,10 @@ namespace Crash bool CrashMonitor::isAppFrozen() { + MSG message; + // Allow the event hook callback to run + PeekMessage(&message, nullptr, 0, 0, PM_NOREMOVE); + if (!mAppWindowHandle) { EnumWindows([](HWND handle, LPARAM param) -> BOOL { @@ -102,7 +107,20 @@ namespace Crash }, (LPARAM)this); if (mAppWindowHandle) { - // TODO: use https://devblogs.microsoft.com/oldnewthing/20111026-00/?p=9263 to monitor for the window being destroyed + DWORD processId; + GetWindowThreadProcessId(mAppWindowHandle, &processId); + HWINEVENTHOOK eventHookHandle = SetWinEventHook(EVENT_OBJECT_DESTROY, EVENT_OBJECT_DESTROY, nullptr, + [](HWINEVENTHOOK hWinEventHook, DWORD event, HWND windowHandle, LONG objectId, LONG childId, DWORD eventThread, DWORD eventTime) + { + CrashMonitor& crashMonitor = *smEventHookOwners[hWinEventHook]; + if (event == EVENT_OBJECT_DESTROY && windowHandle == crashMonitor.mAppWindowHandle && objectId == OBJID_WINDOW && childId == INDEXID_CONTAINER) + { + crashMonitor.mAppWindowHandle = nullptr; + smEventHookOwners.erase(hWinEventHook); + UnhookWinEvent(hWinEventHook); + } + }, processId, mAppMainThreadId, WINEVENT_OUTOFCONTEXT); + smEventHookOwners[eventHookHandle] = this; } else return false; diff --git a/components/crashcatcher/windows_crashmonitor.hpp b/components/crashcatcher/windows_crashmonitor.hpp index 01acd34bdd..031e5b0612 100644 --- a/components/crashcatcher/windows_crashmonitor.hpp +++ b/components/crashcatcher/windows_crashmonitor.hpp @@ -33,6 +33,8 @@ private: HANDLE mShmHandle = nullptr; HANDLE mShmMutex = nullptr; + static std::unordered_map smEventHookOwners; + void signalApp() const; bool waitApp() const; From f05cd901cf02694f9561fc01af2bd73fadbaedad Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Tue, 21 Dec 2021 23:19:13 +0000 Subject: [PATCH 5/7] Show messagebox while OpenMW appears to be frozen If it thaws, the messagebox disappears again. The user can press the Abort button to kill OpenMW and generate a crash dump. --- .../crashcatcher/windows_crashmonitor.cpp | 60 +++++++++++++++++-- .../crashcatcher/windows_crashmonitor.hpp | 7 +++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index 4627380356..336210391a 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -148,15 +148,23 @@ namespace Crash bool running = true; bool frozen = false; - while (isAppAlive() && running) + while (isAppAlive() && running && !mFreezeAbort) { if (isAppFrozen()) { - frozen = true; - handleCrash(); - running = false; + if (!frozen) + { + showFreezeMessageBox(); + frozen = true; + } } - if (!frozen && waitApp()) + else if (frozen) + { + hideFreezeMessageBox(); + frozen = false; + } + + if (!mFreezeAbort && waitApp()) { shmLock(); @@ -180,6 +188,9 @@ namespace Crash } if (frozen) + hideFreezeMessageBox(); + + if (mFreezeAbort) { TerminateProcess(mAppProcessHandle, 0xDEAD); std::string message = "OpenMW appears to have frozen.\nCrash log saved to '" + std::string(mShm->mStartup.mLogFilePath) + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; @@ -258,4 +269,43 @@ namespace Crash } } + void CrashMonitor::showFreezeMessageBox() + { + std::thread messageBoxThread([&]() { + SDL_MessageBoxButtonData button = { SDL_MESSAGEBOX_BUTTON_RETURNKEY_DEFAULT, 0, "Abort" }; + SDL_MessageBoxData messageBoxData = { + SDL_MESSAGEBOX_ERROR, + nullptr, + "OpenMW appears to have frozen", + "OpenMW appears to have frozen. Press Abort to terminate it and generate a crash dump.\nIf OpenMW hasn't actually frozen, this message box will disappear a within a few seconds of it becoming responsive.", + 1, + &button, + nullptr + }; + + int buttonId; + if (SDL_ShowMessageBox(&messageBoxData, &buttonId) == 0 && buttonId == 0) + mFreezeAbort = true; + }); + + mFreezeMessageBoxThreadId = GetThreadId(messageBoxThread.native_handle()); + messageBoxThread.detach(); + } + + void CrashMonitor::hideFreezeMessageBox() + { + if (!mFreezeMessageBoxThreadId) + return; + + EnumWindows([](HWND handle, LPARAM param) -> BOOL { + CrashMonitor& crashMonitor = *(CrashMonitor*)param; + DWORD processId; + if (GetWindowThreadProcessId(handle, &processId) == crashMonitor.mFreezeMessageBoxThreadId && processId == GetCurrentProcessId()) + PostMessage(handle, WM_CLOSE, 0, 0); + return true; + }, (LPARAM)this); + + mFreezeMessageBoxThreadId = 0; + } + } // namespace Crash diff --git a/components/crashcatcher/windows_crashmonitor.hpp b/components/crashcatcher/windows_crashmonitor.hpp index 031e5b0612..b520e309b0 100644 --- a/components/crashcatcher/windows_crashmonitor.hpp +++ b/components/crashcatcher/windows_crashmonitor.hpp @@ -33,6 +33,9 @@ private: HANDLE mShmHandle = nullptr; HANDLE mShmMutex = nullptr; + DWORD mFreezeMessageBoxThreadId = 0; + std::atomic_bool mFreezeAbort; + static std::unordered_map smEventHookOwners; void signalApp() const; @@ -48,6 +51,10 @@ private: void shmUnlock(); void handleCrash(); + + void showFreezeMessageBox(); + + void hideFreezeMessageBox(); }; } // namespace Crash From 0e29a760d8080f2f63a582a283e3d7d3b2e201c1 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 26 Dec 2021 02:09:14 +0000 Subject: [PATCH 6/7] Tidy up includes --- components/crashcatcher/windows_crashcatcher.cpp | 3 ++- components/crashcatcher/windows_crashmonitor.cpp | 3 ++- components/crashcatcher/windows_crashmonitor.hpp | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/components/crashcatcher/windows_crashcatcher.cpp b/components/crashcatcher/windows_crashcatcher.cpp index eea6ac40ad..e9eb60e845 100644 --- a/components/crashcatcher/windows_crashcatcher.cpp +++ b/components/crashcatcher/windows_crashcatcher.cpp @@ -1,10 +1,11 @@ +#include "windows_crashcatcher.hpp" + #include #include #include #include #include -#include "windows_crashcatcher.hpp" #include "windows_crashmonitor.hpp" #include "windows_crashshm.hpp" #include diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index 336210391a..50d3fc08a1 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -1,3 +1,5 @@ +#include "windows_crashmonitor.hpp" + #undef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN #include @@ -12,7 +14,6 @@ #include #include "windows_crashcatcher.hpp" -#include "windows_crashmonitor.hpp" #include "windows_crashshm.hpp" #include diff --git a/components/crashcatcher/windows_crashmonitor.hpp b/components/crashcatcher/windows_crashmonitor.hpp index b520e309b0..871346d292 100644 --- a/components/crashcatcher/windows_crashmonitor.hpp +++ b/components/crashcatcher/windows_crashmonitor.hpp @@ -1,7 +1,9 @@ #ifndef WINDOWS_CRASHMONITOR_HPP #define WINDOWS_CRASHMONITOR_HPP -#include +#undef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#include namespace Crash { From fa05b0b96cbdaae575c5b5f65696965139d63042 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 26 Dec 2021 02:10:37 +0000 Subject: [PATCH 7/7] Include Should fix compilation on CI --- components/crashcatcher/windows_crashmonitor.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/crashcatcher/windows_crashmonitor.hpp b/components/crashcatcher/windows_crashmonitor.hpp index 871346d292..4028362836 100644 --- a/components/crashcatcher/windows_crashmonitor.hpp +++ b/components/crashcatcher/windows_crashmonitor.hpp @@ -5,6 +5,8 @@ #define WIN32_LEAN_AND_MEAN #include +#include + namespace Crash {