Skip to content

Commit c0b274e

Browse files
summeroffclaude
andcommitted
Address review: bound actions, lock-safe crash-time reads
- RegisterAction: pop on size() > Maximum so it truly retains 50 (was >=, capping at 49); matches AddServerWarning and the PR description. - ComputeServerWarnings: try_to_lock messageMutex instead of a blocking lock_guard. The crashing thread may already hold it (crashed inside AddServerWarning/RegisterAction); a blocking acquire would hang crash reporting. Return empty if it can't be acquired. - Replace OBS_API::getOBSLogGeneral() (returned a mutable deque ref that RequestOBSLog drained with no synchronization, racing node_obs_log()) with snapshotOBSLogGeneral(): copies and clears logReport.general under logMutex via try_to_lock. RequestOBSLog now consumes the snapshot, reverse-iterating to keep the newest-first ordering. - Include <algorithm> explicitly (std::transform was relying on a transitive include). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8886ac5 commit c0b274e

3 files changed

Lines changed: 24 additions & 12 deletions

File tree

obs-studio-server/source/nodeobs_api.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,9 +2048,15 @@ double OBS_API::getMemoryUsage()
20482048
return (double)os_get_proc_resident_size() / (1024.0 * 1024.0);
20492049
}
20502050

2051-
std::deque<std::string> &OBS_API::getOBSLogGeneral()
2051+
std::deque<std::string> OBS_API::snapshotOBSLogGeneral()
20522052
{
2053-
return logReport.general;
2053+
std::unique_lock<std::mutex> lock(logMutex, std::try_to_lock);
2054+
if (!lock.owns_lock())
2055+
return {};
2056+
2057+
std::deque<std::string> snapshot = std::move(logReport.general);
2058+
logReport.general.clear();
2059+
return snapshot;
20542060
}
20552061

20562062
std::string OBS_API::getCurrentVersion()

obs-studio-server/source/nodeobs_api.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,10 @@ class OBS_API {
120120
static double getMemoryUsage();
121121
static void getCurrentOutputStats(obs_output_t *output, OBS_API::OutputStats &outputStats);
122122

123-
static std::deque<std::string> &getOBSLogGeneral();
123+
// Snapshot (and clear) the general log tail under logMutex. Uses try_lock — the crash
124+
// handler calls this and must never block on the logging thread. Returns oldest-first,
125+
// empty if the lock can't be acquired.
126+
static std::deque<std::string> snapshotOBSLogGeneral();
124127

125128
static std::string getCurrentVersion();
126129
static std::string getUsername();

obs-studio-server/source/util-crashmanager.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "util-crashmanager.h"
2020
#include "util-metricsprovider.h"
2121

22+
#include <algorithm>
2223
#include <chrono>
2324
#include <codecvt>
2425
#include <filesystem>
@@ -1029,13 +1030,10 @@ nlohmann::json util::CrashManager::RequestOBSLog()
10291030
{
10301031
nlohmann::json result;
10311032

1032-
auto &general = OBS_API::getOBSLogGeneral();
1033-
while (!general.empty()) {
1034-
result.push_back(general.front());
1035-
general.pop_front();
1036-
}
1037-
1038-
std::reverse(result.begin(), result.end());
1033+
// snapshotOBSLogGeneral returns oldest-first under logMutex; emit newest-first.
1034+
std::deque<std::string> general = OBS_API::snapshotOBSLogGeneral();
1035+
for (auto it = general.rbegin(); it != general.rend(); ++it)
1036+
result.push_back(*it);
10391037

10401038
return result;
10411039
}
@@ -1064,7 +1062,12 @@ nlohmann::json util::CrashManager::ComputeServerWarnings()
10641062
{
10651063
nlohmann::json result;
10661064

1067-
std::lock_guard<std::mutex> lock(messageMutex);
1065+
// try_lock — the crashing thread may already hold messageMutex (e.g. it crashed inside
1066+
// AddServerWarning/RegisterAction); a blocking lock would hang crash reporting.
1067+
std::unique_lock<std::mutex> lock(messageMutex, std::try_to_lock);
1068+
if (!lock.owns_lock())
1069+
return result;
1070+
10681071
for (auto &msg : serverWarnings)
10691072
result.push_back(msg);
10701073

@@ -1243,7 +1246,7 @@ void RegisterAction(const std::string &message)
12431246
lastActions.back().first++;
12441247
} else {
12451248
lastActions.push({0, message});
1246-
if (lastActions.size() >= MaximumActionsRegistered) {
1249+
if (lastActions.size() > MaximumActionsRegistered) {
12471250
lastActions.pop();
12481251
}
12491252
}

0 commit comments

Comments
 (0)