Merge branch 'no-crash-dump-in-temp' into 'master'

Don't put crash dumps in Temp on Windows

Closes #7455

See merge request OpenMW/openmw!3200

components/crashcatcher/windows_crashcatcher.cpp

@@ -27,7 +27,8 @@ namespace Crash
 
     CrashCatcher* CrashCatcher::sInstance = nullptr;
 
-    CrashCatcher::CrashCatcher(int argc, char** argv, const std::filesystem::path& crashLogPath)
+    CrashCatcher::CrashCatcher(
+        int argc, char** argv, const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath)
     {
         assert(sInstance == nullptr); // don't allow two instances
 
@@ -49,7 +50,7 @@ namespace Crash
         if (!shmHandle)
         {
             setupIpc();
-            startMonitorProcess(crashLogPath);
+            startMonitorProcess(crashDumpPath, freezeDumpPath);
             installHandler();
         }
         else
@@ -76,6 +77,32 @@ namespace Crash
             CloseHandle(mShmHandle);
     }
 
+    void CrashCatcher::updateDumpPaths(
+        const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath)
+    {
+        shmLock();
+
+        memset(mShm->mStartup.mCrashDumpFilePath, 0, sizeof(mShm->mStartup.mCrashDumpFilePath));
+        const auto str = crashDumpPath.u8string();
+        size_t length = str.length();
+        if (length >= MAX_LONG_PATH)
+            length = MAX_LONG_PATH - 1;
+        strncpy_s(mShm->mStartup.mCrashDumpFilePath, sizeof mShm->mStartup.mCrashDumpFilePath,
+            Misc::StringUtils::u8StringToString(str).c_str(), length);
+        mShm->mStartup.mCrashDumpFilePath[length] = '\0';
+
+        memset(mShm->mStartup.mFreezeDumpFilePath, 0, sizeof(mShm->mStartup.mFreezeDumpFilePath));
+        const auto strFreeze = freezeDumpPath.u8string();
+        length = strFreeze.length();
+        if (length >= MAX_LONG_PATH)
+            length = MAX_LONG_PATH - 1;
+        strncpy_s(mShm->mStartup.mFreezeDumpFilePath, sizeof mShm->mStartup.mFreezeDumpFilePath,
+            Misc::StringUtils::u8StringToString(strFreeze).c_str(), length);
+        mShm->mStartup.mFreezeDumpFilePath[length] = '\0';
+
+        shmUnlock();
+    }
+
     void CrashCatcher::setupIpc()
     {
         SECURITY_ATTRIBUTES attributes;
@@ -126,7 +153,8 @@ namespace Crash
         SetUnhandledExceptionFilter(vectoredExceptionHandler);
     }
 
-    void CrashCatcher::startMonitorProcess(const std::filesystem::path& crashLogPath)
+    void CrashCatcher::startMonitorProcess(
+        const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath)
     {
         std::wstring executablePath;
         DWORD copied = 0;
@@ -137,14 +165,23 @@ namespace Crash
         } while (copied >= executablePath.size());
         executablePath.resize(copied);
 
-        memset(mShm->mStartup.mLogFilePath, 0, sizeof(mShm->mStartup.mLogFilePath));
-        const auto str = crashLogPath.u8string();
+        memset(mShm->mStartup.mCrashDumpFilePath, 0, sizeof(mShm->mStartup.mCrashDumpFilePath));
+        const auto str = crashDumpPath.u8string();
         size_t length = str.length();
         if (length >= MAX_LONG_PATH)
             length = MAX_LONG_PATH - 1;
-        strncpy_s(mShm->mStartup.mLogFilePath, sizeof mShm->mStartup.mLogFilePath,
+        strncpy_s(mShm->mStartup.mCrashDumpFilePath, sizeof mShm->mStartup.mCrashDumpFilePath,
             Misc::StringUtils::u8StringToString(str).c_str(), length);
-        mShm->mStartup.mLogFilePath[length] = '\0';
+        mShm->mStartup.mCrashDumpFilePath[length] = '\0';
+
+        memset(mShm->mStartup.mFreezeDumpFilePath, 0, sizeof(mShm->mStartup.mFreezeDumpFilePath));
+        const auto strFreeze = freezeDumpPath.u8string();
+        length = strFreeze.length();
+        if (length >= MAX_LONG_PATH)
+            length = MAX_LONG_PATH - 1;
+        strncpy_s(mShm->mStartup.mFreezeDumpFilePath, sizeof mShm->mStartup.mFreezeDumpFilePath,
+            Misc::StringUtils::u8StringToString(strFreeze).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;
@@ -204,7 +241,7 @@ namespace Crash
         waitMonitor();
 
         std::string message = "OpenMW has encountered a fatal error.\nCrash log saved to '"
-            + std::string(mShm->mStartup.mLogFilePath)
+            + std::string(mShm->mStartup.mCrashDumpFilePath)
             + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !";
         SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr);
     }

components/crashcatcher/windows_crashcatcher.hpp

@@ -27,9 +27,14 @@ namespace Crash
     class CrashCatcher final
     {
     public:
-        CrashCatcher(int argc, char** argv, const std::filesystem::path& crashLogPath);
+        static CrashCatcher* instance() { return sInstance; }
+
+        CrashCatcher(int argc, char** argv, const std::filesystem::path& crashDumpPath,
+            const std::filesystem::path& freezeDumpPath);
         ~CrashCatcher();
 
+        void updateDumpPaths(const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath);
+
     private:
         static CrashCatcher* sInstance;
 
@@ -54,7 +59,8 @@ namespace Crash
 
         void shmUnlock();
 
-        void startMonitorProcess(const std::filesystem::path& crashLogPath);
+        void startMonitorProcess(
+            const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath);
 
         void waitMonitor();
 

components/crashcatcher/windows_crashmonitor.cpp

@@ -192,7 +192,7 @@ namespace Crash
                         case CrashSHM::Event::None:
                             break;
                         case CrashSHM::Event::Crashed:
-                            handleCrash();
+                            handleCrash(false);
                             running = false;
                             break;
                         case CrashSHM::Event::Shutdown:
@@ -211,10 +211,10 @@ 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)
+                    + std::string(mShm->mStartup.mFreezeDumpFilePath)
                     + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !";
                 SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr);
             }
@@ -238,7 +238,7 @@ namespace Crash
         return utf16;
     }
 
-    void CrashMonitor::handleCrash()
+    void CrashMonitor::handleCrash(bool isFreeze)
     {
         DWORD processId = GetProcessId(mAppProcessHandle);
 
@@ -256,7 +256,8 @@ 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;
 

components/crashcatcher/windows_crashmonitor.hpp

@@ -51,7 +51,7 @@ namespace Crash
 
         void shmUnlock();
 
-        void handleCrash();
+        void handleCrash(bool isFreeze);
 
         void showFreezeMessageBox();
 

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

components/debug/debugging.cpp

@@ -17,6 +17,19 @@
 #include <components/crashcatcher/windows_crashcatcher.hpp>
 #include <components/files/conversion.hpp>
 #include <components/windows.hpp>
+
+#include <Knownfolders.h>
+
+#pragma push_macro("FAR")
+#pragma push_macro("NEAR")
+#undef FAR
+#define FAR
+#undef NEAR
+#define NEAR
+#include <Shlobj.h>
+#pragma pop_macro("NEAR")
+#pragma pop_macro("FAR")
+
 #endif
 
 #include <SDL_messagebox.h>
@@ -301,6 +314,16 @@ void setupLogging(const std::filesystem::path& logDir, std::string_view appName,
     std::cout.rdbuf(&coutsb);
     std::cerr.rdbuf(&cerrsb);
 #endif
+
+#ifdef _WIN32
+    if (Crash::CrashCatcher::instance())
+    {
+        const std::string crashDumpName = Misc::StringUtils::lowerCase(appName) + "-crash.dmp";
+        const std::string freezeDumpName = Misc::StringUtils::lowerCase(appName) + "-freeze.dmp";
+        std::filesystem::path dumpDirectory = logDir;
+        Crash::CrashCatcher::instance()->updateDumpPaths(dumpDirectory / crashDumpName, dumpDirectory / freezeDumpName);
+    }
+#endif
 }
 
 int wrapApplication(int (*innerApplication)(int argc, char* argv[]), int argc, char* argv[], std::string_view appName)
@@ -319,9 +342,16 @@ int wrapApplication(int (*innerApplication)(int argc, char* argv[]), int argc, c
             env == nullptr || Misc::StringUtils::toNumeric<int>(env, 0) == 0)
         {
 #if defined(_WIN32)
-            const std::string crashLogName = Misc::StringUtils::lowerCase(appName) + "-crash.dmp";
-            Crash::CrashCatcher crashy(
-                argc, argv, Files::pathToUnicodeString(std::filesystem::temp_directory_path() / crashLogName));
+            const std::string crashDumpName = Misc::StringUtils::lowerCase(appName) + "-crash.dmp";
+            const std::string freezeDumpName = Misc::StringUtils::lowerCase(appName) + "-freeze.dmp";
+            std::filesystem::path dumpDirectory = std::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, dumpDirectory / freezeDumpName);
 #else
             const std::string crashLogName = Misc::StringUtils::lowerCase(appName) + "-crash.log";
             // install the crash handler as soon as possible.