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