From 2035a07084236feeec42ef1d2dfb9c87f5e1de15 Mon Sep 17 00:00:00 2001 From: Mack Straight Date: Sat, 3 Oct 2026 10:43:23 -0400 Subject: [PATCH 1/2] test: check that Configure doesn't create a second inner driver Adds SarTest control-panel and an issue-133-control-panel scenario in a new "issues" group (on by default). With the host running, it opens SAR's control panel, sends the hardware interface's Configure button click, cancels the panel, and fails if the software clock ever had two live instances. The clock now counts its live instances and controlPanel calls and reports them in ClockStats. Co-Authored-By: Claude Opus 5.5 --- SarTest/SarTest.cpp | 169 ++++++++++++++++++++++++++++++++++ SarTest/asiohost.cpp | 19 ++++ SarTest/asiohost.h | 6 +- SarTestClock/SarTestClock.cpp | 15 +++ SarTestClock/clockstats.h | 6 ++ tools/test/Invoke-SarTest.ps1 | 10 +- tools/test/README.md | 13 ++- 7 files changed, 232 insertions(+), 6 deletions(-) diff --git a/SarTest/SarTest.cpp b/SarTest/SarTest.cpp index cd94627..c42c483 100644 --- a/SarTest/SarTest.cpp +++ b/SarTest/SarTest.cpp @@ -23,6 +23,9 @@ #include "wasapi.h" #include "clockstats.h" +#include +#include + using namespace SarTest; namespace { @@ -48,6 +51,10 @@ int usage() " SarTest race [--cycles K] [--up S] [--down MS] [--openers T] [--hold MS]\n" " Start and stop the host repeatedly while threads keep opening and\n" " closing streams on its endpoints; report any call that hangs.\n" + " SarTest control-panel\n" + " Open SAR's control panel on a running host, press the hardware\n" + " interface's Configure button and close it again; fail if a second\n" + " clock instance was created (issue #133).\n" "\n" "Layout options: --endpoints N (playback/recording pairs, default 2)\n" " --channels C (per endpoint, default 2) --prefix \n" @@ -484,6 +491,166 @@ int cmdRun(const Args& args) return rc; } +// Control IDs in SarAsio's endpoints page (SarAsio.rc). +const int kConfigureHardwareInterfaceButton = 1002; + +struct ControlPanelWindow +{ + DWORD pid = 0; + HWND sheet = nullptr; + HWND button = nullptr; +}; + +BOOL CALLBACK findConfigureButton(HWND hwnd, LPARAM lparam) +{ + auto *find = (ControlPanelWindow *)lparam; + wchar_t cls[32] = {}; + + GetClassNameW(hwnd, cls, 32); + + if (GetDlgCtrlID(hwnd) == kConfigureHardwareInterfaceButton && + _wcsicmp(cls, L"Button") == 0) { + find->button = hwnd; + return FALSE; + } + + return TRUE; +} + +BOOL CALLBACK findControlPanel(HWND hwnd, LPARAM lparam) +{ + auto *find = (ControlPanelWindow *)lparam; + DWORD pid = 0; + + GetWindowThreadProcessId(hwnd, &pid); + + if (pid != find->pid || !IsWindowVisible(hwnd)) { + return TRUE; + } + + EnumChildWindows(hwnd, findConfigureButton, lparam); + + if (find->button) { + find->sheet = hwnd; + return FALSE; + } + + return TRUE; +} + +// Issue #133: with the host running, the hardware interface's Configure +// button in SAR's control panel created a second instance of the inner ASIO +// driver instead of using the running one. ASIO drivers may assume one +// instance per process; FlexASIO crashed on it. +int cmdControlPanel(const Args& args) +{ + EndpointLayout layout = EndpointLayout::fromArgs(args); + + if (!prepareConfig(args, layout)) { + return 1; + } + + AsioHost host(hostOptions(args, layout)); + + if (!host.load() || !host.open()) { + return 1; + } + + if (!host.start()) { + host.close(); + return 1; + } + + bool found = false; + bool enabled = false; + bool clicked = false; + std::thread clicker([&] { + ControlPanelWindow window; + + window.pid = GetCurrentProcessId(); + + for (int i = 0; i < 200 && !window.button; ++i) { + Sleep(50); + EnumWindows(findControlPanel, (LPARAM)&window); + } + + if (!window.button) { + logf("SAR's control panel did not appear within 10 s"); + return; + } + + found = true; + enabled = IsWindowEnabled(window.button) != FALSE; + logf("Found the control panel; Configure is %s", enabled ? "enabled" : "disabled"); + + DWORD_PTR ignored = 0; + + // What a click on the button sends its page. The handler runs on the + // panel's thread and returns once the inner driver's panel does. + clicked = SendMessageTimeoutW(GetParent(window.button), WM_COMMAND, + MAKEWPARAM(kConfigureHardwareInterfaceButton, BN_CLICKED), + (LPARAM)window.button, SMTO_NORMAL, 10000, &ignored) != 0; + + if (!clicked) { + logf("The Configure handler did not return within 10 s"); + } + + PostMessageW(window.sheet, PSM_PRESSBUTTON, PSBTN_CANCEL, 0); + }); + + logf("Opening SAR's control panel"); + + bool opened = host.controlPanel(); + + clicker.join(); + host.readClockStats(); + + SarTestClock::ClockStats clock = host.stats().clock; + + host.close(); + logf("Clock instances: %u alive, at most %u; inner controlPanel calls: %llu", + clock.liveInstances, clock.maxLiveInstances, + (unsigned long long)clock.controlPanelCalls); + + int rc = 0; + std::string failure; + + if (!opened || !found || !enabled || !clicked) { + rc = 2; + failure = "could not drive the control panel"; + } else if (clock.controlPanelCalls == 0) { + rc = 2; + failure = "Configure did not open the clock's control panel"; + } else if (clock.maxLiveInstances > 1) { + rc = 2; + failure = "Configure created a second clock instance"; + } + + if (rc != 0) { + logf("%s", failure.c_str()); + } + + JsonObject result; + + result.setString("command", "control-panel") + .setBool("passed", rc == 0) + .setInt("exitCode", rc) + .setBool("panelFound", found) + .setBool("configureEnabled", enabled) + .setBool("configureClicked", clicked) + .setInt("maxClockInstances", clock.maxLiveInstances) + .setInt("clockControlPanelCalls", (long long)clock.controlPanelCalls) + .setRaw("host", host.toJson()); + + if (!failure.empty()) { + result.setString("failure", failure); + } + + writeResults(args, result.str()); + logf("%s", rc == 0 ? "PASSED" : "FAILED"); + return rc; +} + } // namespace int wmain(int argc, wchar_t **argv) @@ -517,6 +684,8 @@ int wmain(int argc, wchar_t **argv) rc = cmdRun(args); } else if (args.command == L"race") { rc = cmdRace(args); + } else if (args.command == L"control-panel") { + rc = cmdControlPanel(args); } else { rc = usage(); } diff --git a/SarTest/asiohost.cpp b/SarTest/asiohost.cpp index 90dfed1..4db0d37 100644 --- a/SarTest/asiohost.cpp +++ b/SarTest/asiohost.cpp @@ -365,6 +365,25 @@ void AsioHost::close() _asio = nullptr; } +bool AsioHost::controlPanel() +{ + if (!_asio) { + return false; + } + + // The panel waits for whoever closes it, so it is not a watched phase. + setPhase("idle"); + + AsioStatus status = _asio->controlPanel(); + + if (status != AsioStatus::OK) { + logf("IASIO::controlPanel failed: %ld", (long)status); + return false; + } + + return true; +} + void AsioHost::readClockStats() { SarTestClock::ClockStats stats = {}; diff --git a/SarTest/asiohost.h b/SarTest/asiohost.h index 5935211..fcf6368 100644 --- a/SarTest/asiohost.h +++ b/SarTest/asiohost.h @@ -76,6 +76,11 @@ class AsioHost bool start(); bool stop(); void close(); + // IASIO::controlPanel. SarAsio's panel is modal: this returns once + // something closes it. + bool controlPanel(); + // Refreshes stats().clock from the software clock. + void readClockStats(); bool running() const { return _running; } const AsioHostStats& stats() const { return _stats; } @@ -91,7 +96,6 @@ class AsioHost void onTick(long bufferIndex); void setPhase(const char *phase); void watchdog(); - void readClockStats(); AsioHostOptions _options; AsioHostStats _stats; diff --git a/SarTestClock/SarTestClock.cpp b/SarTestClock/SarTestClock.cpp index fdc8c67..4b553ea 100644 --- a/SarTestClock/SarTestClock.cpp +++ b/SarTestClock/SarTestClock.cpp @@ -62,6 +62,9 @@ const wchar_t kInprocKey[] = L"CLSID\\" SAR_TEST_CLOCK_CLSID_STR L"\\InProcServe HMODULE gModule = nullptr; std::atomic gObjectCount{ 0 }; std::atomic gLockCount{ 0 }; +std::atomic gClockCount{ 0 }; +std::atomic gMaxClockCount{ 0 }; +std::atomic gControlPanelCalls{ 0 }; long envLong(const wchar_t *name, long def) { @@ -90,6 +93,13 @@ class SoftwareClock: public IASIO SoftwareClock() { gObjectCount++; + + long clocks = ++gClockCount; + long max = gMaxClockCount.load(); + + while (clocks > max && !gMaxClockCount.compare_exchange_weak(max, clocks)) { + } + _sampleRate = (double)envLong(L"SAR_TEST_CLOCK_RATE", 48000); _bufferFrames = envLong(L"SAR_TEST_CLOCK_FRAMES", 480); _inputCount = envLong(L"SAR_TEST_CLOCK_INPUTS", 2); @@ -99,6 +109,7 @@ class SoftwareClock: public IASIO virtual ~SoftwareClock() { disposeBuffers(); + gClockCount--; gObjectCount--; } @@ -359,6 +370,7 @@ class SoftwareClock: public IASIO AsioStatus controlPanel() override { + gControlPanelCalls++; return AsioStatus::OK; } @@ -381,6 +393,9 @@ class SoftwareClock: public IASIO stats->reserved = 0; stats->maxCallbackMs = _maxCallbackMs.load(); stats->slowCallbacks = _slowCallbacks.load(); + stats->liveInstances = (uint32_t)gClockCount.load(); + stats->maxLiveInstances = (uint32_t)gMaxClockCount.load(); + stats->controlPanelCalls = gControlPanelCalls.load(); return AsioStatus::OK; } diff --git a/SarTestClock/clockstats.h b/SarTestClock/clockstats.h index 29a5fe6..a6c2461 100644 --- a/SarTestClock/clockstats.h +++ b/SarTestClock/clockstats.h @@ -48,6 +48,12 @@ struct ClockStats // there stalls every endpoint at once. double maxCallbackMs; uint64_t slowCallbacks; // callbacks that took more than half a period + // Clock objects alive in the process now and at most so far, and + // controlPanel calls on any of them. ASIO drivers may assume a single + // instance per process, so a host (or SarAsio) must never create two. + uint32_t liveInstances; + uint32_t maxLiveInstances; + uint64_t controlPanelCalls; }; } // namespace SarTestClock diff --git a/tools/test/Invoke-SarTest.ps1 b/tools/test/Invoke-SarTest.ps1 index f1d501b..559139e 100644 --- a/tools/test/Invoke-SarTest.ps1 +++ b/tools/test/Invoke-SarTest.ps1 @@ -34,8 +34,8 @@ param( [int]$TimeoutSeconds = 600, [switch]$SkipInstall, [switch]$SkipKillTest, - # Any of matrix, race, kill. Installation always runs. - [string[]]$Scenarios = @('matrix', 'race', 'kill'), + # Any of issues, matrix, race, kill. Installation always runs. + [string[]]$Scenarios = @('issues', 'matrix', 'race', 'kill'), [int]$RaceCycles = 10 ) @@ -145,6 +145,12 @@ if (-not $SkipInstall) { $common = @('--sarasio', "`"$sarAsio`"", '--channels', $Channels) +if ($Scenarios -contains 'issues') { + # Regression checks for specific GitHub issues, one short host each. + $summary.scenarios += Invoke-Scenario 'issue-133-control-panel' ( + @('control-panel', '--endpoints', 1) + $common) -Timeout 120 +} + if ($Scenarios -contains 'matrix') { foreach ($count in $EndpointCounts) { $summary.scenarios += Invoke-Scenario "run-${count}x${Channels}" ( diff --git a/tools/test/README.md b/tools/test/README.md index 57f1a79..fa74263 100644 --- a/tools/test/README.md +++ b/tools/test/README.md @@ -80,8 +80,14 @@ them, which is what applications that use a SAR endpoint as their default device do while a DAW starts. Every WASAPI call and every host call is watched; one that does not return within `--phase-timeout` is reported as `HANG`. The scenario fails if any call hung, the host failed to start or -stop, or no stream was ever opened. `Invoke-SarTest.ps1 -Scenarios` selects -any of `matrix`, `race` and `kill`. +stop, or no stream was ever opened. + +The `issue-*` scenarios are regression checks for specific GitHub issues. +`SarTest.exe control-panel` (#133) opens SAR's control panel on a running +host, presses the hardware interface's Configure button and checks that the +clock never had a second instance alive. It needs an interactive desktop. +`Invoke-SarTest.ps1 -Scenarios` selects any of `issues`, `matrix`, `race` +and `kill`. ## Commands @@ -93,6 +99,7 @@ SarTest host [--iterations K] [--duration S] SarTest wasapi [--duration S] [--wait S] [--expect-invalidation] SarTest run [--iterations K] [--duration S] SarTest race [--cycles K] [--up S] [--down MS] [--openers T] [--hold MS] +SarTest control-panel ``` `install` creates the SAR software device node if needed, installs the @@ -143,7 +150,7 @@ Then, elevated: .\Invoke-SarTest.ps1 -PackageDir -ToolsDir -CertPath test.cer ``` -The script runs `run` for 1, 4, 8 and 16 endpoint pairs with three +The script runs the `issue-*` checks, then `run` for 1, 4, 8 and 16 endpoint pairs with three start/stop iterations each, `race` on the largest layout, then a scenario that kills the host while WASAPI clients are streaming and checks that a new host can still create endpoints afterwards. Results land in `sartest-results\`: one `.log` and `.json` per From 95bafbde178b60b4964b21618824ec2bbedc8676 Mon Sep 17 00:00:00 2001 From: Mack Straight Date: Sat, 3 Oct 2026 10:43:23 -0400 Subject: [PATCH 2/2] SarAsio: configure the running inner driver instead of a second one With the host running, the hardware interface's Configure button created and initialized a second instance of the inner ASIO driver just to call controlPanel() on it. ASIO drivers may assume a single instance per process, and FlexASIO crashed on it. The control panel now gets the wrapper's running inner driver and its CLSID, and uses it when it is the selected one. Other selections, and SarAsio without a running driver, still open a temporary instance. Fixes #133. Co-Authored-By: Claude Opus 5.5 --- SarAsio/configui.cpp | 22 ++++++++++++++++++---- SarAsio/configui.h | 12 ++++++++++-- SarAsio/wrapper.cpp | 5 ++++- SarAsio/wrapper.h | 1 + 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/SarAsio/configui.cpp b/SarAsio/configui.cpp index 6803257..3511610 100644 --- a/SarAsio/configui.cpp +++ b/SarAsio/configui.cpp @@ -112,8 +112,11 @@ INT_PTR CALLBACK SimpleDialog::dialogProcStub( return 0; } -EndpointsPropertySheetPage::EndpointsPropertySheetPage(DriverConfig& config) - : _config(config) +EndpointsPropertySheetPage::EndpointsPropertySheetPage( + DriverConfig& config, IASIO *runningDriver, + const std::string& runningDriverClsid) + : _config(config), _runningDriver(runningDriver), + _runningDriverClsid(runningDriverClsid) { pszTemplate = MAKEINTRESOURCE(IDD_CONFIG_ENDPOINTS); @@ -198,6 +201,14 @@ void EndpointsPropertySheetPage::onConfigureHardwareInterface() { for (auto& driver : _drivers) { if (driver.clsid == _config.driverClsid) { + // ASIO drivers expect a single instance per process, and some + // crash when a second one is created next to a running one. Use + // the instance the host already has when it is the selected one. + if (_runningDriver && driver.clsid == _runningDriverClsid) { + _runningDriver->controlPanel(); + break; + } + CComPtr asio; if (SUCCEEDED(driver.open(&asio))) { @@ -856,10 +867,13 @@ void ApplicationConfigDialog::onBrowseClicked() } } -ConfigurationPropertyDialog::ConfigurationPropertyDialog(DriverConfig& config) +ConfigurationPropertyDialog::ConfigurationPropertyDialog( + DriverConfig& config, IASIO *runningDriver, + const std::string& runningDriverClsid) : _originalConfig(config), _newConfig(_originalConfig), - _endpoints(std::make_shared(_newConfig)), + _endpoints(std::make_shared( + _newConfig, runningDriver, runningDriverClsid)), _applications(std::make_shared(_newConfig)) { pszCaption = TEXT("Synchronous Audio Router"); diff --git a/SarAsio/configui.h b/SarAsio/configui.h index 0770ce0..1e07007 100644 --- a/SarAsio/configui.h +++ b/SarAsio/configui.h @@ -69,7 +69,11 @@ struct SimpleDialog struct EndpointsPropertySheetPage: public PropertySheetPage { - EndpointsPropertySheetPage(DriverConfig& config); + // runningDriver is the hardware interface the caller already has open and + // initialized (CLSID runningDriverClsid), if any. + EndpointsPropertySheetPage( + DriverConfig& config, IASIO *runningDriver = nullptr, + const std::string& runningDriverClsid = ""); protected: virtual INT_PTR dialogProc( @@ -87,6 +91,8 @@ struct EndpointsPropertySheetPage: public PropertySheetPage DriverConfig& _config; std::vector _drivers; + IASIO *_runningDriver; + std::string _runningDriverClsid; HWND _hardwareInterfaceDropdown; HWND _hardwareInterfaceConfigButton; HWND _listView; @@ -170,7 +176,9 @@ struct ApplicationConfigDialog: public SimpleDialog struct ConfigurationPropertyDialog: public PropertyDialog { - ConfigurationPropertyDialog(DriverConfig& config); + ConfigurationPropertyDialog( + DriverConfig& config, IASIO *runningDriver = nullptr, + const std::string& runningDriverClsid = ""); DriverConfig newConfig() { return _newConfig; } private: diff --git a/SarAsio/wrapper.cpp b/SarAsio/wrapper.cpp index 41c46b2..92d0853 100644 --- a/SarAsio/wrapper.cpp +++ b/SarAsio/wrapper.cpp @@ -540,7 +540,8 @@ AsioStatus SarAsioWrapper::disposeBuffers() AsioStatus SarAsioWrapper::controlPanel() { LOG(INFO) << "SarAsioWrapper::controlPanel"; - auto sheet = std::make_shared(_config); + auto sheet = std::make_shared( + _config, _innerDriver, _innerDriverClsid); if (sheet->show(_hwnd) > 0) { _config = sheet->newConfig(); @@ -578,6 +579,7 @@ AsioStatus SarAsioWrapper::outputReady() bool SarAsioWrapper::initInnerDriver() { _innerDriver = nullptr; + _innerDriverClsid = ""; for (auto driver : InstalledAsioDrivers()) { if (driver.clsid == _config.driverClsid) { @@ -593,6 +595,7 @@ bool SarAsioWrapper::initInnerDriver() return false; } + _innerDriverClsid = driver.clsid; return true; } } diff --git a/SarAsio/wrapper.h b/SarAsio/wrapper.h index 1e5d284..5cd8312 100644 --- a/SarAsio/wrapper.h +++ b/SarAsio/wrapper.h @@ -104,6 +104,7 @@ struct ATL_NO_VTABLE SarAsioWrapper: std::shared_ptr _sar; std::shared_ptr _castMaster; CComPtr _innerDriver; + std::string _innerDriverClsid; std::vector _virtualInputs; std::vector _virtualOutputs; AsioTickCallback *_userTick;