diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index 7d96794a98..f64f41b198 100644 --- a/components/CMakeLists.txt +++ b/components/CMakeLists.txt @@ -333,6 +333,7 @@ if(WIN32) windows_crashcatcher windows_crashmonitor windows_crashshm + windowscrashdumppathhelpers ) elseif(NOT ANDROID) add_component_dir (crashcatcher diff --git a/components/crashcatcher/windows_crashcatcher.cpp b/components/crashcatcher/windows_crashcatcher.cpp index 6236db19c3..a791290328 100644 --- a/components/crashcatcher/windows_crashcatcher.cpp +++ b/components/crashcatcher/windows_crashcatcher.cpp @@ -7,12 +7,28 @@ #include "windows_crashmonitor.hpp" #include "windows_crashshm.hpp" +#include "windowscrashdumppathhelpers.hpp" #include #include namespace Crash { + namespace + { + template + void writePathToShm(T(&buffer)[N], const std::filesystem::path& path) + { + memset(buffer, 0, sizeof(buffer)); + const auto str = path.u8string(); + size_t length = str.length(); + if (length >= sizeof(buffer)) + length = sizeof(buffer) - 1; + strncpy_s(buffer, sizeof(buffer), + Misc::StringUtils::u8StringToString(str).c_str(), length); + buffer[length] = '\0'; + } + } HANDLE duplicateHandle(HANDLE handle) { @@ -27,8 +43,8 @@ namespace Crash CrashCatcher* CrashCatcher::sInstance = nullptr; - CrashCatcher::CrashCatcher( - int argc, char** argv, const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath) + CrashCatcher::CrashCatcher(int argc, char** argv, const std::filesystem::path& dumpPath, + const std::filesystem::path& crashDumpName, const std::filesystem::path& freezeDumpName) { assert(sInstance == nullptr); // don't allow two instances @@ -50,7 +66,7 @@ namespace Crash if (!shmHandle) { setupIpc(); - startMonitorProcess(crashDumpPath, freezeDumpPath); + startMonitorProcess(dumpPath, crashDumpName, freezeDumpName); installHandler(); } else @@ -77,28 +93,22 @@ namespace Crash CloseHandle(mShmHandle); } - void CrashCatcher::updateDumpPaths( - const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath) + void CrashCatcher::updateDumpPath(const std::filesystem::path& dumpPath) + { + shmLock(); + + writePathToShm(mShm->mStartup.mDumpDirectoryPath, dumpPath); + + shmUnlock(); + } + + void CrashCatcher::updateDumpNames( + const std::filesystem::path& crashDumpName, const std::filesystem::path& freezeDumpName) { 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'; + writePathToShm(mShm->mStartup.mCrashDumpFileName, crashDumpName); + writePathToShm(mShm->mStartup.mFreezeDumpFileName, freezeDumpName); shmUnlock(); } @@ -153,8 +163,8 @@ namespace Crash SetUnhandledExceptionFilter(vectoredExceptionHandler); } - void CrashCatcher::startMonitorProcess( - const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath) + void CrashCatcher::startMonitorProcess(const std::filesystem::path& dumpPath, + const std::filesystem::path& crashDumpName, const std::filesystem::path& freezeDumpName) { std::wstring executablePath; DWORD copied = 0; @@ -165,23 +175,9 @@ namespace Crash } while (copied >= executablePath.size()); executablePath.resize(copied); - 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'; + writePathToShm(mShm->mStartup.mDumpDirectoryPath, dumpPath); + writePathToShm(mShm->mStartup.mCrashDumpFileName, crashDumpName); + writePathToShm(mShm->mStartup.mFreezeDumpFileName, freezeDumpName); // note that we don't need to lock the SHM here, the other process has not started yet mShm->mEvent = CrashSHM::Event::Startup; @@ -204,6 +200,9 @@ namespace Crash if (!CreateProcessW(executablePath.data(), arguments.data(), NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) throw std::runtime_error("Could not start crash monitor process"); + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + waitMonitor(); } @@ -241,7 +240,7 @@ namespace Crash waitMonitor(); std::string message = "OpenMW has encountered a fatal error.\nCrash log saved to '" - + std::string(mShm->mStartup.mCrashDumpFilePath) + + Misc::StringUtils::u8StringToString(getCrashDumpPath(*mShm).u8string()) + "'.\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 98c5a3bce6..385567545f 100644 --- a/components/crashcatcher/windows_crashcatcher.hpp +++ b/components/crashcatcher/windows_crashcatcher.hpp @@ -29,11 +29,13 @@ namespace Crash public: static CrashCatcher* instance() { return sInstance; } - CrashCatcher(int argc, char** argv, const std::filesystem::path& crashDumpPath, - const std::filesystem::path& freezeDumpPath); + CrashCatcher(int argc, char** argv, const std::filesystem::path& dumpPath, + const std::filesystem::path& crashDumpName, const std::filesystem::path& freezeDumpName); ~CrashCatcher(); - void updateDumpPaths(const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath); + void updateDumpPath(const std::filesystem::path& dumpPath); + + void updateDumpNames(const std::filesystem::path& crashDumpName, const std::filesystem::path& freezeDumpName); private: static CrashCatcher* sInstance; @@ -59,8 +61,8 @@ namespace Crash void shmUnlock(); - void startMonitorProcess( - const std::filesystem::path& crashDumpPath, const std::filesystem::path& freezeDumpPath); + void startMonitorProcess(const std::filesystem::path& dumpPath, const std::filesystem::path& crashDumpName, + const std::filesystem::path& freezeDumpName); void waitMonitor(); diff --git a/components/crashcatcher/windows_crashmonitor.cpp b/components/crashcatcher/windows_crashmonitor.cpp index ea316c8a5c..0c268e1661 100644 --- a/components/crashcatcher/windows_crashmonitor.cpp +++ b/components/crashcatcher/windows_crashmonitor.cpp @@ -12,7 +12,9 @@ #include "windows_crashcatcher.hpp" #include "windows_crashshm.hpp" +#include "windowscrashdumppathhelpers.hpp" #include +#include namespace Crash { @@ -214,7 +216,7 @@ namespace Crash handleCrash(true); TerminateProcess(mAppProcessHandle, 0xDEAD); std::string message = "OpenMW appears to have frozen.\nCrash log saved to '" - + std::string(mShm->mStartup.mFreezeDumpFilePath) + + Misc::StringUtils::u8StringToString(getFreezeDumpPath(*mShm).u8string()) + "'.\nPlease report this to https://gitlab.com/OpenMW/openmw/issues !"; SDL_ShowSimpleMessageBox(0, "Fatal Error", message.c_str(), nullptr); } @@ -256,8 +258,7 @@ namespace Crash if (miniDumpWriteDump == NULL) return; - std::wstring utf16Path - = utf8ToUtf16(isFreeze ? mShm->mStartup.mFreezeDumpFilePath : mShm->mStartup.mCrashDumpFilePath); + std::wstring utf16Path = (isFreeze ? getFreezeDumpPath(*mShm) : getCrashDumpPath(*mShm)).native(); if (utf16Path.empty()) return; diff --git a/components/crashcatcher/windows_crashshm.hpp b/components/crashcatcher/windows_crashshm.hpp index 1b3150fe84..b3ba38fb2a 100644 --- a/components/crashcatcher/windows_crashshm.hpp +++ b/components/crashcatcher/windows_crashshm.hpp @@ -8,6 +8,7 @@ namespace Crash // Used to communicate between the app and the monitor, fields are is overwritten with each event. static constexpr const int MAX_LONG_PATH = 0x7fff; + static constexpr const int MAX_FILENAME = 0xff; struct CrashSHM { @@ -28,8 +29,9 @@ namespace Crash HANDLE mSignalApp; HANDLE mSignalMonitor; HANDLE mShmMutex; - char mCrashDumpFilePath[MAX_LONG_PATH]; - char mFreezeDumpFilePath[MAX_LONG_PATH]; + char mDumpDirectoryPath[MAX_LONG_PATH]; + char mCrashDumpFileName[MAX_FILENAME]; + char mFreezeDumpFileName[MAX_FILENAME]; } mStartup; struct Crashed diff --git a/components/crashcatcher/windowscrashdumppathhelpers.cpp b/components/crashcatcher/windowscrashdumppathhelpers.cpp new file mode 100644 index 0000000000..ecb6b38d86 --- /dev/null +++ b/components/crashcatcher/windowscrashdumppathhelpers.cpp @@ -0,0 +1,20 @@ +#include "windowscrashdumppathhelpers.hpp" + +#include + +namespace Crash +{ + std::filesystem::path getCrashDumpPath(const CrashSHM& crashShm) + { + return (std::filesystem::path(Misc::StringUtils::stringToU8String(crashShm.mStartup.mDumpDirectoryPath)) + / Misc::StringUtils::stringToU8String(crashShm.mStartup.mCrashDumpFileName)) + .make_preferred(); + } + + std::filesystem::path getFreezeDumpPath(const CrashSHM& crashShm) + { + return (std::filesystem::path(Misc::StringUtils::stringToU8String(crashShm.mStartup.mDumpDirectoryPath)) + / Misc::StringUtils::stringToU8String(crashShm.mStartup.mFreezeDumpFileName)) + .make_preferred(); + } +} diff --git a/components/crashcatcher/windowscrashdumppathhelpers.hpp b/components/crashcatcher/windowscrashdumppathhelpers.hpp new file mode 100644 index 0000000000..fa64969301 --- /dev/null +++ b/components/crashcatcher/windowscrashdumppathhelpers.hpp @@ -0,0 +1,13 @@ +#ifndef COMPONENTS_CRASH_WINDOWSCRASHDUMPPATHHELPERS_H +#include "windows_crashshm.hpp" + +#include + +namespace Crash +{ + std::filesystem::path getCrashDumpPath(const CrashSHM& crashShm); + + std::filesystem::path getFreezeDumpPath(const CrashSHM& crashShm); +} + +#endif diff --git a/components/debug/debugging.cpp b/components/debug/debugging.cpp index b4e433273a..0bec74ce73 100644 --- a/components/debug/debugging.cpp +++ b/components/debug/debugging.cpp @@ -318,10 +318,7 @@ void setupLogging(const std::filesystem::path& logDir, std::string_view appName, #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); + Crash::CrashCatcher::instance()->updateDumpPath(logDir); } #endif } @@ -351,7 +348,7 @@ int wrapApplication(int (*innerApplication)(int argc, char* argv[]), int argc, c dumpDirectory = userProfile; } CoTaskMemFree(userProfile); - Crash::CrashCatcher crashy(argc, argv, dumpDirectory / crashDumpName, dumpDirectory / freezeDumpName); + Crash::CrashCatcher crashy(argc, argv, dumpDirectory, crashDumpName, freezeDumpName); #else const std::string crashLogName = Misc::StringUtils::lowerCase(appName) + "-crash.log"; // install the crash handler as soon as possible.