[CrashReporting] Breakpad everywhere, session detection, symbol CI (inc 3)

Breakpad on all platforms (no crashpad packaging); remove the Windows
MiniDumpWithFullMemory filter and the installer WER LocalDumps full-dump
registration. QLockFile per-process session tracking replaces the
Windows-only .dmp scan, placed after parser.process and the instance
hand-off so help/version exits and handed-off secondaries never leak a
lock. CI: Release-only DSN bake from SENTRY_DSN secret, tag-only
sentry-cli symbol upload (org cockatrice, project native).

Tests: 533/533 ctest green, format clean, code gate PASS.

cockatrice/src/client/crash/crash_reporter.cpp

@@ -5,12 +5,15 @@
 #endif
 
 #include <QByteArray>
+#include <QCoreApplication>
 #include <QDir>
 #include <QFile>
 #include <QJsonDocument>
 #include <QJsonObject>
+#include <QLockFile>
 #include <QLoggingCategory>
 #include <QRegularExpression>
+#include <QThread>
 #include <QUrl>
 #include <cstring>
 #include <libcockatrice/settings/personal_settings.h>
@@ -263,8 +266,13 @@ void CrashReporter::init(const QString &dsn,
     sentry_options_set_release(options, release.toUtf8().constData());
     sentry_options_set_database_path(options, sentryDatabasePath.toUtf8().constData());
     // Consent-gated uploads: crashes are captured into the local database but
-    // nothing leaves the machine until giveConsent().
+    // nothing leaves the machine until giveConsent(). cache_keep holds the previous run's
+    // crash envelope in <database>/cache instead of dropping it at init, so the Ask dialog
+    // decides its fate: giveConsent() flushes it via the transport retry, revokeConsent()
+    // deletes it below. This also covers the first crash after switching to AlwaysSend,
+    // which giveConsent() (running after init) would otherwise miss.
     sentry_options_set_require_user_consent(options, 1);
+    sentry_options_set_cache_keep(options, SENTRY_CACHE_KEEP_OFFLINE);
     sentry_options_set_before_send(options, scrubEventBeforeSend, nullptr);
     ready = (sentry_init(options) == 0);
 #else
@@ -291,6 +299,79 @@ void CrashReporter::shutdown()
     ready = false;
 }
 
+CrashReporter::~CrashReporter() = default;
+CrashReporter::CrashReporter() = default;
+
+bool CrashReporter::beginSession(const QString &dir)
+{
+    // Restartable: releasing first keeps begin->end->begin round-trips clean.
+    endSession();
+
+    QDir().mkpath(dir);
+    const QDir sessionDir(dir);
+    const QString selfName = QStringLiteral("session-%1").arg(QCoreApplication::applicationPid());
+
+    bool unclean = false;
+    const QStringList leftovers = sessionDir.entryList({QStringLiteral("session-*")}, QDir::Files);
+    for (const QString &name : leftovers) {
+        if (name == selfName) {
+            continue;
+        }
+        const QString path = sessionDir.filePath(name);
+        QLockFile probe(path);
+        // Long-lived session lock: stale means "owner dead", regardless of file age, so a
+        // crash followed by an immediate relaunch (younger than the 30 s default) is still
+        // detected. tryLock() only deletes the file when it finds it stale, so a live
+        // concurrent instance's lock is never touched here. (The static
+        // QLockFile::removeStaleLockFile(path) form does not exist in Qt5/Qt6, and the
+        // member removeStaleLockFile() force-removes without a staleness check, so neither
+        // is usable for this scan.)
+        probe.setStaleLockTime(0);
+        if (probe.tryLock(0)) {
+            unclean = true;
+            probe.unlock(); // deletes the stale file
+            continue;
+        }
+        // tryLock failed: either a live concurrent instance owns the file, or it is
+        // unreadable (garbage/empty) and therefore has no live owner. Only the latter is
+        // cleaned up; a live instance is left alone. On Unix QLockFile creates the file
+        // before writing the pid, so a second instance starting at the same moment can
+        // briefly observe an empty (or half-written) file for a live owner: re-read once
+        // after a beat so a live owner's fresh lock is never mistaken for garbage and deleted.
+        qint64 pid = 0;
+        QString hostname, appname;
+        if (!probe.getLockInfo(&pid, &hostname, &appname)) {
+            QThread::msleep(100);
+            if (!probe.getLockInfo(&pid, &hostname, &appname)) {
+                QFile::remove(path);
+                unclean = true;
+            }
+        }
+    }
+
+    sessionLockPath = sessionDir.filePath(selfName);
+    sessionLock = std::make_unique<QLockFile>(sessionLockPath);
+    sessionLock->setStaleLockTime(0);
+    if (!sessionLock->tryLock(0)) {
+        qCWarning(CrashReportLog) << "Failed to claim session lock at" << sessionLockPath;
+    }
+    return unclean;
+}
+
+void CrashReporter::endSession()
+{
+    if (sessionLock) {
+        // unlock() removes the lock file; nothing left to clean up.
+        sessionLock->unlock();
+        sessionLock.reset();
+    } else if (!sessionLockPath.isEmpty()) {
+        // The lock was never claimed (a live concurrent instance owns its file): only our
+        // own unclaimed file, if tryLock left one behind, is removed.
+        QFile::remove(sessionLockPath);
+    }
+    sessionLockPath.clear();
+}
+
 void CrashReporter::giveConsent()
 {
 #if COCKATRICE_WITH_CRASH_REPORTING
@@ -307,6 +388,10 @@ void CrashReporter::revokeConsent()
     // return to unknown, which blocks uploads too but loses the decision.
     if (ready) {
         sentry_user_consent_revoke();
+        // Drop the held crash envelope: without this it would sit in <database>/cache and
+        // upload on the next give (e.g. after switching to AlwaysSend), after the user
+        // explicitly refused it. The SDK recreates the directory as needed.
+        QDir(sentryDatabasePath + QStringLiteral("/cache")).removeRecursively();
     }
 #endif
 }

cockatrice/src/client/crash/crash_reporter.h

@@ -11,6 +11,7 @@
 #include <QString>
 #include <QStringList>
 #include <libcockatrice/settings/personal_settings.h>
+#include <memory>
 
 #ifndef COCKATRICE_WITH_CRASH_REPORTING
 #define COCKATRICE_WITH_CRASH_REPORTING 0
@@ -24,6 +25,8 @@
 #define COCKATRICE_SENTRY_RELEASE ""
 #endif
 
+class QLockFile;
+
 Q_DECLARE_LOGGING_CATEGORY(CrashReportLog);
 
 /**
@@ -34,21 +37,28 @@ Q_DECLARE_LOGGING_CATEGORY(CrashReportLog);
  * consent) skips the SDK entirely, so the mock dialog fallback behaves exactly
  * as before. With a DSN, init() arms require-user-consent and before_send
  * scrubbing (stale persisted consent from a previous AlwaysSend run is cleared
- * before init when the mode is not AlwaysSend); giveConsent()/revokeConsent()
+ * before init when the mode is not AlwaysSend, and cached crash envelopes are
+ * held for the dialog instead of being dropped); giveConsent()/revokeConsent()
  * gate uploads. attachToPendingCrashEnvelopes() folds the redacted log and the
  * optional user note into the pending native crash event(s) before consent
  * releases them, so dump, stack, log and note arrive as ONE Sentry event;
  * sendReport() is the fallback for unclean exits that produced no native crash
  * event, capturing a redacted message event with the log preview attached. All
  * SDK calls are guarded so OFF builds compile to no-ops.
+ * Session tracking (beginSession/endSession) is SDK-independent: a QLockFile per
+ * process in the database dir detects unclean exits cross-platform, replacing the
+ * legacy Windows-only .dmp scan. The held lock makes the reporter non-copyable and
+ * non-movable.
  */
 class CrashReporter
 {
 public:
-    CrashReporter() = default;
+    CrashReporter();
     CrashReporter(const CrashReporter &) = delete;
     CrashReporter &operator=(const CrashReporter &) = delete;
-    ~CrashReporter() = default;
+    CrashReporter(CrashReporter &&) = delete;
+    CrashReporter &operator=(CrashReporter &&) = delete;
+    ~CrashReporter();
 
     void init(const QString &dsn,
               const QString &release,
@@ -140,12 +150,22 @@ public:
      */
     static QString describeOriginMismatch(const QString &envelopePath, const QString &currentDsn);
 
+    /**
+     * @brief Claim this process's session lock, reporting a previous unclean exit.
+     * @return true if a leftover session lock from a dead process (or an unreadable
+     * garbage file) was found and cleaned; live concurrent instances are ignored.
+     */
+    bool beginSession(const QString &dir);
+    void endSession();
+
     static QString sanitizeLogLine(const QString &line);
     static QString redactCockatriceUrls(const QString &line);
 
 private:
     bool ready = false;
     QString sentryDatabasePath;
+    QString sessionLockPath;
+    std::unique_ptr<QLockFile> sessionLock;
 };
 
 #endif

cockatrice/src/main.cpp

@@ -46,9 +46,6 @@
 #include <QCoreApplication>
 #include <QCryptographicHash>
 #include <QDebug>
-#include <QDialog>
-#include <QDir>
-#include <QFile>
 #include <QFileInfo>
 #include <QGuiApplication>
 #include <QIcon>
@@ -108,60 +105,6 @@ static void CockatriceLogger(QtMsgType type, const QMessageLogContext &ctx, cons
     Logger::getInstance().log(type, ctx, logMessage);
 }
 
-#ifdef Q_OS_WIN
-// clang-format off
-#include <Windows.h>
-#include <DbgHelp.h>
-#include <ShlObj.h>
-#include <ctime>
-#include <filesystem>
-
-#pragma comment(lib, "DbgHelp.lib") // Link the DbgHelp library
-// clang-format on
-
-LONG WINAPI CockatriceUnhandledExceptionFilter(EXCEPTION_POINTERS *exceptionPointers)
-{
-    std::filesystem::path path;
-
-    // Find %LOCALAPPDATA% (or cheat at finding it)
-    wchar_t *localAppDataFolder;
-    if (SHGetKnownFolderPath(FOLDERID_LocalAppData, KF_FLAG_CREATE, NULL, &localAppDataFolder) != S_OK) {
-        path = std::filesystem::temp_directory_path().parent_path().parent_path();
-    } else {
-        path = std::filesystem::path(localAppDataFolder);
-    }
-
-    // Plan on writing crash files into %LOCALAPPDATA%/CrashDumps/Cockatrice/cockatrice.crash.*.dmp
-    path /= "CrashDumps";
-    path /= "Cockatrice";
-    if (!std::filesystem::exists(path)) {
-        std::filesystem::create_directories(path);
-    }
-    path /= "cockatrice.crash." + std::to_string(std::time(0)) + ".dmp";
-
-    // Create and write crash files
-#ifdef UNICODE
-    HANDLE hDumpFile =
-        CreateFile(path.wstring().c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
-#else
-    HANDLE hDumpFile =
-        CreateFile(path.string().c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
-#endif
-
-    MINIDUMP_EXCEPTION_INFORMATION mei;
-    mei.ExceptionPointers = exceptionPointers;
-    mei.ThreadId = GetCurrentThreadId();
-    mei.ClientPointers = 1;
-
-    MiniDumpWriteDump(GetCurrentProcess(), GetCurrentProcessId(), hDumpFile, MiniDumpWithFullMemory, &mei, nullptr,
-                      nullptr);
-
-    CloseHandle(hDumpFile);
-
-    return EXCEPTION_EXECUTE_HANDLER;
-}
-#endif
-
 void installNewTranslator()
 {
     QString lang = SettingsCache::instance().personal().getLang();
@@ -204,9 +147,9 @@ static QString redactActivationUrl(const QString &url)
 
 int main(int argc, char *argv[])
 {
-#ifdef Q_OS_WIN
-    SetUnhandledExceptionFilter(CockatriceUnhandledExceptionFilter);
-#endif
+    // No custom unhandled-exception filter: sentry owns native crash capture on all
+    // platforms. (The legacy MiniDumpWithFullMemory writer was removed as a privacy
+    // liability; unclean exits are detected via CrashReporter::beginSession below.)
 
     // Logging setup
 #ifdef Q_OS_APPLE
@@ -404,6 +347,16 @@ int main(int argc, char *argv[])
         }
     }
 
+    // Unclean-exit detection (cross-platform QLockFile session tracking, replacing the
+    // legacy Windows-only .dmp scan): a leftover session lock means the previous run died
+    // without reaching endSession(). Our own lock is held until the event loop finishes.
+    // Deliberately placed after parser.process() (--help/--version exit inside it and must
+    // not leak a lock) and after the instance hand-off (a handed-off secondary exits
+    // immediately and must never take a lock). Note this counts any death without
+    // endSession() as a crash, including SIGTERM at logout, Task Manager kills and
+    // session-end shutdowns; in AlwaysSend those send a report too.
+    const bool uncleanExit = crashReporter.beginSession(crashDatabasePath);
+
     rng = new RNG_SFMT(CryptoUtil::randomUInt64());
     themeManager = new ThemeManager;
     soundEngine = new SoundEngine;
@@ -499,30 +452,11 @@ int main(int argc, char *argv[])
 
     // Crash-report flow: with a configured DSN the SDK owns capture and upload
     // (consent-gated); with an empty DSN the dialog falls back to a local mock
-    // file and behaves exactly as before. Triggered by --simulate-crash or by
-    // leftover Windows crash dumps, which are consumed after handling so one
-    // crash never prompts twice.
+    // file and behaves exactly as before. The trigger is --simulate-crash or an
+    // unclean previous exit (leftover session lock); the old
+    // %LOCALAPPDATA%/CrashDumps .dmp scan is gone with the exception filter.
     {
-        QString crashDumpDir;
-#ifdef Q_OS_WIN
-        // Mirrors the writer (CockatriceUnhandledExceptionFilter): it appends CrashDumps/Cockatrice
-        // to %LOCALAPPDATA%, or, when its known-folder lookup fails, to the temp directory walked up
-        // two parent_path() steps. %LOCALAPPDATA% is this scan's stand-in for FOLDERID_LocalAppData:
-        // both come from the same profile, so when the lookup fails the variable is normally unset.
-        QString crashDumpRootPath = QString::fromLocal8Bit(qgetenv("LOCALAPPDATA"));
-        if (crashDumpRootPath.isEmpty()) {
-            // The writer's fallback expression, verbatim — QDir::cdUp() is existence-checked and
-            // would silently leave the path alone here, parent_path() is not.
-            const std::filesystem::path fallback = std::filesystem::temp_directory_path().parent_path().parent_path();
-            crashDumpRootPath = QString::fromStdWString(fallback.wstring());
-        }
-        crashDumpDir = QDir(crashDumpRootPath).absoluteFilePath(QStringLiteral("CrashDumps/Cockatrice"));
-#else
-        crashDumpDir = QDir::tempPath();
-#endif
-        const QStringList crashDumps =
-            QDir(crashDumpDir).entryList({QStringLiteral("cockatrice.crash.*.dmp")}, QDir::Files);
-        if (parser.isSet("simulate-crash") || !crashDumps.isEmpty()) {
+        if (parser.isSet("simulate-crash") || uncleanExit) {
             const PersonalSettings::CrashReportingMode crashMode =
                 SettingsCache::instance().personal().getCrashReportingMode();
             const bool crashReady = crashReporter.isReady();
@@ -549,10 +483,6 @@ int main(int argc, char *argv[])
                 crashDialog.exec();
             }
             // NeverSend: skip silently (no dialog, nothing written or sent).
-            // Handled dumps are consumed so one crash never prompts twice.
-            for (const QString &dump : crashDumps) {
-                QFile::remove(crashDumpDir + QStringLiteral("/") + dump);
-            }
         }
     }
 
@@ -580,8 +510,6 @@ int main(int argc, char *argv[])
 
     int ret = app.exec();
 
-    crashReporter.shutdown();
-
     qCInfo(MainLog) << "Event loop finished, terminating...";
     delete rng;
     PingPixmapGenerator::clear();
@@ -589,5 +517,10 @@ int main(int argc, char *argv[])
     UserLevelPixmapGenerator::clear();
     ManaSymbolPixmapGenerator::clear();
 
+    // Released last: crashes anywhere in the teardown above (a common place for Qt apps to
+    // crash) are still captured by the SDK and detected as unclean exits on the next launch.
+    crashReporter.shutdown();
+    crashReporter.endSession();
+
     return ret;
 }