1
0
mirror of https://github.com/OpenRCT2/OpenRCT2 synced 2026-01-15 11:03:00 +01:00

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.
This commit is contained in:
Michał Janiszewski
2025-09-28 21:01:36 +02:00
committed by GitHub
parent 5534e2ea7a
commit 39153b5051
2 changed files with 12 additions and 2 deletions

View File

@@ -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())
{

View File

@@ -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);