diff --git a/src/RemSound.App/Program.cs b/src/RemSound.App/Program.cs index 82fed46..3074a8c 100644 --- a/src/RemSound.App/Program.cs +++ b/src/RemSound.App/Program.cs @@ -23,7 +23,8 @@ internal static class Program internal static bool IsServiceInvocation(string[] args) => HasArg(args, ServiceControl.RunVerb) || HasArg(args, ServiceControl.InstallVerb) || HasArg(args, ServiceControl.UninstallVerb) - || HasArg(args, ServiceControl.StartVerb) || HasArg(args, ServiceControl.StopVerb); + || HasArg(args, ServiceControl.StartVerb) || HasArg(args, ServiceControl.StopVerb) + || HasArg(args, ServiceControl.SelfUpdateVerb); // Writes an otherwise-fatal exception to a timestamped crash file in the logs folder, so a // "RemSound just disappeared, no dialog" report (#16) leaves a stack behind to diagnose instead diff --git a/src/RemSound.App/RouterPortMapper.cs b/src/RemSound.App/RouterPortMapper.cs index dc3313a..696326f 100644 --- a/src/RemSound.App/RouterPortMapper.cs +++ b/src/RemSound.App/RouterPortMapper.cs @@ -237,6 +237,10 @@ internal sealed class RouterPortMapper : IDisposable private void OnDeviceFound(object? sender, DeviceEventArgs args) { + // A discovery callback can already be in flight when Stop()/Dispose() unsubscribes. Without this + // guard it would re-open the port map that Dispose just removed, leaving the router forwarding to + // us until the lease expires. Narrow race, cheap check (review sweep). + lock (gate) { if (disposed) return; } try { var found = args.Device; diff --git a/src/RemSound.App/SelfTest.cs b/src/RemSound.App/SelfTest.cs index 478a7e9..606d536 100644 --- a/src/RemSound.App/SelfTest.cs +++ b/src/RemSound.App/SelfTest.cs @@ -67,6 +67,7 @@ internal static class SelfTest RunStep(results, "Heartbeat wire round-trip + ping→pong echo", HeartbeatEcho); RunStep(results, "Profile persistence tripwire (every field wired or declared)", ProfileDriftTripwire); RunStep(results, "Cue variant resolution (dedupe, order, chosen-default fallback)", CueVariantResolution); + RunStep(results, "Updater swap + rollback (a failed update restores exactly)", UpdaterSwapRollback); RunStep(results, "App settings save and reload", SettingsRoundTrip); RunStep(results, "Per-peer shaping DSP", PeerShapingDsp); RunStep(results, "Multi-output fan-out (both lanes)", FanOutToBothOutputs); @@ -962,6 +963,51 @@ internal static class SelfTest return "follower flagged + sentinel shared with the app; service resolves it to the live default render endpoint"; } + /// The updater's back-up-and-swap is the one piece of code that can BRICK an install: a bad + /// rollback leaves a half-swapped folder that won't start. Pins, on real temp folders: the swap + /// replaces + adds exactly the release's files (user files untouched), the backup holds the originals, + /// and RollBack restores the target to its byte-exact pre-swap state including deleting new files. + private static string? UpdaterSwapRollback() + { + var root = Path.Combine(Path.GetTempPath(), "remsound-selftest-swap-" + Guid.NewGuid().ToString("N")); + var source = Path.Combine(root, "source"); + var target = Path.Combine(root, "target"); + var backup = Path.Combine(target, "_update-backup"); + try + { + Directory.CreateDirectory(Path.Combine(source, "sub")); + Directory.CreateDirectory(target); + File.WriteAllText(Path.Combine(source, "a.dll"), "A-NEW"); + File.WriteAllText(Path.Combine(source, "b.dll"), "B-NEW"); + File.WriteAllText(Path.Combine(source, "sub", "c.dll"), "C-NEW"); + File.WriteAllText(Path.Combine(target, "a.dll"), "A-OLD"); + File.WriteAllText(Path.Combine(target, "user.json"), "USER"); // not in the release — must survive everything + + var moved = new List<(string backup, string dest)>(); + var created = new List(); + UpdateApplier.SwapInNewFiles(source, target, backup, moved, created, _ => { }); + + Check(File.ReadAllText(Path.Combine(target, "a.dll")) == "A-NEW" + && File.ReadAllText(Path.Combine(target, "b.dll")) == "B-NEW" + && File.ReadAllText(Path.Combine(target, "sub", "c.dll")) == "C-NEW", + "the swap must land every release file, including subfolders"); + Check(File.ReadAllText(Path.Combine(target, "user.json")) == "USER", "files not in the release must be untouched"); + Check(moved.Count == 1 && created.Count == 2, $"bookkeeping must be exact (moved {moved.Count}, created {created.Count})"); + Check(File.ReadAllText(Path.Combine(backup, "a.dll")) == "A-OLD", "the backup must hold the original bytes"); + + UpdateApplier.RollBack(moved, created, _ => { }); + Check(File.ReadAllText(Path.Combine(target, "a.dll")) == "A-OLD", "rollback must restore the original file bytes"); + Check(!File.Exists(Path.Combine(target, "b.dll")) && !File.Exists(Path.Combine(target, "sub", "c.dll")), + "rollback must DELETE files the failed update newly created"); + Check(File.ReadAllText(Path.Combine(target, "user.json")) == "USER", "user files must survive the rollback too"); + return "swap exact + user files untouched; rollback restores byte-exact incl. deleting new files"; + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp cleanup is best-effort */ } + } + } + /// Cue variant resolution feeds the Preferences sound picker AND which WAV actually plays. /// Wrong resolution = the wrong sound (or a duplicate 'Sound 1' row NVDA can't disambiguate). Pins: /// numbered ordering, the drop-bare-when-numbered-exists dedupe, the 'Sound N' labels, and the @@ -1958,7 +2004,7 @@ internal static class SelfTest foreach (var verb in new[] { ServiceControl.RunVerb, ServiceControl.InstallVerb, ServiceControl.UninstallVerb, - ServiceControl.StartVerb, ServiceControl.StopVerb, + ServiceControl.StartVerb, ServiceControl.StopVerb, ServiceControl.SelfUpdateVerb, }) { Check(Program.IsServiceInvocation(new[] { verb }), $"'{verb}' must be recognised as a service invocation"); @@ -1976,7 +2022,7 @@ internal static class SelfTest if (!loadedBefore) Check(!IsAssemblyLoaded(svcAsm), "deciding a normal launch must not load the Windows-service assembly"); - return "normal launches stay load-safe; all five service verbs recognised (case-insensitive)"; + return "normal launches stay load-safe; all six service verbs recognised (case-insensitive)"; } /// The Service menu's "View service log" opens the newest diagnostic log — the log that says @@ -2323,7 +2369,17 @@ internal static class SelfTest var ranAfter = false; apt.Invoke(() => ranAfter = true); Check(ranAfter, "the apartment must keep working after a work item threw"); - return "runs work on one dedicated STA thread; exceptions propagate; survives a throw"; + + // Bounded invoke: a wedged work item must return false promptly instead of hanging the caller + // (this is what stops a wedged ASIO driver freezing a live driver-switch or the resume path). + using var release = new ManualResetEventSlim(false); + var sw = Stopwatch.StartNew(); + var completed = apt.Invoke(() => release.Wait(3000), timeoutMs: 150); + sw.Stop(); + Check(!completed, "a work item that overruns the timeout must report false, not block"); + Check(sw.ElapsedMilliseconds < 1500, $"the bounded wait must return promptly (took {sw.ElapsedMilliseconds} ms)"); + release.Set(); // free the apartment thread so Dispose joins cleanly + return "one dedicated STA thread; exceptions propagate; survives a throw; bounded invoke times out clean"; } /// The instant capture-on-app-open watcher (AudioSessionStartWatcher) must construct, re-hook diff --git a/src/RemSound.App/ServiceControl.cs b/src/RemSound.App/ServiceControl.cs index 6e645f1..cfea050 100644 --- a/src/RemSound.App/ServiceControl.cs +++ b/src/RemSound.App/ServiceControl.cs @@ -251,6 +251,66 @@ public static class ServiceControl return rc; } + /// Verb for the native self-update helper (see ). Spawned BY the + /// running service as SYSTEM, so no elevation is involved. + public const string SelfUpdateVerb = "--service-selfupdate"; + + /// The self-update worker: stop the service, copy the recorded app-source build into the + /// service's own bin, start the service again — all in managed code. Replaces the old PowerShell + /// restart script: where execution policy is enforced by Group Policy, the script's -ExecutionPolicy + /// Bypass is IGNORED and the self-update silently died, stranding the service on the old build. + /// Native code has no policy to fall foul of. Runs detached (spawned by the service just before the + /// stop kills it); logs every step to the update log so the after-death part is still recorded. + public static int DoSelfUpdate() + { + static void Log(string m) + { + try + { + Directory.CreateDirectory(ServiceStore.Directory); + File.AppendAllText(ServiceStore.UpdateLogPath, $"{DateTime.Now:yyyy-MM-dd HH:mm:ss} selfupdate: {m}\r\n"); + } + catch { /* logging is best-effort */ } + } + try + { + Log($"stopping {ServiceName}"); + try + { + using var sc = new ServiceController(ServiceName); + if (sc.Status != ServiceControllerStatus.Stopped) + { + sc.Stop(); + sc.WaitForStatus(ServiceControllerStatus.Stopped, TimeSpan.FromSeconds(30)); + } + } + catch (Exception ex) { Log($"stop failed ({ex.GetType().Name}: {ex.Message}) — continuing"); } + + var appDir = ServiceStore.LoadAppSourcePath(); + if (!string.IsNullOrEmpty(appDir) && Directory.Exists(appDir)) + { + // CopyProgramTo already excludes every user-state folder — same routine the installer uses. + try { CopyProgramTo(appDir, ServiceStore.BinDirectory); Log("copied the new build into bin"); } + catch (Exception ex) { Log($"COPY FAILED ({ex.GetType().Name}: {ex.Message}) — starting the existing build"); } + } + else + { + Log("no app-source recorded; restarting onto the existing bin"); + } + + try + { + using var sc = new ServiceController(ServiceName); + sc.Start(); + sc.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromSeconds(30)); + Log("service started"); + return 0; + } + catch (Exception ex) { Log($"START FAILED — {ex.GetType().Name}: {ex.Message}"); return 1; } + } + catch (Exception ex) { Log($"FATAL {ex.GetType().Name}: {ex.Message}"); return 2; } + } + /// Restart the service WITHOUT elevation, using the start/stop rights the installer granted /// the installing user (the SDDL ACE from ). Returns true when the /// service ends up Running. No UAC prompt, no elevated helper — so it's safe to run from a background diff --git a/src/RemSound.App/ServiceEntry.cs b/src/RemSound.App/ServiceEntry.cs index 6e4948a..9de580c 100644 --- a/src/RemSound.App/ServiceEntry.cs +++ b/src/RemSound.App/ServiceEntry.cs @@ -38,6 +38,7 @@ internal static class ServiceEntry : Has(args, ServiceControl.UninstallVerb) ? "uninstall" : Has(args, ServiceControl.StartVerb) ? "start" : Has(args, ServiceControl.StopVerb) ? "stop" + : Has(args, ServiceControl.SelfUpdateVerb) ? "selfupdate" : null; if (verb is null) return 0; @@ -54,6 +55,7 @@ internal static class ServiceEntry "uninstall" => ServiceControl.DoUninstall(), "start" => ServiceControl.DoStart(), "stop" => ServiceControl.DoStop(), + "selfupdate" => ServiceControl.DoSelfUpdate(), _ => 0, }; ServiceStore.AppendServiceEvent($"elevated {verb}: finished with code {rc}"); diff --git a/src/RemSound.App/ServiceUpdate.cs b/src/RemSound.App/ServiceUpdate.cs index 34fda2d..e5b9d97 100644 --- a/src/RemSound.App/ServiceUpdate.cs +++ b/src/RemSound.App/ServiceUpdate.cs @@ -51,42 +51,31 @@ internal static class ServiceUpdate public static Version? RunningVersion() => Assembly.GetExecutingAssembly().GetName().Version; - /// Restart the service onto the new binary. Spawns a DETACHED PowerShell (as SYSTEM, inherited - /// from the service) that stops this service — which exits this process — then starts it again, so the - /// SCM launches the freshly-installed exe. The script LOGS its own stop/start outcome to the update log, - /// so even the part that runs after this process is gone (and any failed start) is recorded. Never - /// throws; worst case the service picks up the update on the next reboot. + /// Restart the service onto the new binary. Spawns a DETACHED helper — a copy of the NEW + /// RemSound.exe running the verb, as SYSTEM (inherited + /// from the service) — which stops this service (exiting this process), copies the new build into the + /// service bin, and starts the service again. All managed code: the previous PowerShell restart + /// script silently died on machines where Group Policy enforces execution policy (Bypass is ignored + /// there), stranding the service on the old build. The helper logs every step to the update log, so + /// the part that runs after this process is gone is still recorded. Never throws; worst case the + /// service picks up the update on the next reboot. public static void RestartSelf() { try { + // Run the helper from the NEW build in the app-source folder — it must not execute from the + // bin it's about to overwrite. Falls back to the bin exe when no app source is recorded (the + // helper then just restarts the service without copying, so nothing conflicts). var appDir = ServiceStore.LoadAppSourcePath(); - var binDir = ServiceStore.BinDirectory; - var dir = ServiceStore.Directory; - System.IO.Directory.CreateDirectory(dir); - var script = Path.Combine(dir, "restart.ps1"); - var log = ServiceStore.UpdateLogPath; - var svc = ServiceControl.ServiceName; - // robocopy the new build into bin, minus user-state; exit codes 0-7 are success (8+ = failure). - var copyLine = string.IsNullOrEmpty(appDir) - ? "$rc = 0 # no app-source recorded; restart onto whatever is already in bin" - : $"robocopy \"{appDir}\" \"{binDir}\" /E /XD \"user settings and logs\" logs recordings profiles config /XF \"global config.json\" remsound.config.json /R:2 /W:1 | Out-Null; $rc = $LASTEXITCODE"; - var content = - "$ts = { (Get-Date).ToString('yyyy-MM-dd HH:mm:ss') }\r\n" + - $"Add-Content -LiteralPath '{log}' -Value \"$(& $ts) restarter: stopping {svc}\"\r\n" + - $"Stop-Service -Name {svc} -Force -ErrorAction SilentlyContinue\r\n" + - copyLine + "\r\n" + - $"Add-Content -LiteralPath '{log}' -Value \"$(& $ts) restarter: copied new build (robocopy code $rc)\"\r\n" + - $"if ($rc -ge 8) {{ Add-Content -LiteralPath '{log}' -Value \"$(& $ts) restart: COPY FAILED (code $rc) - starting existing build\" }}\r\n" + - $"try {{ Start-Service -Name {svc} -ErrorAction Stop; $r = 'restart: service started' }} catch {{ $r = 'restart: START FAILED - ' + $_.Exception.Message }}\r\n" + - $"Add-Content -LiteralPath '{log}' -Value \"$(& $ts) $r\"\r\n"; - File.WriteAllText(script, content); + var appExe = string.IsNullOrEmpty(appDir) ? null : Path.Combine(appDir, "RemSound.exe"); + var helperExe = appExe is not null && File.Exists(appExe) ? appExe : ServiceStore.BinExePath; var psi = new ProcessStartInfo { - FileName = "powershell.exe", - Arguments = $"-NonInteractive -WindowStyle Hidden -ExecutionPolicy Bypass -File \"{script}\"", + FileName = helperExe, + Arguments = ServiceControl.SelfUpdateVerb, UseShellExecute = false, CreateNoWindow = true, + WorkingDirectory = Path.GetDirectoryName(helperExe)!, }; Process.Start(psi); } diff --git a/src/RemSound.App/UpdateApplier.cs b/src/RemSound.App/UpdateApplier.cs index 01b4188..5534533 100644 --- a/src/RemSound.App/UpdateApplier.cs +++ b/src/RemSound.App/UpdateApplier.cs @@ -124,8 +124,10 @@ internal static class UpdateApplier /// Rename each existing target file aside into the backup folder, then copy the new /// one in. Throws if a file genuinely can't be replaced after the retry window — the caller - /// rolls back. Only writes files that exist in the new release, so user data is left untouched. - private static void SwapInNewFiles(string source, string target, string backupDir, + /// rolls back. Only writes files that exist in the new release, so user data is left untouched. + /// Internal (was private) so the self-test can pin the swap/rollback contract — a broken rollback + /// is the one failure mode of the updater that bricks an install. + internal static void SwapInNewFiles(string source, string target, string backupDir, List<(string backup, string dest)> moved, List created, Action log) { var srcFull = Path.GetFullPath(source); @@ -154,8 +156,9 @@ internal static class UpdateApplier log($"swap complete: {copiedCount} files in, {moved.Count} replaced, {created.Count} new"); } - /// Restore the old files (and remove the partial new ones) after a failed swap. - private static void RollBack(List<(string backup, string dest)> moved, List created, Action log) + /// Restore the old files (and remove the partial new ones) after a failed swap. + /// Internal for the self-test — see . + internal static void RollBack(List<(string backup, string dest)> moved, List created, Action log) { foreach (var dest in created) { diff --git a/src/RemSound.Sender/AsioApartment.cs b/src/RemSound.Sender/AsioApartment.cs index a0545ff..2efb2a8 100644 --- a/src/RemSound.Sender/AsioApartment.cs +++ b/src/RemSound.Sender/AsioApartment.cs @@ -46,15 +46,23 @@ internal sealed class AsioApartment : IDisposable /// Run on the apartment thread and block until it finishes, /// rethrowing anything it raised. If called from the apartment thread itself, or after Dispose, runs /// inline so a teardown path can never deadlock on itself. - public void Invoke(Action action) + public void Invoke(Action action) => Invoke(action, Timeout.Infinite); + + /// Bounded variant: wait at most for the work to finish. + /// Returns false on timeout — the work is NOT cancelled (it may still complete later on the + /// apartment thread); the caller just stops waiting. Used by the ASIO close path so a driver that + /// wedges on Stop/Dispose can no longer hang a live driver-switch or the resume path — the caller + /// abandons the driver (the old park semantics) and the OS reclaims it at process exit. + public bool Invoke(Action action, int timeoutMs) { - if (shutdown || Thread.CurrentThread == thread) { action(); return; } + if (shutdown || Thread.CurrentThread == thread) { action(); return true; } var item = new WorkItem { Action = action, Done = new ManualResetEventSlim(false) }; lock (queueGate) queue.Enqueue(item); workAvailable.Set(); - item.Done.Wait(); + if (!item.Done.Wait(timeoutMs)) return false; // deliberately NOT disposing Done — the worker's finally will still Set it item.Done.Dispose(); if (item.Error is not null) throw item.Error; + return true; } private void Run() diff --git a/src/RemSound.Sender/AsioCaptureBackend.cs b/src/RemSound.Sender/AsioCaptureBackend.cs index 0a5453a..9cb3b2e 100644 --- a/src/RemSound.Sender/AsioCaptureBackend.cs +++ b/src/RemSound.Sender/AsioCaptureBackend.cs @@ -229,7 +229,12 @@ internal sealed class AsioCaptureBackend : ICaptureBackend // blew past these try/catch blocks with no managed stack). Step-by-step logging still pinpoints // any native call that dies, and the callback is unhooked + drained before stop/dispose so the // close isn't racing a live buffer callback (a common trigger for the crash). - apartment.Invoke(() => + // + // BOUNDED (review sweep): a driver that wedges inside Stop/Dispose used to hang the CALLER + // forever — and live driver-switches and the resume path close on the UI thread. 8 s is + // generous for a healthy close (Ed's Audient releases in ~5 ms); past that we abandon the + // driver (the old park semantics — the OS reclaims it at process exit) and move on. + var closed = apartment.Invoke(() => { onDiagnostic?.Invoke("asio close: unhooking callback"); try { toClose.AudioAvailable -= OnAudioAvailable; } catch { /* ignore */ } @@ -239,7 +244,9 @@ internal sealed class AsioCaptureBackend : ICaptureBackend onDiagnostic?.Invoke("asio close: releasing driver (dispose)"); try { toClose.Dispose(); } catch (Exception ex) { onDiagnostic?.Invoke($"asio close: dispose threw {ex.GetType().Name}: {ex.Message}"); } onDiagnostic?.Invoke("asio close: driver released cleanly"); - }); + }, timeoutMs: 8000); + if (!closed) + onDiagnostic?.Invoke("asio close: TIMED OUT after 8s — abandoning the driver (parked; reclaimed at process exit)"); asio = null; } uptime.Stop();