From 39153b50517e19abe0067504aa657da8cc4f8bac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Janiszewski?=
 <janisozaur@users.noreply.github.com>
Date: Sun, 28 Sep 2025 21:01:36 +0200
Subject: [PATCH] Fix multithreaded access to additional crash files (#25271)

`GetScenarioInfo` can be called from a `JobPool` in `FileIndex::Build`,
resulting in multiple threads trying to access the `_uploadFiles` map
for breakpad.

This commit adds a mutex guard for `_uploadFiles` and provides unique
names for the entries.

The crash handler itself does not verify mutex state, possibly causing
data races, but I don't think it's safe to access it at this point and
we have to make do with whatever state is present at this point.
---
 src/openrct2/platform/Crash.cpp              |  4 ++++
 src/openrct2/scenario/ScenarioRepository.cpp | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/openrct2/platform/Crash.cpp b/src/openrct2/platform/Crash.cpp
index 1e23d9816b..7ebb4b8fd6 100644
--- a/src/openrct2/platform/Crash.cpp
+++ b/src/openrct2/platform/Crash.cpp
@@ -13,6 +13,7 @@
     #include <iterator>
     #include <map>
     #include <memory>
+    #include <mutex>
     #include <stdio.h>
 
     #if defined(_WIN32)
@@ -55,6 +56,7 @@ static const wchar_t* _wszCommitSha1Short = WSZ("");
 // OPENRCT2_ARCHITECTURE is required to be defined in version.h
 static const wchar_t* _wszArchitecture = WSZ(OPENRCT2_ARCHITECTURE);
 static std::map<std::wstring, std::wstring> _uploadFiles;
+static std::mutex _uploadFilesMutex;
 
     #define BACKTRACE_TOKEN "03ef82f2423418e09421e7758ee960d699f02146056e27c4e7eab5d08eb7d675"
 
@@ -345,6 +347,7 @@ CExceptionHandler CrashInit()
 void CrashRegisterAdditionalFile(const std::string& key, const std::string& path)
 {
 #ifdef USE_BREAKPAD
+    std::lock_guard<std::mutex> lock(_uploadFilesMutex);
     _uploadFiles[String::toWideChar(key.c_str())] = String::toWideChar(path.c_str());
 #endif // USE_BREAKPAD
 }
@@ -352,6 +355,7 @@ void CrashRegisterAdditionalFile(const std::string& key, const std::string& path
 void CrashUnregisterAdditionalFile(const std::string& key)
 {
 #ifdef USE_BREAKPAD
+    std::lock_guard<std::mutex> lock(_uploadFilesMutex);
     auto it = _uploadFiles.find(String::toWideChar(key.c_str()));
     if (it != _uploadFiles.end())
     {
diff --git a/src/openrct2/scenario/ScenarioRepository.cpp b/src/openrct2/scenario/ScenarioRepository.cpp
index 590362d0e6..94c7ba5aa5 100644
--- a/src/openrct2/scenario/ScenarioRepository.cpp
+++ b/src/openrct2/scenario/ScenarioRepository.cpp
@@ -34,6 +34,7 @@
 #include "ScenarioCategory.h"
 #include "ScenarioSources.h"
 
+#include <cstdint>
 #include <memory>
 #include <string>
 #include <vector>
@@ -207,15 +208,20 @@ private:
 
         struct CrashAdditionalFileRegistration
         {
+            std::string _key;
+
             CrashAdditionalFileRegistration(const std::string& path)
             {
+                // Use a unique key to avoid conflicts when GetScenarioInfo is called in a JobPool and multiple files are being
+                // processed in parallel.
+                _key = "load_park_" + std::to_string(reinterpret_cast<uintptr_t>(this));
                 // Register the file for crash upload if it asserts while loading.
-                CrashRegisterAdditionalFile("load_park", path);
+                CrashRegisterAdditionalFile(_key, path);
             }
             ~CrashAdditionalFileRegistration()
             {
                 // Deregister park file in case it was processed without hitting an assert.
-                CrashUnregisterAdditionalFile("load_park");
+                CrashUnregisterAdditionalFile(_key);
             }
         } crash_additional_file_registration(path);