Skip to content

Commit c89957c

Browse files
committed
MacOS: enable tests to achieve parity with Windows
* enable recording, streaming, dual-output tests on MacOS * set CI env var for MacOS github runner for obs_handler.ts * fix warning we cant assign null: `let secondContext: osn.IVideo = null;` * do not invoke `AddVideoContext` on CI builds in enhanced broadcasting tests since these are disabled. * main.yml: upgrade to macos-15 for arm64 ARCH (same OS Intel ARCH is using). * main.yml: upload OBS logs to troubleshoot unit tests. main.yml: set fail-fast to false * turn off fail-fast so the tests can continue running which will fix issue where if macos-15-intel fails, macos-arm64 is cancelled before it can finish running. osn_handler: log time delta for long duration * add warning when signal received took longer then expectedDeadline obs_handler: assign explicit log filename for tests Fix orphaned worker thread: The WorkerSignals::worker keeps polling Query. When a test fails before calling destroy(), the server-side recording gets freed when OBS shuts down, but the orphaned worker thread continues. When the next test file creates a fresh OBS connection, Controller::GetInstance().GetConnection() now returns the new session's connection. The orphaned worker sends Query(oldUID) to the new server, which has no knowledge of that uid → "Recording reference is not valid." Remove source message listener when test ends * stops worker thread properly * test_osn_simple_recording: remove lamda to enable timeout Tests: simple_recording,test_osn_advanced_replayBuffer,test_osn_advanced_recording,dual_output - wrap with try/finally * update any test that starts a worker thread via create() to properly invoke destroy() within a finally block to guanrantee the worker thread is stopped even if the test fails. Prevents orphaned worker threads from invoking Query() every 33ms on the new IPC connection created by other tests. guarantees cleanup even when tests throw so they tests will clean up proactively, and any orphaned workers that slip through will stop themselves rather than polluting subsequent test runs. validate totalSleepMS does not have a negative value When the IPC call takes longer than sleepIntervalMS on an overloaded CI runner), sleepIntervalMS - dur.count() produces a negative result that wraps to a huge size_t value, causing the worker to sleep for an effectively infinite duration. After that, no more signals are ever polled — any test waiting for a signal hits the 30-second timeout. do not share stdout/stderr with parent process On macOS, the server is spawned with posix_spawnp with NULL for file_actions, which means it inherits the parent's stdout/stderr. On Windows this doesn't happen because CreateProcessW uses DETACHED_PROCESS | CREATE_NO_WINDOW, which cuts the server off from the parent's console entirely. obs_handler: add more retryable timeouts for flaky tests osn-replay-buffer: invoke output handler to save * makes test more reliable * follows similar pattern used by OBS::ReplayBufferSave
1 parent 3e13fce commit c89957c

22 files changed

Lines changed: 1183 additions & 1144 deletions

.github/workflows/main.yml

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ jobs:
4242
ReleaseName: release
4343
Architecture: x86_64
4444
- BuildReleases: Release-arm64
45-
image: macos-14
45+
image: macos-15
4646
BuildConfig: RelWithDebInfo
4747
ReleaseName: release
4848
Architecture: arm64
@@ -99,6 +99,7 @@ jobs:
9999
needs: build-macos
100100
runs-on: ${{ matrix.image }}
101101
strategy:
102+
fail-fast: false # Don't cancel the other architecture's tests if one fails, so we can get test results for both.
102103
matrix:
103104
BuildReleases: [Release-x86_64, Release-arm64]
104105
include:
@@ -108,7 +109,7 @@ jobs:
108109
ReleaseName: release
109110
Architecture: x86_64
110111
- BuildReleases: Release-arm64
111-
image: macos-14
112+
image: macos-15
112113
BuildConfig: RelWithDebInfo
113114
ReleaseName: release
114115
Architecture: arm64
@@ -151,6 +152,15 @@ jobs:
151152
OSN_ACCESS_KEY_ID: ${{secrets.AWS_RELEASE_ACCESS_KEY_ID}}
152153
OSN_SECRET_ACCESS_KEY: ${{secrets.AWS_RELEASE_SECRET_ACCESS_KEY}}
153154
RELEASE_NAME: ${{matrix.ReleaseName}}
155+
CI: true
156+
SUPPRESS_STREAMLABS_OBS_LOGS: true # Prevents logs from being printed in the test output, but still generates log files that can be uploaded as artifacts.
157+
- name: Upload OBS logs
158+
if: ${{ always() }}
159+
uses: actions/upload-artifact@v4
160+
with:
161+
name: obs-logs-mac-${{ matrix.Architecture }}
162+
path: tests/osn-tests/osnData/slobs-client/node-obs/logs/
163+
if-no-files-found: ignore
154164
# Run even after test failures so the PR still gets the flaky summary.
155165
- name: Publish flaky test check
156166
if: ${{ always() }}
@@ -181,7 +191,7 @@ jobs:
181191
ReleaseName: release
182192
Architecture: x86_64
183193
- BuildReleases: Release-arm64
184-
image: macos-14
194+
image: macos-15
185195
BuildConfig: RelWithDebInfo
186196
ReleaseName: release
187197
Architecture: arm64
@@ -234,7 +244,7 @@ jobs:
234244
ReleaseName: release
235245
Architecture: x86_64
236246
- BuildReleases: Release-arm64
237-
image: macos-14
247+
image: macos-15
238248
BuildConfig: RelWithDebInfo
239249
ReleaseName: release
240250
Architecture: arm64
@@ -360,6 +370,13 @@ jobs:
360370
OSN_ACCESS_KEY_ID: ${{secrets.AWS_RELEASE_ACCESS_KEY_ID}}
361371
OSN_SECRET_ACCESS_KEY: ${{secrets.AWS_RELEASE_SECRET_ACCESS_KEY}}
362372
RELEASE_NAME: release
373+
- name: Upload OBS logs
374+
if: ${{ always() }}
375+
uses: actions/upload-artifact@v4
376+
with:
377+
name: obs-logs-windows
378+
path: tests/osn-tests/osnData/slobs-client/node-obs/logs/
379+
if-no-files-found: ignore
363380
# Run even after test failures so the PR still gets the flaky summary.
364381
- name: Publish flaky test check
365382
if: ${{ always() }}

obs-studio-client/source/controller.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,16 @@ std::wstring utfWorkingDir = L"";
4949
#include <wchar.h>
5050
#include <windows.h>
5151
#else
52+
#include <algorithm>
53+
#include <errno.h>
5254
#include <signal.h>
5355
#include <libproc.h>
5456
#include <iostream>
5557
#include <spawn.h>
58+
#include <fcntl.h>
59+
#include <strings.h>
60+
#include <unistd.h>
61+
#include <sys/wait.h>
5662
extern char **environ;
5763
#endif
5864

@@ -301,16 +307,31 @@ std::shared_ptr<ipc::client> Controller::host(const std::string &uri)
301307
int st = proc_pidinfo(pids[i], PROC_PIDTBSDINFO, 0, &proc, PROC_PIDTBSDINFO_SIZE);
302308
if (st == PROC_PIDTBSDINFO_SIZE) {
303309
if (strcmp("obs64", proc.pbi_name) == 0) {
304-
if (pids[i] != 0)
305-
kill(pids[i], SIGKILL);
310+
if (pids[i] != 0 && kill(pids[i], SIGKILL) != 0) {
311+
std::cout << "Warning: could not kill orphaned/former obs64 process" << std::endl;
312+
}
306313
}
307314
}
308315
}
309316

310317
pid_t pid;
311318
std::vector<const char *> argv = {"obs64", uri.c_str(), version.c_str(), serverBinaryPath.c_str(), nullptr};
312319

313-
int ret = posix_spawnp(&pid, serverBinaryPath.c_str(), NULL, NULL, const_cast<char *const *>(argv.data()), environ);
320+
const char *suppressLogsEnv = std::getenv("SUPPRESS_STREAMLABS_OBS_LOGS");
321+
int ret = 0;
322+
if (suppressLogsEnv == nullptr || strcasecmp(suppressLogsEnv, "false") == 0) {
323+
// For development, it can be helpful for process to share stdout/stderr.
324+
ret = posix_spawnp(&pid, serverBinaryPath.c_str(), NULL, NULL, const_cast<char *const *>(argv.data()), environ);
325+
} else {
326+
// Do not send the logs to stdout/stderr.
327+
posix_spawn_file_actions_t file_actions;
328+
posix_spawn_file_actions_init(&file_actions);
329+
posix_spawn_file_actions_addopen(&file_actions, STDOUT_FILENO, "/dev/null", O_WRONLY, 0);
330+
posix_spawn_file_actions_addopen(&file_actions, STDERR_FILENO, "/dev/null", O_WRONLY, 0);
331+
ret = posix_spawnp(&pid, serverBinaryPath.c_str(), &file_actions, NULL, const_cast<char *const *>(argv.data()), environ);
332+
posix_spawn_file_actions_destroy(&file_actions);
333+
}
334+
314335
if (ret != 0) {
315336
std::cerr << "Could not spawn the server at " << serverBinaryPath.c_str() << " with error code: " << ret << std::endl;
316337
return nullptr;

obs-studio-client/source/nodeobs_api.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
******************************************************************************/
1818

1919
#include "controller.hpp"
20+
#include <filesystem>
2021
#include "osn-error.hpp"
2122
#include "nodeobs_api.hpp"
2223
#include <sstream>
@@ -36,12 +37,15 @@ Napi::Value api::OBS_API_initAPI(const Napi::CallbackInfo &info)
3637
std::string language;
3738
std::string version;
3839
std::string crashserverurl;
40+
std::string logFilename;
3941

4042
ASSERT_GET_VALUE(info, info[0], language);
4143
ASSERT_GET_VALUE(info, info[1], path);
4244
ASSERT_GET_VALUE(info, info[2], version);
4345
if (info.Length() > 3)
4446
ASSERT_GET_VALUE(info, info[3], crashserverurl);
47+
if (info.Length() > 4)
48+
ASSERT_GET_VALUE(info, info[4], logFilename);
4549

4650
auto conn = GetConnection(info);
4751
if (!conn)
@@ -50,7 +54,7 @@ Napi::Value api::OBS_API_initAPI(const Napi::CallbackInfo &info)
5054
conn->set_freeze_callback(ipc_freeze_callback, path);
5155

5256
std::vector<ipc::value> response = conn->call_synchronous_helper(
53-
"API", "OBS_API_initAPI", {ipc::value(path), ipc::value(language), ipc::value(version), ipc::value(crashserverurl)});
57+
"API", "OBS_API_initAPI", {ipc::value(path), ipc::value(language), ipc::value(version), ipc::value(crashserverurl), ipc::value(logFilename)});
5458

5559
// The API init method will return a response error + graphical error
5660
// If there is a problem with the IPC the number of responses here will be zero so we must validate the

obs-studio-client/source/nodeobs_autoconfig.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ void autoConfig::worker()
6666
do_sleep:
6767
auto tp_end = std::chrono::high_resolution_clock::now();
6868
auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(tp_end - tp_start);
69-
totalSleepMS = sleepIntervalMS - dur.count();
69+
auto durCount = dur.count();
70+
totalSleepMS = durCount < sleepIntervalMS ? sleepIntervalMS - durCount : 0;
7071
std::this_thread::sleep_for(std::chrono::milliseconds(totalSleepMS));
7172
}
7273
return;

obs-studio-client/source/nodeobs_service.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,8 @@ void service::worker()
348348

349349
auto tp_end = std::chrono::high_resolution_clock::now();
350350
auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(tp_end - tp_start);
351-
totalSleepMS = sleepIntervalMS - dur.count();
351+
auto durCount = dur.count();
352+
totalSleepMS = durCount < sleepIntervalMS ? sleepIntervalMS - durCount : 0;
352353
std::this_thread::sleep_for(std::chrono::milliseconds(totalSleepMS));
353354
}
354355

obs-studio-client/source/worker-signals.hpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
******************************************************************************/
1818

1919
#pragma once
20+
#include <atomic>
21+
#include <iostream>
2022
#include <napi.h>
2123
#include "osn-error.hpp"
2224
#include "utility.hpp"
@@ -42,18 +44,21 @@ class WorkerSignals {
4244
~WorkerSignals(){};
4345

4446
protected:
45-
bool isWorkerRunning;
46-
bool workerStop;
47+
std::atomic<bool> isWorkerRunning;
48+
std::atomic<bool> workerStop;
49+
std::atomic<bool> isOrphaned;
4750
uint32_t sleepIntervalMS;
4851
std::thread *workerThread;
4952
Napi::ThreadSafeFunction jsThread;
5053
Napi::FunctionReference cb;
5154

5255
void startWorker(napi_env env, Napi::Function asyncCallback, const std::string &name, const uint64_t &refID)
5356
{
54-
if (!workerStop || isWorkerRunning)
57+
// If worker has been orphaned; allow it to be rejoined
58+
if (!isOrphaned && (!workerStop || isWorkerRunning))
5559
return;
5660

61+
isOrphaned = false;
5762
isWorkerRunning = true;
5863
workerStop = false;
5964
jsThread = Napi::ThreadSafeFunction::New(env, asyncCallback, name.c_str(), 0, 1, [](Napi::Env) {});
@@ -88,6 +93,17 @@ class WorkerSignals {
8893
auto conn = Controller::GetInstance().GetConnection();
8994
if (conn) {
9095
std::vector<ipc::value> response = conn->call_synchronous_helper(name, "Query", {ipc::value(refID)});
96+
if (!response.empty()) {
97+
ErrorCode firstError = (ErrorCode)response[0].value_union.ui64;
98+
if (firstError == ErrorCode::InvalidReference) {
99+
// This typically happens if the worker thread is orphaned.
100+
std::string errorMessage = response.size() > 1 ? response[1].value_str : "";
101+
std::cout << "Worker thread exiting due to Invalid reference error encountered: " << errorMessage << std::endl;
102+
isWorkerRunning = false;
103+
isOrphaned = true;
104+
break;
105+
}
106+
}
91107
if ((response.size() == 5) && signalsList.size() < maximum_signals_in_queue) {
92108
ErrorCode error = (ErrorCode)response[0].value_union.ui64;
93109
if (error == ErrorCode::Ok) {
@@ -122,7 +138,8 @@ class WorkerSignals {
122138

123139
auto tp_end = std::chrono::high_resolution_clock::now();
124140
auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(tp_end - tp_start);
125-
totalSleepMS = sleepIntervalMS - dur.count();
141+
auto durCount = dur.count();
142+
totalSleepMS = durCount < sleepIntervalMS ? sleepIntervalMS - durCount : 0;
126143
std::this_thread::sleep_for(std::chrono::milliseconds(totalSleepMS));
127144
}
128145

@@ -140,4 +157,4 @@ class WorkerSignals {
140157
workerThread->join();
141158
}
142159
}
143-
};
160+
};

obs-studio-server/source/nodeobs_api.cpp

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,17 @@ void addModulePaths()
835835
#endif
836836
}
837837

838+
std::filesystem::path sanitize_path(const std::filesystem::path &input)
839+
{
840+
std::filesystem::path normalized = input.lexically_normal();
841+
842+
if (normalized.is_absolute() || normalized.string().find("..") != std::string::npos) {
843+
return {};
844+
}
845+
846+
return normalized;
847+
}
848+
838849
static void listEncoders(obs_encoder_type type)
839850
{
840851
constexpr uint32_t hide_flags = OBS_ENCODER_CAP_DEPRECATED | OBS_ENCODER_CAP_INTERNAL;
@@ -874,10 +885,20 @@ void OBS_API::OBS_API_initAPI(void *data, const int64_t id, const std::vector<ip
874885
std::string appdata = args[0].value_str;
875886
std::string locale = args[1].value_str;
876887
currentVersion = args[2].value_str;
888+
std::string logFilename;
889+
// Skip index 3 which is reserved for crash-handler server
890+
if (args.size() > 4) {
891+
std::string logname = sanitize_path(args[4].value_str).string();
892+
if (logname.size() > 0) {
893+
std::ostringstream ss;
894+
ss << logname << '-' << GenerateTimeDateFilename("txt");
895+
logFilename = ss.str();
896+
}
897+
}
877898
utility::osn_current_version(currentVersion);
878899

879900
/* Logging */
880-
std::string filename = GenerateTimeDateFilename("txt");
901+
std::string filename = logFilename.size() > 0 ? logFilename : GenerateTimeDateFilename("txt");
881902
std::string log_path = appdata;
882903
log_path.append("/node-obs/logs/");
883904

obs-studio-server/source/osn-replay-buffer.cpp

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -152,19 +152,23 @@ void osn::IReplayBuffer::Query(void *data, const int64_t id, const std::vector<i
152152

153153
void osn::IReplayBuffer::Save(void *data, const int64_t id, const std::vector<ipc::value> &args, std::vector<ipc::value> &rval)
154154
{
155-
obs_enum_hotkeys(
156-
[](void *data, obs_hotkey_id id, obs_hotkey_t *key) {
157-
if (obs_hotkey_get_registerer_type(key) == OBS_HOTKEY_REGISTERER_OUTPUT) {
158-
std::string key_name = obs_hotkey_get_name(key);
159-
if (key_name.compare("ReplayBuffer.Save") == 0) {
160-
obs_hotkey_enable_callback_rerouting(true);
161-
obs_hotkey_trigger_routed_callback(id, true);
162-
}
163-
}
164-
return true;
165-
},
166-
nullptr);
155+
ReplayBuffer *replayBuffer = static_cast<ReplayBuffer *>(osn::IFileOutput::Manager::GetInstance().find(args.at(0).value_union.ui64));
156+
if (!replayBuffer) {
157+
PRETTY_ERROR_RETURN(ErrorCode::InvalidReference, "ReplayBuffer reference is not valid.");
158+
}
167159

160+
obs_output_t *output = replayBuffer->GetOutput();
161+
if (!output) {
162+
PRETTY_ERROR_RETURN(ErrorCode::InvalidReference, "Invalid replay buffer output.");
163+
}
164+
165+
calldata_t cd = {0};
166+
proc_handler_t *ph = obs_output_get_proc_handler(output);
167+
bool hasInvoked = proc_handler_call(ph, "save", &cd);
168+
calldata_free(&cd);
169+
170+
if (!hasInvoked)
171+
PRETTY_ERROR_RETURN(ErrorCode::NotFound, "Could not find ReplayBuffer::Save");
168172
rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
169173
AUTO_DEBUG;
170-
}
174+
}

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"local:build": "cmake --build build --target install --config Debug",
2323
"local:clean": "rm -rf build/*",
2424
"test": "electron-mocha -t 80000 --js-flags=\"--expose-gc\" --color -r ts-node/register tests/osn-tests/src/**/*.ts --reporter tests/osn-tests/util/list-reporter.js",
25-
"test:ci": "yarn run test --retries 2"
25+
"test:ci": "yarn run test --retries 3"
2626
},
2727
"devDependencies": {
2828
"@aws-sdk/client-s3": "^3.0.0",

tests/osn-tests/src/test_nodeobs_autoconfig.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { deleteConfigFiles } from '../util/general';
99
const testName = 'nodeobs_autoconfig';
1010

1111
describe(testName, function() {
12-
this.timeout(30000)
1312
let obs: OBSHandler;
1413
let hasTestFailed: boolean = false;
1514

@@ -50,9 +49,6 @@ describe(testName, function() {
5049
});
5150

5251
it('Run autoconfig', async function() {
53-
if (obs.isDarwin()) {
54-
this.skip();
55-
}
5652
const start = performance.now();
5753
let progressInfo: IConfigProgress;
5854
let settingValue: any;

0 commit comments

Comments
 (0)