Fix double IRP completion, filter factory error handling, and other bugs from patch review

[eiz-claude]

This is the result of reviewing fGeorjje/SynchronousAudioRouter@dcaf4b1 by @fGeorjje (Paul Schwandes), an AI-generated patch that reportedly stopped BSODs on a 32-channel WDM → X32 setup. Each hunk was checked against the actual code paths. This PR takes the changes that fix real bugs, reimplements one of them differently, and leaves out the rest with reasons below. Paul is credited as co-author on the commit.

Build status. Verified on the eiz-claude fork using the #143 pipeline on top of master: driver (x64), SarAsio (x64, x86), and, with the follow-up CI change that adds them to the usermode job, SarConfigure and SarCtl (x64, x86) all compile with no new warnings. It still needs a retest on the original reporter's setup, since nothing in the original patch clearly explains the BSOD they saw. The filter factory fix is the best candidate.

Taken

• SarWaitHandleQueue double completion. When the output buffer was smaller than one response it completed the IRP itself, and SarIrpDeviceControl completes every non-pending IRP again. Latent, since SarAsio never sends a small buffer, but it is a real bugcheck 0x44.
• SarWaitHandleQueue leak on transfer failure. The break left the remaining items in the local list, leaking pool and process handles.
• SarCreateEndpoint filter factory status. The second KsCreateFilterFactory overwrote the first call's status and both KsFilterFactoryUpdateCacheData calls ran on possibly null factories. Two endpoints with the same ID would hit this. Rewritten to check each call and log which one failed.
• getPhysicalConnection terminator. SymbolicLinkName[len/2]; was a no-op statement in both filter descriptors.
• Pending-endpoint work item flag. Inferring "work item queued" from "pending list empty" is fragile because the work item pops entries one at a time with the mutex dropped. The synchronous client can't currently hit the double-queue case, but the explicit flag is strictly tighter.
• Zero channel count validation in SarCreateEndpoint.
• strcpy_s in getChannelInfo. Channel names of 32+ characters trip the invalid parameter handler and abort the host.
• Notification handle leak in SarClient::stop.
• _innerDriver left set after a failed init(). The wrapper then calls into a driver that never initialized instead of falling back to running without one.
• SUCCEEDED(RegOpenKeyEx(...)) in both tinyasio copies. LSTATUS errors are positive, so it never failed. Cosmetic effect only.
• VT_LPWSTR check before reading the default device path, and a calloc null check for virtual channel buffers.

Taken with a different implementation

• Wrong-process unmap in SarDeleteControlContext. The original patch is right that this function can run in an arbitrary process, for example when a DAW closes its pin and drops the last reference to an orphaned context after the ASIO host exited. Its fix opened a GENERIC_ALL handle to the client process at create time, never checked the result, and held it for the lifetime of the context. This PR instead unmaps the view in SarOrphanControlContext, which runs during IRP_MJ_CLEANUP in the context of the process that mapped it. The driver never touches that view itself; all register access goes through the per-process contexts.
• SarKsPinRtGetBufferCore error paths. The original patch duplicated the cleanup three times inline. This PR factors it into a helper, and also fixes the pre-existing endpoint->owner dereference before the null check in the same function. Note that SarKsPinClose already reclaims the cells on the normal path, so this only matters if the audio engine retries the buffer allocation after a failure.

Not taken: no-ops or unverified claims

• "Corrupted circular LIST_ENTRY pointers." RemoveEntryList(&head) followed by AppendTailList is a correct O(1) list splice. The three sites were rewritten into equivalent loops.
• "IRQL DISPATCH_LEVEL mutex violation in the cancel routine." The routine releases the cancel spinlock first, so it runs at the canceller's IRQL, which is PASSIVE for CancelIoEx and thread exit. Reading FsContext2 instead of the table works but fixes nothing.
• "Use-after-free on DAW exit" via endpoints retaining their owner. Every path that retains an endpoint already retains its context through SarGetEndpointFromIrp, so an endpoint can't outlive its owner.
• "Divide-by-zero in clock registers." The diff touches the position register, and 0/0 there has shipped for years.
• "Surround buffer math" in mux/demux. Buffer size and position are always multiples of the frame stride, so the rewrite is functionally identical for every reachable input.
• try_to_lock in tick() "to stop deadlocking". stop() and tick() never nest, so there was no deadlock. The rewrite also polls the completion port with a syscall on every tick of the real-time thread instead of only on a generation change.
• 5.1/7.1 speaker masks. A behavior change, not a fix. Every stereo endpoint would report STEREO instead of DIRECTOUT, which changes the audio engine's mixing for all users, and it picks the old KSAUDIO_SPEAKER_7POINT1 layout rather than 7POINT1_SURROUND. Endpoints above 8 channels still get DIRECTOUT, so it doesn't help the reporter's case either. Worth a separate discussion if someone wants it.
• Guards KS never exercises (filter/pin close with a null context), E_POINTER checks, regex try/catch, unregister-notification returning success, KdBreakPoint removal (a no-op in free builds), WiX path probing, Spectre mitigation off, and a reindent of SarEndpoint in sar.h.

Not taken: would introduce bugs

• SarPostHandleQueue skipping completion when IoSetCancelRoutine returns null. The cancel routine only completes the IRP if it finds it in the pending list, and PostHandleQueue has already removed it under the spinlock. In that race nobody completes the IRP and the ASIO host's thread hangs forever on exit. The existing "whoever removes the list entry completes it" rule is correct.
• SarFilterMMDeviceQuery/Enum returning STATUS_CALLBACK_BYPASS on BUFFER_TOO_SMALL/OVERFLOW. The configuration manager reports STATUS_SUCCESS to the caller on bypass, so a size probe would succeed with an unfilled buffer, breaking the two-call RegQueryValueEx pattern for application routing.
• CmUnRegisterCallback in SarOrphanControlContext. Letting the next session re-register is reasonable, but the patch nulls filterUser before unregistering, and SarFilterMatchesCurrentProcess reads it without a lock. An in-flight callback would dereference null, and the window is exactly when endpoints are being disabled, which triggers MMDevices registry traffic. Could be done correctly as a follow-up by unregistering first.
• createEndpoints overlapped I/O rewrite. Ignores the return value of GetOverlappedResult, so endpoint creation failures are silently treated as success.

Testing

• Fork CI build on top of Add a verifiable WDK build & driver-packaging CI pipeline #143: driver x64, SarAsio x64/x86, SarConfigure and SarCtl x64/x86
• CI build on this PR
• VM suite with the Add SarTest: headless, hardware-free test harness #147 harness: master bugchecked in SarKsPinRtGetBufferCore in 2 of 2 start-up races and this PR passed; details in the comments. That crash is a separate bug from the reported start-up freeze.
• Retest on the original reporter's 32-channel setup
• Endpoint creation failure path: configure two endpoints with the same ID and confirm the driver logs the error instead of crashing

🤖 Generated with Claude Code

https://claude.ai/code/session_015Ae1KbSGxTuh8F6i85MGxp

[fGeorjje]

Paul is credited as co-author on the commit.

i'm going to respectfully decline this. i just pasted the SAR codebase into gemini and understand about 0.01% of what actually happens

[eiz-claude]

VM testing: a separate NULL-dereference bugcheck on master, fixed by this PR

Ran with the harness from #147 on a disposable Windows 11 25H2 VM with test signing and Driver Verifier (standard flags) on SynchronousAudioRouter.sys. The suite drives SarAsio headlessly on a software ASIO clock, streams a verifiable signal through every endpoint with WASAPI clients, and runs a start-up race that restarts the ASIO host back to back while other threads keep opening streams on its endpoints.

What it found on master. The race bugchecked the master driver: 0x3B SYSTEM_SERVICE_EXCEPTION, access violation at SynchronousAudioRouter!SarKsPinRtGetBufferCore+0x50, reached from SarKsPinRtGetBufferWithNotification via SarIrpDeviceControl in the audio service's svchost.

Why. Master reads endpoint->owner on line 24 of wavert.cpp, before the if (!endpoint) check on line 28. The compiler takes the dereference as proof the pointer is non-NULL and deletes the check: the compiled function never tests SarGetEndpointFromIrp's result, and the faulting instruction is mov r15, [rax+0x60] with rax = 0, where 0x60 is SarEndpoint::owner. The lookup returns NULL when the endpoint has been orphaned by the ASIO host stopping while a client still holds the pin and asks for its buffer.

This PR fixes it. Moving the load after the check makes the check real again, so the handler returns STATUS_NOT_FOUND. With this PR the same suite, including a 30-cycle race, ran without a crash:

	master	master + only this NULL check	this PR
Start-up race, 30 host restarts (full suite)	bugcheck, 2 of 2 runs (cycles 4 and 6), same instruction	passed	passed, 2 of 2 runs
Endpoint matrix, 1 to 16 pairs	passed	not run	passed
Host killed with streams open, then recovery	passed	passed	passed

Master with only this one-line change survives the race, which pins the crash on the missing check.

What this does not show. This is not established as the cause of the reported start-up problem. That report describes Ardour and the audio UI freezing for tens of seconds before the bugcheck; this crash is immediate, with no hang before it. The reporter's minidump should settle it: this bug appears as 0x3B in SarKsPinRtGetBufferCore, while a hang that escalates would more likely end in a watchdog or critical-process bugcheck. The closest thing to the reported freeze the harness has seen is a stall after repeated host restarts: once the race has restarted the host 30 times, IAudioClient::Activate on SAR endpoints blocks for 4 s to 36 s when a new host comes up after a kill, and many streams are released at the same instant. Stopping the host also occasionally takes 6–7 s during the race while a concurrent Initialize blocks for as long. Master with only the NULL-check fix behaves the same way, so this predates the PR and the PR doesn't fix it. A plain kill and restart doesn't trigger it.

Also seen on both builds: SarAsio's format-change broadcast on every endpoint activation invalidates open streams, repeatedly for seconds at 16 endpoint pairs, and mid-stream dropouts hit many endpoints at the same instant.

🤖 Generated with Claude Code

https://claude.ai/code/session_015Ae1KbSGxTuh8F6i85MGxp