From c21695c951d4b83aea6fdbc037d78931c702c20b Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 5 Jul 2023 14:25:02 +0100 Subject: [PATCH] Don't put crash dumps in Temp on Windows Well... unless we fail to get the user profile directory. Also put freeze dumps in a more appropriately-named file. Discussed in https://gitlab.com/OpenMW/openmw/-/issues/7455 --- .../crashcatcher/windows_crashcatcher.cpp | 22 ++++++++++++------- .../crashcatcher/windows_crashcatcher.hpp | 4 ++-- .../crashcatcher/windows_crashmonitor.cpp | 10 ++++----- .../crashcatcher/windows_crashmonitor.hpp | 2 +- components/crashcatcher/windows_crashshm.hpp | 3 ++- components/debug/debugging.cpp | 18 +++++++++++++-- 6 files changed, 40 insertions(+), 19 deletions(-) diff --git a/components/crashcatcher/windows_crashcatcher.cpp b/components/crashcatcher/windows_crashcatcher.cpp index d31fdac578..080bbda673 100644 --- a/components/crashcatcher/windows_crashcatcher.cpp +++ b/components/crashcatcher/windows_crashcatcher.cpp @@ -26,7 +26,7 @@ namespace Crash CrashCatcher* CrashCatcher::sInstance = nullptr; - CrashCatcher::CrashCatcher(int argc, char **argv, const std::string& crashLogPath) + CrashCatcher::CrashCatcher(int argc, char** argv, const std::string& crashDumpPath, const std::string& freezeDumpPath) { assert(sInstance == nullptr); // don't allow two instances @@ -48,7 +48,7 @@ namespace Crash if (!shmHandle) { setupIpc(); - startMonitorProcess(crashLogPath); + startMonitorProcess(crashDumpPath, freezeDumpPath); installHandler(); } else @@ -124,7 +124,7 @@ namespace Crash SetUnhandledExceptionFilter(vectoredExceptionHandler); } - void CrashCatcher::startMonitorProcess(const std::string& crashLogPath) + void CrashCatcher::startMonitorProcess(const std::string& crashDumpPath, const std::string& freezeDumpPath) { std::wstring executablePath; DWORD copied = 0; @@ -134,11 +134,17 @@ namespace Crash } while (copied >= executablePath.size()); executablePath.resize(copied); - memset(mShm->mStartup.mLogFilePath, 0, sizeof(mShm->mStartup.mLogFilePath)); - size_t length = crashLogPath.length(); + memset(mShm->mStartup.mCrashDumpFilePath, 0, sizeof(mShm->mStartup.mCrashDumpFilePath)); + size_t length = crashDumpPath.length(); if (length >= MAX_LONG_PATH) length = MAX_LONG_PATH - 1; - strncpy(mShm->mStartup.mLogFilePath, crashLogPath.c_str(), length); - mShm->mStartup.mLogFilePath[length] = '\0'; + strncpy(mShm->mStartup.mCrashDumpFilePath, crashDumpPath.c_str(), length); + mShm->mStartup.mCrashDumpFilePath[length] = '\0'; + + memset(mShm->mStartup.mFreezeDumpFilePath, 0, sizeof(mShm->mStartup.mFreezeDumpFilePath)); + length = freezeDumpPath.length(); + if (length >= MAX_LONG_PATH) length = MAX_LONG_PATH - 1; + strncpy(mShm->mStartup.mFreezeDumpFilePath, freezeDumpPath.c_str(), length); + mShm->mStartup.mFreezeDumpFilePath[length] = '\0'; // note that we don't need to lock the SHM here, the other process has not started yet mShm->mEvent = CrashSHM::Event::Startup; @@ -197,7 +203,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) + "'.\nPlease 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.mCrashDumpFilePath) + "'.\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_crashcatcher.hpp b/components/crashcatcher/windows_crashcatcher.hpp index 1133afe69c..85b30f762f 100644 --- a/components/crashcatcher/windows_crashcatcher.hpp +++ b/components/crashcatcher/windows_crashcatcher.hpp @@ -28,7 +28,7 @@ namespace Crash { public: - CrashCatcher(int argc, char **argv, const std::string& crashLogPath); + CrashCatcher(int argc, char** argv, const std::string& crashDumpPath, const std::string& freezeDumpPath); ~CrashCatcher(); private: @@ -56,7 +56,7 @@ namespace Crash void shmUnlock(); - void startMonitorProcess(const std::string& crashLogPath); + void startMonitorProcess(const std::string& crashDumpPath, const std::string& freezeDumpPath); void waitMonitor(); diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index fb6db269c8..b46efdeda8 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -186,7 +186,7 @@ namespace Crash case CrashSHM::Event::None: break; case CrashSHM::Event::Crashed: - handleCrash(); + handleCrash(false); running = false; break; case CrashSHM::Event::Shutdown: @@ -205,9 +205,9 @@ namespace Crash if (mFreezeAbort) { - handleCrash(); + handleCrash(true); 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 !"; + std::string message = "OpenMW appears to have frozen.\nCrash log saved to '" + std::string(mShm->mStartup.mFreezeDumpFilePath) + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr); } @@ -231,7 +231,7 @@ namespace Crash return utf16; } - void CrashMonitor::handleCrash() + void CrashMonitor::handleCrash(bool isFreeze) { DWORD processId = GetProcessId(mAppProcessHandle); @@ -250,7 +250,7 @@ namespace Crash if (miniDumpWriteDump == NULL) return; - std::wstring utf16Path = utf8ToUtf16(mShm->mStartup.mLogFilePath); + std::wstring utf16Path = utf8ToUtf16(isFreeze ? mShm->mStartup.mFreezeDumpFilePath : mShm->mStartup.mCrashDumpFilePath); if (utf16Path.empty()) return; diff --git a/components/crashcatcher/windows_crashmonitor.hpp b/components/crashcatcher/windows_crashmonitor.hpp index d6a8dd7ac5..834c5d1643 100644 --- a/components/crashcatcher/windows_crashmonitor.hpp +++ b/components/crashcatcher/windows_crashmonitor.hpp @@ -53,7 +53,7 @@ private: void shmUnlock(); - void handleCrash(); + void handleCrash(bool isFreeze); void showFreezeMessageBox(); diff --git a/components/crashcatcher/windows_crashshm.hpp b/components/crashcatcher/windows_crashshm.hpp index eba478e744..1b3150fe84 100644 --- a/components/crashcatcher/windows_crashshm.hpp +++ b/components/crashcatcher/windows_crashshm.hpp @@ -28,7 +28,8 @@ namespace Crash HANDLE mSignalApp; HANDLE mSignalMonitor; HANDLE mShmMutex; - char mLogFilePath[MAX_LONG_PATH]; + char mCrashDumpFilePath[MAX_LONG_PATH]; + char mFreezeDumpFilePath[MAX_LONG_PATH]; } mStartup; struct Crashed diff --git a/components/debug/debugging.cpp b/components/debug/debugging.cpp index 933b3d2335..24383bd673 100644 --- a/components/debug/debugging.cpp +++ b/components/debug/debugging.cpp @@ -9,6 +9,12 @@ #ifdef _WIN32 #include #include +#include +#define FAR +#define NEAR +#include +#undef NEAR +#undef FAR #endif #include @@ -304,8 +310,16 @@ int wrapApplication(int (*innerApplication)(int argc, char *argv[]), int argc, c if (const auto env = std::getenv("OPENMW_DISABLE_CRASH_CATCHER"); env == nullptr || std::atol(env) == 0) { #if defined(_WIN32) - const std::string crashLogName = Misc::StringUtils::lowerCase(appName) + "-crash.dmp"; - Crash::CrashCatcher crashy(argc, argv, (boost::filesystem::temp_directory_path() / crashLogName).make_preferred().string()); + const std::string crashDumpName = Misc::StringUtils::lowerCase(appName) + "-crash.dmp"; + const std::string freezeDumpName = Misc::StringUtils::lowerCase(appName) + "-freeze.dmp"; + boost::filesystem::path dumpDirectory = boost::filesystem::temp_directory_path(); + PWSTR userProfile = nullptr; + if (SUCCEEDED(SHGetKnownFolderPath(FOLDERID_Profile, 0, nullptr, &userProfile))) + { + dumpDirectory = userProfile; + } + CoTaskMemFree(userProfile); + Crash::CrashCatcher crashy(argc, argv, (dumpDirectory / crashDumpName).make_preferred().string(), (dumpDirectory / freezeDumpName).make_preferred().string()); #else const std::string crashLogName = Misc::StringUtils::lowerCase(appName) + "-crash.log"; // install the crash handler as soon as possible. note that the log path