From 0f6805736f1b4c4ce552c966c4683780dbccbc0c Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 13 Jul 2026 12:46:03 +0000 Subject: [PATCH 1/4] feat: send WakeRequest to the tunnel on system resume After the machine sleeps and wakes, the tunnel can stay unusable for several minutes because a short, same-network wake often produces no link-change event that would trigger recovery. The tunnel binary (tunnel protocol 1.3) accepts a WakeRequest hint that forces network path re-discovery. Subscribe to SystemEvents.PowerModeChanged and, on resume, send a WakeRequest over the existing tunnel RPC channel when the negotiated protocol version supports it. Failures are logged and never affect the running tunnel. Fixes #172 --- Tests.Vpn.Proto/RpcVersionTest.cs | 12 ++ Tests.Vpn.Service/ManagerTest.cs | 195 +++++++++++++++++++++++++++ Tests.Vpn.Service/packages.lock.json | 6 + Tests.Vpn/SpeakerTest.cs | 30 +++++ Vpn.Proto/RpcVersion.cs | 15 ++- Vpn.Proto/vpn.proto | 10 ++ Vpn.Service/Manager.cs | 53 ++++++++ Vpn.Service/Program.cs | 1 + Vpn.Service/SystemResumeMonitor.cs | 46 +++++++ Vpn.Service/TunnelSupervisor.cs | 8 ++ Vpn.Service/Vpn.Service.csproj | 1 + Vpn.Service/packages.lock.json | 6 + Vpn/Speaker.cs | 10 +- 13 files changed, 391 insertions(+), 2 deletions(-) create mode 100644 Tests.Vpn.Service/ManagerTest.cs create mode 100644 Vpn.Service/SystemResumeMonitor.cs diff --git a/Tests.Vpn.Proto/RpcVersionTest.cs b/Tests.Vpn.Proto/RpcVersionTest.cs index 3f6e8f2..85d6aec 100644 --- a/Tests.Vpn.Proto/RpcVersionTest.cs +++ b/Tests.Vpn.Proto/RpcVersionTest.cs @@ -40,6 +40,18 @@ public void IsCompatibleWith() // 2.1 && 1.1 => null IsCompatibleWithBothWays(twoOne, new RpcVersion(1, 1), Is.Null); } + + [Test(Description = "Check feature support against the version that introduced the feature")] + public void SupportsFeature() + { + var featureVersion = new RpcVersion(1, 3); + Assert.That(new RpcVersion(1, 3).SupportsFeature(featureVersion), Is.True); + Assert.That(new RpcVersion(1, 4).SupportsFeature(featureVersion), Is.True); + Assert.That(new RpcVersion(1, 2).SupportsFeature(featureVersion), Is.False); + // Major versions must match exactly. + Assert.That(new RpcVersion(2, 3).SupportsFeature(featureVersion), Is.False); + Assert.That(new RpcVersion(2, 4).SupportsFeature(featureVersion), Is.False); + } } [TestFixture] diff --git a/Tests.Vpn.Service/ManagerTest.cs b/Tests.Vpn.Service/ManagerTest.cs new file mode 100644 index 0000000..e7a4691 --- /dev/null +++ b/Tests.Vpn.Service/ManagerTest.cs @@ -0,0 +1,195 @@ +using Coder.Desktop.Vpn; +using Coder.Desktop.Vpn.Proto; +using Coder.Desktop.Vpn.Service; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace Coder.Desktop.Tests.Vpn.Service; + +#region Fakes + +internal class FakeTunnelSupervisor : ITunnelSupervisor +{ + public RpcVersion? NegotiatedVersion { get; set; } + + public List SentRequests { get; } = []; + public TunnelMessage? Reply { get; set; } + public Exception? SendException { get; set; } + + public Task StartAsync(string binPath, Speaker.OnReceiveDelegate messageHandler, + Speaker.OnErrorDelegate errorHandler, CancellationToken ct = default) + { + throw new NotImplementedException(); + } + + public Task StopAsync(CancellationToken ct = default) + { + return Task.CompletedTask; + } + + public Task SendMessage(ManagerMessage message, CancellationToken ct = default) + { + throw new NotImplementedException(); + } + + public ValueTask SendRequestAwaitReply(ManagerMessage message, CancellationToken ct = default) + { + SentRequests.Add(message); + if (SendException != null) throw SendException; + if (Reply == null) throw new InvalidOperationException("No reply configured on FakeTunnelSupervisor"); + return ValueTask.FromResult(Reply); + } + + public ValueTask DisposeAsync() + { + return ValueTask.CompletedTask; + } +} + +internal class FakeManagerRpc : IManagerRpc +{ +#pragma warning disable CS0067 // The event is only subscribed to by Manager, never raised in these tests. + public event IManagerRpc.OnReceiveHandler? OnReceive; +#pragma warning restore CS0067 + + public Task StopAsync(CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + + public Task ExecuteAsync(CancellationToken stoppingToken) + { + return Task.CompletedTask; + } + + public Task BroadcastAsync(ServiceMessage message, CancellationToken ct = default) + { + return Task.CompletedTask; + } + + public ValueTask DisposeAsync() + { + return ValueTask.CompletedTask; + } +} + +internal class FakeDownloader : IDownloader +{ + public Task StartDownloadAsync(HttpRequestMessage req, string destinationPath, + IDownloadValidator validator, CancellationToken ct = default) + { + throw new NotImplementedException(); + } +} + +internal class FakeTelemetryEnricher : ITelemetryEnricher +{ + public StartRequest EnrichStartRequest(StartRequest original) + { + return original; + } +} + +#endregion + +[TestFixture] +public class ManagerTest +{ + private static Manager NewManager(ITunnelSupervisor tunnelSupervisor) + { + return new Manager(Options.Create(new ManagerConfig()), NullLogger.Instance, new FakeDownloader(), + tunnelSupervisor, new FakeManagerRpc(), new FakeTelemetryEnricher()); + } + + [Test(Description = "Send a wake request to the tunnel on system resume")] + [CancelAfter(30_000)] + public async Task HandleSystemResumeSendsWakeRequest(CancellationToken ct) + { + var tunnelSupervisor = new FakeTunnelSupervisor + { + NegotiatedVersion = new RpcVersion(1, 3), + Reply = new TunnelMessage + { + Wake = new WakeResponse + { + Success = true, + }, + }, + }; + using var manager = NewManager(tunnelSupervisor); + + await manager.HandleSystemResume(ct); + + Assert.That(tunnelSupervisor.SentRequests, Has.Count.EqualTo(1)); + Assert.That(tunnelSupervisor.SentRequests[0].MsgCase, Is.EqualTo(ManagerMessage.MsgOneofCase.Wake)); + } + + [Test(Description = "Skip the wake request when the tunnel is not running")] + [CancelAfter(30_000)] + public async Task HandleSystemResumeSkipsWhenTunnelNotRunning(CancellationToken ct) + { + var tunnelSupervisor = new FakeTunnelSupervisor + { + NegotiatedVersion = null, + }; + using var manager = NewManager(tunnelSupervisor); + + await manager.HandleSystemResume(ct); + + Assert.That(tunnelSupervisor.SentRequests, Is.Empty); + } + + [Test(Description = "Skip the wake request when the negotiated version does not support it")] + [CancelAfter(30_000)] + public async Task HandleSystemResumeSkipsWhenVersionTooOld(CancellationToken ct) + { + var tunnelSupervisor = new FakeTunnelSupervisor + { + NegotiatedVersion = new RpcVersion(1, 2), + }; + using var manager = NewManager(tunnelSupervisor); + + await manager.HandleSystemResume(ct); + + Assert.That(tunnelSupervisor.SentRequests, Is.Empty); + } + + [Test(Description = "Ignore an unexpected reply message type to a wake request")] + [CancelAfter(30_000)] + public async Task HandleSystemResumeIgnoresUnexpectedReply(CancellationToken ct) + { + var tunnelSupervisor = new FakeTunnelSupervisor + { + NegotiatedVersion = new RpcVersion(1, 3), + Reply = new TunnelMessage + { + Stop = new StopResponse(), + }, + }; + using var manager = NewManager(tunnelSupervisor); + + // HandleSystemResume must not throw when the tunnel replies with an + // unexpected message type. + await manager.HandleSystemResume(ct); + + Assert.That(tunnelSupervisor.SentRequests, Has.Count.EqualTo(1)); + } + + [Test(Description = "Ignore a wake request send failure without affecting the tunnel")] + [CancelAfter(30_000)] + public async Task HandleSystemResumeIgnoresSendFailure(CancellationToken ct) + { + var tunnelSupervisor = new FakeTunnelSupervisor + { + NegotiatedVersion = new RpcVersion(1, 3), + SendException = new InvalidOperationException("TunnelSupervisor is not running"), + }; + using var manager = NewManager(tunnelSupervisor); + + // HandleSystemResume must not throw when sending the wake request + // fails. + await manager.HandleSystemResume(ct); + + Assert.That(tunnelSupervisor.SentRequests, Has.Count.EqualTo(1)); + } +} diff --git a/Tests.Vpn.Service/packages.lock.json b/Tests.Vpn.Service/packages.lock.json index 08a9b56..e6d0d5a 100644 --- a/Tests.Vpn.Service/packages.lock.json +++ b/Tests.Vpn.Service/packages.lock.json @@ -379,6 +379,11 @@ "Newtonsoft.Json": "13.0.1" } }, + "Microsoft.Win32.SystemEvents": { + "type": "Transitive", + "resolved": "9.0.4", + "contentHash": "kHgtAkXhNEP8oGuAVe3Q5admxsdMlSdWE2rXcA9FfeGDZJQawPccmZgnOswgW3ugUPSJt7VH+TMQPz65mnhGSQ==" + }, "Newtonsoft.Json": { "type": "Transitive", "resolved": "13.0.1", @@ -513,6 +518,7 @@ "Microsoft.Extensions.Hosting.WindowsServices": "[9.0.4, )", "Microsoft.Extensions.Options.DataAnnotations": "[9.0.4, )", "Microsoft.Security.Extensions": "[1.3.0, )", + "Microsoft.Win32.SystemEvents": "[9.0.4, )", "Semver": "[3.0.0, )", "Serilog.Extensions.Hosting": "[9.0.0, )", "Serilog.Settings.Configuration": "[9.0.0, )", diff --git a/Tests.Vpn/SpeakerTest.cs b/Tests.Vpn/SpeakerTest.cs index 51950f7..6f38983 100644 --- a/Tests.Vpn/SpeakerTest.cs +++ b/Tests.Vpn/SpeakerTest.cs @@ -163,6 +163,36 @@ await message.SendReply(new TunnelMessage Assert.That(reply.Message.Start.Success, Is.True); } + [Test(Description = "Negotiated version is exposed after a successful handshake")] + [CancelAfter(30_000)] + public async Task NegotiatedVersion(CancellationToken ct) + { + var (stream1, stream2) = BidirectionalPipe.NewInMemory(); + await using var speaker1 = new Speaker(stream1); + await using var speaker2 = new Speaker(stream2); + + Assert.That(speaker1.NegotiatedVersion, Is.Null); + Assert.That(speaker2.NegotiatedVersion, Is.Null); + + await Task.WhenAll(speaker1.StartAsync(ct), speaker2.StartAsync(ct)); + + Assert.That(speaker1.NegotiatedVersion, Is.EqualTo(RpcVersion.Current)); + Assert.That(speaker2.NegotiatedVersion, Is.EqualTo(RpcVersion.Current)); + } + + [Test(Description = "Negotiated version is the lower version of an older peer")] + [CancelAfter(30_000)] + public async Task NegotiatedVersionOlderPeer(CancellationToken ct) + { + var (stream1, stream2) = BidirectionalPipe.NewInMemory(); + await using var speaker1 = new Speaker(stream1); + + await stream2.WriteAsync(Encoding.UTF8.GetBytes("codervpn tunnel 1.2\n"), ct); + await speaker1.StartAsync(ct); + + Assert.That(speaker1.NegotiatedVersion, Is.EqualTo(new RpcVersion(1, 2))); + } + [Test(Description = "Encounter a write error during handshake")] [CancelAfter(30_000)] public async Task WriteError(CancellationToken ct) diff --git a/Vpn.Proto/RpcVersion.cs b/Vpn.Proto/RpcVersion.cs index 641a10d..d7b74ac 100644 --- a/Vpn.Proto/RpcVersion.cs +++ b/Vpn.Proto/RpcVersion.cs @@ -5,7 +5,8 @@ namespace Coder.Desktop.Vpn.Proto; /// public class RpcVersion { - public static readonly RpcVersion Current = new(1, 1); + // 1.3 adds WakeRequest and WakeResponse. + public static readonly RpcVersion Current = new(1, 3); public ulong Major { get; } public ulong Minor { get; } @@ -62,6 +63,18 @@ public override string ToString() return Minor < other.Minor ? this : other; } + /// + /// Returns true if a peer that negotiated this version supports a feature introduced in the given version. + /// The major versions must match exactly, and this version's minor version must be at least the feature + /// version's minor version. + /// + /// Version that introduced the feature + /// Whether the feature is supported + public bool SupportsFeature(RpcVersion featureVersion) + { + return Major == featureVersion.Major && Minor >= featureVersion.Minor; + } + #region RpcVersion Equality public static bool operator ==(RpcVersion a, RpcVersion b) diff --git a/Vpn.Proto/vpn.proto b/Vpn.Proto/vpn.proto index 11a481c..84287b5 100644 --- a/Vpn.Proto/vpn.proto +++ b/Vpn.Proto/vpn.proto @@ -30,6 +30,7 @@ message ManagerMessage { NetworkSettingsResponse network_settings = 3; StartRequest start = 4; StopRequest stop = 5; + WakeRequest wake = 6; } } @@ -42,6 +43,7 @@ message TunnelMessage { NetworkSettingsRequest network_settings = 4; StartResponse start = 5; StopResponse stop = 6; + WakeResponse wake = 7; } } @@ -290,3 +292,11 @@ message Status { // be populated. PeerUpdate peer_update = 3; } + +// WakeRequest is sent by the manager after the system wakes from sleep. The +// tunnel uses this as a hint to rediscover network paths. +message WakeRequest {} + +message WakeResponse { + bool success = 1; +} diff --git a/Vpn.Service/Manager.cs b/Vpn.Service/Manager.cs index 027a882..65d8361 100644 --- a/Vpn.Service/Manager.cs +++ b/Vpn.Service/Manager.cs @@ -19,6 +19,8 @@ public enum TunnelStatus public interface IManager : IDisposable { public Task StopAsync(CancellationToken ct = default); + + public Task HandleSystemResume(CancellationToken ct = default); } /// @@ -26,6 +28,11 @@ public interface IManager : IDisposable /// public class Manager : IManager { + // WakeMinimumTunnelRpcVersion is the lowest RPC version negotiated with + // the tunnel that supports WakeRequest messages. + private static readonly RpcVersion WakeMinimumTunnelRpcVersion = new(1, 3); + private static readonly TimeSpan WakeReplyTimeout = TimeSpan.FromSeconds(5); + private readonly ManagerConfig _config; private readonly IDownloader _downloader; private readonly ILogger _logger; @@ -70,6 +77,52 @@ public async Task StopAsync(CancellationToken ct = default) await BroadcastStatus(null, ct); } + /// + /// Sends a WakeRequest to the tunnel so it re-discovers network paths. After the system resumes from sleep, + /// the tunnel can be unusable for several minutes because a short sleep on the same network often produces no + /// link-change event that would otherwise trigger recovery. The request is a hint only; failures are logged + /// and never affect the running tunnel. + /// + /// Cancellation token + public async Task HandleSystemResume(CancellationToken ct = default) + { + var version = _tunnelSupervisor.NegotiatedVersion; + if (version is null) + { + _logger.LogDebug("Skipping wake request, tunnel is not running"); + return; + } + + if (!version.SupportsFeature(WakeMinimumTunnelRpcVersion)) + { + _logger.LogDebug( + "Skipping wake request, negotiated tunnel RPC version {Version} is older than {MinimumVersion}", + version, WakeMinimumTunnelRpcVersion); + return; + } + + try + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); + cts.CancelAfter(WakeReplyTimeout); + var reply = await _tunnelSupervisor.SendRequestAwaitReply(new ManagerMessage + { + Wake = new WakeRequest(), + }, cts.Token); + if (reply.MsgCase != TunnelMessage.MsgOneofCase.Wake) + _logger.LogWarning("Tunnel replied to wake request with unexpected message type {MessageType}", + reply.MsgCase); + else if (!reply.Wake.Success) + _logger.LogWarning("Tunnel failed to handle wake request"); + else + _logger.LogDebug("Tunnel handled wake request successfully"); + } + catch (Exception e) + { + _logger.LogWarning(e, "Failed to send wake request to tunnel"); + } + } + /// /// Processes a message sent from a Client to the ManagerRpcService over the codervpn RPC protocol. /// diff --git a/Vpn.Service/Program.cs b/Vpn.Service/Program.cs index 094875d..6fd12ae 100644 --- a/Vpn.Service/Program.cs +++ b/Vpn.Service/Program.cs @@ -85,6 +85,7 @@ private static async Task BuildAndRun(string[] args) // Services builder.Services.AddHostedService(); builder.Services.AddHostedService(); + builder.Services.AddHostedService(); // Either run as a Windows service or a console application if (!Environment.UserInteractive) diff --git a/Vpn.Service/SystemResumeMonitor.cs b/Vpn.Service/SystemResumeMonitor.cs new file mode 100644 index 0000000..89907b1 --- /dev/null +++ b/Vpn.Service/SystemResumeMonitor.cs @@ -0,0 +1,46 @@ +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Win32; + +namespace Coder.Desktop.Vpn.Service; + +/// +/// Watches for the system resuming from sleep and notifies the manager so it can prompt the tunnel to re-discover +/// network paths. +/// +public class SystemResumeMonitor : IHostedService +{ + private readonly ILogger _logger; + private readonly IManager _manager; + + // ReSharper disable once ConvertToPrimaryConstructor + public SystemResumeMonitor(ILogger logger, IManager manager) + { + _logger = logger; + _manager = manager; + } + + public Task StartAsync(CancellationToken ct) + { + // SystemEvents delivers power notifications on a dedicated broadcast + // thread, so no message pump is required in this process. + SystemEvents.PowerModeChanged += OnPowerModeChanged; + return Task.CompletedTask; + } + + public Task StopAsync(CancellationToken ct) + { + SystemEvents.PowerModeChanged -= OnPowerModeChanged; + return Task.CompletedTask; + } + + private void OnPowerModeChanged(object sender, PowerModeChangedEventArgs e) + { + if (e.Mode != PowerModes.Resume) return; + _logger.LogInformation("System resumed from sleep, notifying manager"); + // Handle the resume in the background to avoid blocking the shared + // SystemEvents broadcast thread. HandleSystemResume logs and swallows + // all send failures. + _ = Task.Run(() => _manager.HandleSystemResume()); + } +} diff --git a/Vpn.Service/TunnelSupervisor.cs b/Vpn.Service/TunnelSupervisor.cs index 7dd6738..ef414d8 100644 --- a/Vpn.Service/TunnelSupervisor.cs +++ b/Vpn.Service/TunnelSupervisor.cs @@ -10,6 +10,12 @@ namespace Coder.Desktop.Vpn.Service; public interface ITunnelSupervisor : IAsyncDisposable { + /// + /// The RPC version negotiated with the currently running tunnel. Null if the tunnel is not running or its + /// handshake has not completed yet. + /// + public RpcVersion? NegotiatedVersion { get; } + /// /// Starts the tunnel subprocess with the given executable path. If the subprocess is already running, this method will /// kill it first. @@ -64,6 +70,8 @@ public class TunnelSupervisor : ITunnelSupervisor private Speaker? _speaker; private Process? _subprocess; + public RpcVersion? NegotiatedVersion => _speaker?.NegotiatedVersion; + // ReSharper disable once ConvertToPrimaryConstructor public TunnelSupervisor(ILogger logger) { diff --git a/Vpn.Service/Vpn.Service.csproj b/Vpn.Service/Vpn.Service.csproj index aaed3cc..05a0ef6 100644 --- a/Vpn.Service/Vpn.Service.csproj +++ b/Vpn.Service/Vpn.Service.csproj @@ -29,6 +29,7 @@ + diff --git a/Vpn.Service/packages.lock.json b/Vpn.Service/packages.lock.json index 09c7b76..7e541d8 100644 --- a/Vpn.Service/packages.lock.json +++ b/Vpn.Service/packages.lock.json @@ -59,6 +59,12 @@ "resolved": "1.3.0", "contentHash": "xK8WFEo5WMUE8DI8W+GjhRwpVcPrxc4DyTjfxh39+yOyhAtC5TBHDlFEJks5toNZHsUeUuiWELIX25oTWOKPBw==" }, + "Microsoft.Win32.SystemEvents": { + "type": "Direct", + "requested": "[9.0.4, )", + "resolved": "9.0.4", + "contentHash": "kHgtAkXhNEP8oGuAVe3Q5admxsdMlSdWE2rXcA9FfeGDZJQawPccmZgnOswgW3ugUPSJt7VH+TMQPz65mnhGSQ==" + }, "Semver": { "type": "Direct", "requested": "[3.0.0, )", diff --git a/Vpn/Speaker.cs b/Vpn/Speaker.cs index 37ec554..7c6b09f 100644 --- a/Vpn/Speaker.cs +++ b/Vpn/Speaker.cs @@ -83,6 +83,12 @@ public class Speaker : IAsyncDisposable /// public event OnReceiveDelegate? Receive; + /// + /// The RPC version negotiated with the peer. Null until the handshake performed by StartAsync has + /// completed successfully. + /// + public RpcVersion? NegotiatedVersion { get; private set; } + private readonly Stream _conn; // _cts is cancelled when Dispose is called and will cause all ongoing I/O @@ -157,8 +163,10 @@ private async Task PerformHandshake(CancellationToken ct = default) if (header.Role != expectedRole) throw new ArgumentException($"Expected peer role '{expectedRole}' but got '{header.Role}'"); - if (header.VersionList.IsCompatibleWith(RpcVersionList.Current) is null) + var negotiatedVersion = header.VersionList.IsCompatibleWith(RpcVersionList.Current); + if (negotiatedVersion is null) throw new RpcVersionCompatibilityException(RpcVersionList.Current, header.VersionList); + NegotiatedVersion = negotiatedVersion; } private async Task WriteHeader(CancellationToken ct = default) From b0e58f84456176e1bf1f08a72cc3146b204c0755 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 14 Jul 2026 21:42:48 +0000 Subject: [PATCH 2/4] refactor: simplify wake request handling and tests - Sync WakeResponse with upstream (success field was removed) - Convert SystemResumeMonitor to a standard event-based class wired into Manager, instead of a hosted service calling into IManager - Trim comments and condense tests --- Tests.Vpn.Proto/RpcVersionTest.cs | 2 - Tests.Vpn.Service/ManagerTest.cs | 128 +++++++++++------------------ Tests.Vpn/SpeakerTest.cs | 15 ---- Vpn.Proto/RpcVersion.cs | 7 +- Vpn.Proto/vpn.proto | 4 +- Vpn.Service/Manager.cs | 45 +++++----- Vpn.Service/Program.cs | 2 +- Vpn.Service/SystemResumeMonitor.cs | 40 +++------ Vpn.Service/TunnelSupervisor.cs | 3 +- Vpn/Speaker.cs | 3 +- 10 files changed, 87 insertions(+), 162 deletions(-) diff --git a/Tests.Vpn.Proto/RpcVersionTest.cs b/Tests.Vpn.Proto/RpcVersionTest.cs index 85d6aec..dd1514e 100644 --- a/Tests.Vpn.Proto/RpcVersionTest.cs +++ b/Tests.Vpn.Proto/RpcVersionTest.cs @@ -48,9 +48,7 @@ public void SupportsFeature() Assert.That(new RpcVersion(1, 3).SupportsFeature(featureVersion), Is.True); Assert.That(new RpcVersion(1, 4).SupportsFeature(featureVersion), Is.True); Assert.That(new RpcVersion(1, 2).SupportsFeature(featureVersion), Is.False); - // Major versions must match exactly. Assert.That(new RpcVersion(2, 3).SupportsFeature(featureVersion), Is.False); - Assert.That(new RpcVersion(2, 4).SupportsFeature(featureVersion), Is.False); } } diff --git a/Tests.Vpn.Service/ManagerTest.cs b/Tests.Vpn.Service/ManagerTest.cs index e7a4691..06c23fc 100644 --- a/Tests.Vpn.Service/ManagerTest.cs +++ b/Tests.Vpn.Service/ManagerTest.cs @@ -11,10 +11,17 @@ namespace Coder.Desktop.Tests.Vpn.Service; internal class FakeTunnelSupervisor : ITunnelSupervisor { public RpcVersion? NegotiatedVersion { get; set; } - - public List SentRequests { get; } = []; - public TunnelMessage? Reply { get; set; } public Exception? SendException { get; set; } + public List SentRequests { get; } = []; + public TaskCompletionSource Sent { get; } = new(); + + public ValueTask SendRequestAwaitReply(ManagerMessage message, CancellationToken ct = default) + { + SentRequests.Add(message); + Sent.TrySetResult(); + if (SendException != null) throw SendException; + return ValueTask.FromResult(new TunnelMessage { Wake = new WakeResponse() }); + } public Task StartAsync(string binPath, Speaker.OnReceiveDelegate messageHandler, Speaker.OnErrorDelegate errorHandler, CancellationToken ct = default) @@ -32,23 +39,29 @@ public Task SendMessage(ManagerMessage message, CancellationToken ct = default) throw new NotImplementedException(); } - public ValueTask SendRequestAwaitReply(ManagerMessage message, CancellationToken ct = default) + public ValueTask DisposeAsync() { - SentRequests.Add(message); - if (SendException != null) throw SendException; - if (Reply == null) throw new InvalidOperationException("No reply configured on FakeTunnelSupervisor"); - return ValueTask.FromResult(Reply); + return ValueTask.CompletedTask; } +} - public ValueTask DisposeAsync() +internal class FakeSystemResumeMonitor : ISystemResumeMonitor +{ + public event EventHandler? Resumed; + + public void Raise() + { + Resumed?.Invoke(this, EventArgs.Empty); + } + + public void Dispose() { - return ValueTask.CompletedTask; } } internal class FakeManagerRpc : IManagerRpc { -#pragma warning disable CS0067 // The event is only subscribed to by Manager, never raised in these tests. +#pragma warning disable CS0067 // Never raised in tests. public event IManagerRpc.OnReceiveHandler? OnReceive; #pragma warning restore CS0067 @@ -95,101 +108,56 @@ public StartRequest EnrichStartRequest(StartRequest original) [TestFixture] public class ManagerTest { - private static Manager NewManager(ITunnelSupervisor tunnelSupervisor) + private static Manager NewManager(ITunnelSupervisor tunnelSupervisor, ISystemResumeMonitor? resumeMonitor = null) { return new Manager(Options.Create(new ManagerConfig()), NullLogger.Instance, new FakeDownloader(), - tunnelSupervisor, new FakeManagerRpc(), new FakeTelemetryEnricher()); + tunnelSupervisor, new FakeManagerRpc(), new FakeTelemetryEnricher(), + resumeMonitor ?? new FakeSystemResumeMonitor()); } [Test(Description = "Send a wake request to the tunnel on system resume")] [CancelAfter(30_000)] - public async Task HandleSystemResumeSendsWakeRequest(CancellationToken ct) + public async Task SendsWakeRequestOnResume(CancellationToken ct) { - var tunnelSupervisor = new FakeTunnelSupervisor - { - NegotiatedVersion = new RpcVersion(1, 3), - Reply = new TunnelMessage - { - Wake = new WakeResponse - { - Success = true, - }, - }, - }; - using var manager = NewManager(tunnelSupervisor); + var supervisor = new FakeTunnelSupervisor { NegotiatedVersion = new RpcVersion(1, 3) }; + var monitor = new FakeSystemResumeMonitor(); + using var manager = NewManager(supervisor, monitor); - await manager.HandleSystemResume(ct); + monitor.Raise(); - Assert.That(tunnelSupervisor.SentRequests, Has.Count.EqualTo(1)); - Assert.That(tunnelSupervisor.SentRequests[0].MsgCase, Is.EqualTo(ManagerMessage.MsgOneofCase.Wake)); + await supervisor.Sent.Task.WaitAsync(ct); + Assert.That(supervisor.SentRequests[0].MsgCase, Is.EqualTo(ManagerMessage.MsgOneofCase.Wake)); } - [Test(Description = "Skip the wake request when the tunnel is not running")] + [TestCase(null, Description = "Tunnel not running")] + [TestCase("1.2", Description = "Version does not support wake")] [CancelAfter(30_000)] - public async Task HandleSystemResumeSkipsWhenTunnelNotRunning(CancellationToken ct) + public async Task SkipsWakeRequestWhenUnsupported(string? version, CancellationToken ct) { - var tunnelSupervisor = new FakeTunnelSupervisor + var supervisor = new FakeTunnelSupervisor { - NegotiatedVersion = null, - }; - using var manager = NewManager(tunnelSupervisor); - - await manager.HandleSystemResume(ct); - - Assert.That(tunnelSupervisor.SentRequests, Is.Empty); - } - - [Test(Description = "Skip the wake request when the negotiated version does not support it")] - [CancelAfter(30_000)] - public async Task HandleSystemResumeSkipsWhenVersionTooOld(CancellationToken ct) - { - var tunnelSupervisor = new FakeTunnelSupervisor - { - NegotiatedVersion = new RpcVersion(1, 2), - }; - using var manager = NewManager(tunnelSupervisor); - - await manager.HandleSystemResume(ct); - - Assert.That(tunnelSupervisor.SentRequests, Is.Empty); - } - - [Test(Description = "Ignore an unexpected reply message type to a wake request")] - [CancelAfter(30_000)] - public async Task HandleSystemResumeIgnoresUnexpectedReply(CancellationToken ct) - { - var tunnelSupervisor = new FakeTunnelSupervisor - { - NegotiatedVersion = new RpcVersion(1, 3), - Reply = new TunnelMessage - { - Stop = new StopResponse(), - }, + NegotiatedVersion = version == null ? null : RpcVersion.Parse(version), }; - using var manager = NewManager(tunnelSupervisor); + using var manager = NewManager(supervisor); - // HandleSystemResume must not throw when the tunnel replies with an - // unexpected message type. - await manager.HandleSystemResume(ct); + await manager.SendWakeRequest(ct); - Assert.That(tunnelSupervisor.SentRequests, Has.Count.EqualTo(1)); + Assert.That(supervisor.SentRequests, Is.Empty); } - [Test(Description = "Ignore a wake request send failure without affecting the tunnel")] + [Test(Description = "Wake request failures are swallowed")] [CancelAfter(30_000)] - public async Task HandleSystemResumeIgnoresSendFailure(CancellationToken ct) + public async Task IgnoresWakeRequestFailure(CancellationToken ct) { - var tunnelSupervisor = new FakeTunnelSupervisor + var supervisor = new FakeTunnelSupervisor { NegotiatedVersion = new RpcVersion(1, 3), SendException = new InvalidOperationException("TunnelSupervisor is not running"), }; - using var manager = NewManager(tunnelSupervisor); + using var manager = NewManager(supervisor); - // HandleSystemResume must not throw when sending the wake request - // fails. - await manager.HandleSystemResume(ct); + await manager.SendWakeRequest(ct); - Assert.That(tunnelSupervisor.SentRequests, Has.Count.EqualTo(1)); + Assert.That(supervisor.SentRequests, Has.Count.EqualTo(1)); } } diff --git a/Tests.Vpn/SpeakerTest.cs b/Tests.Vpn/SpeakerTest.cs index 6f38983..04966c7 100644 --- a/Tests.Vpn/SpeakerTest.cs +++ b/Tests.Vpn/SpeakerTest.cs @@ -172,27 +172,12 @@ public async Task NegotiatedVersion(CancellationToken ct) await using var speaker2 = new Speaker(stream2); Assert.That(speaker1.NegotiatedVersion, Is.Null); - Assert.That(speaker2.NegotiatedVersion, Is.Null); - await Task.WhenAll(speaker1.StartAsync(ct), speaker2.StartAsync(ct)); Assert.That(speaker1.NegotiatedVersion, Is.EqualTo(RpcVersion.Current)); Assert.That(speaker2.NegotiatedVersion, Is.EqualTo(RpcVersion.Current)); } - [Test(Description = "Negotiated version is the lower version of an older peer")] - [CancelAfter(30_000)] - public async Task NegotiatedVersionOlderPeer(CancellationToken ct) - { - var (stream1, stream2) = BidirectionalPipe.NewInMemory(); - await using var speaker1 = new Speaker(stream1); - - await stream2.WriteAsync(Encoding.UTF8.GetBytes("codervpn tunnel 1.2\n"), ct); - await speaker1.StartAsync(ct); - - Assert.That(speaker1.NegotiatedVersion, Is.EqualTo(new RpcVersion(1, 2))); - } - [Test(Description = "Encounter a write error during handshake")] [CancelAfter(30_000)] public async Task WriteError(CancellationToken ct) diff --git a/Vpn.Proto/RpcVersion.cs b/Vpn.Proto/RpcVersion.cs index d7b74ac..0a0c797 100644 --- a/Vpn.Proto/RpcVersion.cs +++ b/Vpn.Proto/RpcVersion.cs @@ -64,12 +64,9 @@ public override string ToString() } /// - /// Returns true if a peer that negotiated this version supports a feature introduced in the given version. - /// The major versions must match exactly, and this version's minor version must be at least the feature - /// version's minor version. + /// Returns true if a peer with this version supports a feature introduced in the given version. Major + /// versions must match exactly. /// - /// Version that introduced the feature - /// Whether the feature is supported public bool SupportsFeature(RpcVersion featureVersion) { return Major == featureVersion.Major && Minor >= featureVersion.Minor; diff --git a/Vpn.Proto/vpn.proto b/Vpn.Proto/vpn.proto index 84287b5..bd3a8e5 100644 --- a/Vpn.Proto/vpn.proto +++ b/Vpn.Proto/vpn.proto @@ -297,6 +297,4 @@ message Status { // tunnel uses this as a hint to rediscover network paths. message WakeRequest {} -message WakeResponse { - bool success = 1; -} +message WakeResponse {} diff --git a/Vpn.Service/Manager.cs b/Vpn.Service/Manager.cs index 65d8361..1d8cb44 100644 --- a/Vpn.Service/Manager.cs +++ b/Vpn.Service/Manager.cs @@ -19,8 +19,6 @@ public enum TunnelStatus public interface IManager : IDisposable { public Task StopAsync(CancellationToken ct = default); - - public Task HandleSystemResume(CancellationToken ct = default); } /// @@ -28,8 +26,7 @@ public interface IManager : IDisposable /// public class Manager : IManager { - // WakeMinimumTunnelRpcVersion is the lowest RPC version negotiated with - // the tunnel that supports WakeRequest messages. + // Minimum tunnel RPC version that supports WakeRequest. private static readonly RpcVersion WakeMinimumTunnelRpcVersion = new(1, 3); private static readonly TimeSpan WakeReplyTimeout = TimeSpan.FromSeconds(5); @@ -39,6 +36,7 @@ public class Manager : IManager private readonly ITunnelSupervisor _tunnelSupervisor; private readonly IManagerRpc _managerRpc; private readonly ITelemetryEnricher _telemetryEnricher; + private readonly ISystemResumeMonitor _systemResumeMonitor; private volatile TunnelStatus _status = TunnelStatus.Stopped; @@ -54,7 +52,8 @@ public class Manager : IManager // ReSharper disable once ConvertToPrimaryConstructor public Manager(IOptions config, ILogger logger, IDownloader downloader, - ITunnelSupervisor tunnelSupervisor, IManagerRpc managerRpc, ITelemetryEnricher telemetryEnricher) + ITunnelSupervisor tunnelSupervisor, IManagerRpc managerRpc, ITelemetryEnricher telemetryEnricher, + ISystemResumeMonitor systemResumeMonitor) { _config = config.Value; _logger = logger; @@ -63,10 +62,13 @@ public Manager(IOptions config, ILogger logger, IDownloa _managerRpc = managerRpc; _managerRpc.OnReceive += HandleClientRpcMessage; _telemetryEnricher = telemetryEnricher; + _systemResumeMonitor = systemResumeMonitor; + _systemResumeMonitor.Resumed += HandleSystemResumed; } public void Dispose() { + _systemResumeMonitor.Resumed -= HandleSystemResumed; _managerRpc.OnReceive -= HandleClientRpcMessage; GC.SuppressFinalize(this); } @@ -77,32 +79,29 @@ public async Task StopAsync(CancellationToken ct = default) await BroadcastStatus(null, ct); } + private void HandleSystemResumed(object? sender, EventArgs e) + { + // Runs on the SystemEvents broadcast thread, so send in the background. + _ = SendWakeRequest(); + } + /// - /// Sends a WakeRequest to the tunnel so it re-discovers network paths. After the system resumes from sleep, - /// the tunnel can be unusable for several minutes because a short sleep on the same network often produces no - /// link-change event that would otherwise trigger recovery. The request is a hint only; failures are logged - /// and never affect the running tunnel. + /// Sends a WakeRequest to the tunnel so it re-discovers network paths after a system resume. Failures are + /// logged and never affect the running tunnel. /// - /// Cancellation token - public async Task HandleSystemResume(CancellationToken ct = default) + public async Task SendWakeRequest(CancellationToken ct = default) { var version = _tunnelSupervisor.NegotiatedVersion; - if (version is null) - { - _logger.LogDebug("Skipping wake request, tunnel is not running"); - return; - } - - if (!version.SupportsFeature(WakeMinimumTunnelRpcVersion)) + if (version is null || !version.SupportsFeature(WakeMinimumTunnelRpcVersion)) { - _logger.LogDebug( - "Skipping wake request, negotiated tunnel RPC version {Version} is older than {MinimumVersion}", - version, WakeMinimumTunnelRpcVersion); + _logger.LogDebug("Skipping wake request, tunnel is not running or version {Version} does not support it", + version); return; } try { + _logger.LogInformation("Sending wake request to tunnel after system resume"); using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); cts.CancelAfter(WakeReplyTimeout); var reply = await _tunnelSupervisor.SendRequestAwaitReply(new ManagerMessage @@ -112,10 +111,6 @@ public async Task HandleSystemResume(CancellationToken ct = default) if (reply.MsgCase != TunnelMessage.MsgOneofCase.Wake) _logger.LogWarning("Tunnel replied to wake request with unexpected message type {MessageType}", reply.MsgCase); - else if (!reply.Wake.Success) - _logger.LogWarning("Tunnel failed to handle wake request"); - else - _logger.LogDebug("Tunnel handled wake request successfully"); } catch (Exception e) { diff --git a/Vpn.Service/Program.cs b/Vpn.Service/Program.cs index 6fd12ae..753fb16 100644 --- a/Vpn.Service/Program.cs +++ b/Vpn.Service/Program.cs @@ -81,11 +81,11 @@ private static async Task BuildAndRun(string[] args) builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); + builder.Services.AddSingleton(); // Services builder.Services.AddHostedService(); builder.Services.AddHostedService(); - builder.Services.AddHostedService(); // Either run as a Windows service or a console application if (!Environment.UserInteractive) diff --git a/Vpn.Service/SystemResumeMonitor.cs b/Vpn.Service/SystemResumeMonitor.cs index 89907b1..365b5b7 100644 --- a/Vpn.Service/SystemResumeMonitor.cs +++ b/Vpn.Service/SystemResumeMonitor.cs @@ -1,46 +1,32 @@ -using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; using Microsoft.Win32; namespace Coder.Desktop.Vpn.Service; -/// -/// Watches for the system resuming from sleep and notifies the manager so it can prompt the tunnel to re-discover -/// network paths. -/// -public class SystemResumeMonitor : IHostedService +public interface ISystemResumeMonitor : IDisposable { - private readonly ILogger _logger; - private readonly IManager _manager; + /// + /// Raised when the system resumes from sleep. + /// + event EventHandler? Resumed; +} - // ReSharper disable once ConvertToPrimaryConstructor - public SystemResumeMonitor(ILogger logger, IManager manager) - { - _logger = logger; - _manager = manager; - } +public class SystemResumeMonitor : ISystemResumeMonitor +{ + public event EventHandler? Resumed; - public Task StartAsync(CancellationToken ct) + public SystemResumeMonitor() { - // SystemEvents delivers power notifications on a dedicated broadcast - // thread, so no message pump is required in this process. SystemEvents.PowerModeChanged += OnPowerModeChanged; - return Task.CompletedTask; } - public Task StopAsync(CancellationToken ct) + public void Dispose() { SystemEvents.PowerModeChanged -= OnPowerModeChanged; - return Task.CompletedTask; + GC.SuppressFinalize(this); } private void OnPowerModeChanged(object sender, PowerModeChangedEventArgs e) { - if (e.Mode != PowerModes.Resume) return; - _logger.LogInformation("System resumed from sleep, notifying manager"); - // Handle the resume in the background to avoid blocking the shared - // SystemEvents broadcast thread. HandleSystemResume logs and swallows - // all send failures. - _ = Task.Run(() => _manager.HandleSystemResume()); + if (e.Mode == PowerModes.Resume) Resumed?.Invoke(this, EventArgs.Empty); } } diff --git a/Vpn.Service/TunnelSupervisor.cs b/Vpn.Service/TunnelSupervisor.cs index ef414d8..16e7e8e 100644 --- a/Vpn.Service/TunnelSupervisor.cs +++ b/Vpn.Service/TunnelSupervisor.cs @@ -11,8 +11,7 @@ namespace Coder.Desktop.Vpn.Service; public interface ITunnelSupervisor : IAsyncDisposable { /// - /// The RPC version negotiated with the currently running tunnel. Null if the tunnel is not running or its - /// handshake has not completed yet. + /// The RPC version negotiated with the running tunnel, or null if the tunnel is not running. /// public RpcVersion? NegotiatedVersion { get; } diff --git a/Vpn/Speaker.cs b/Vpn/Speaker.cs index 7c6b09f..1864abf 100644 --- a/Vpn/Speaker.cs +++ b/Vpn/Speaker.cs @@ -84,8 +84,7 @@ public class Speaker : IAsyncDisposable public event OnReceiveDelegate? Receive; /// - /// The RPC version negotiated with the peer. Null until the handshake performed by StartAsync has - /// completed successfully. + /// The RPC version negotiated during the handshake. Null until StartAsync completes. /// public RpcVersion? NegotiatedVersion { get; private set; } From 03d1cd408971202b876d5c3759482ef14fa75a4c Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 14 Jul 2026 21:50:42 +0000 Subject: [PATCH 3/4] refactor: use ordered version comparison and condense manager tests Rename SupportsFeature to IsAtLeast with standard version ordering so a future major protocol bump keeps sending wake requests. --- Tests.Vpn.Proto/RpcVersionTest.cs | 14 ++--- Tests.Vpn.Service/ManagerTest.cs | 87 ++++++++----------------------- Vpn.Proto/RpcVersion.cs | 7 ++- Vpn.Service/Manager.cs | 2 +- 4 files changed, 32 insertions(+), 78 deletions(-) diff --git a/Tests.Vpn.Proto/RpcVersionTest.cs b/Tests.Vpn.Proto/RpcVersionTest.cs index dd1514e..150da0a 100644 --- a/Tests.Vpn.Proto/RpcVersionTest.cs +++ b/Tests.Vpn.Proto/RpcVersionTest.cs @@ -41,14 +41,14 @@ public void IsCompatibleWith() IsCompatibleWithBothWays(twoOne, new RpcVersion(1, 1), Is.Null); } - [Test(Description = "Check feature support against the version that introduced the feature")] - public void SupportsFeature() + [Test(Description = "Compare versions with IsAtLeast")] + public void IsAtLeast() { - var featureVersion = new RpcVersion(1, 3); - Assert.That(new RpcVersion(1, 3).SupportsFeature(featureVersion), Is.True); - Assert.That(new RpcVersion(1, 4).SupportsFeature(featureVersion), Is.True); - Assert.That(new RpcVersion(1, 2).SupportsFeature(featureVersion), Is.False); - Assert.That(new RpcVersion(2, 3).SupportsFeature(featureVersion), Is.False); + var oneThree = new RpcVersion(1, 3); + Assert.That(oneThree.IsAtLeast(oneThree), Is.True); + Assert.That(new RpcVersion(1, 4).IsAtLeast(oneThree), Is.True); + Assert.That(new RpcVersion(2, 0).IsAtLeast(oneThree), Is.True); + Assert.That(new RpcVersion(1, 2).IsAtLeast(oneThree), Is.False); } } diff --git a/Tests.Vpn.Service/ManagerTest.cs b/Tests.Vpn.Service/ManagerTest.cs index 06c23fc..f003a0b 100644 --- a/Tests.Vpn.Service/ManagerTest.cs +++ b/Tests.Vpn.Service/ManagerTest.cs @@ -6,53 +6,39 @@ namespace Coder.Desktop.Tests.Vpn.Service; -#region Fakes - -internal class FakeTunnelSupervisor : ITunnelSupervisor +internal class FakeTunnelSupervisor(RpcVersion? negotiatedVersion, Exception? sendException = null) + : ITunnelSupervisor { - public RpcVersion? NegotiatedVersion { get; set; } - public Exception? SendException { get; set; } public List SentRequests { get; } = []; public TaskCompletionSource Sent { get; } = new(); + public RpcVersion? NegotiatedVersion => negotiatedVersion; + public ValueTask SendRequestAwaitReply(ManagerMessage message, CancellationToken ct = default) { SentRequests.Add(message); Sent.TrySetResult(); - if (SendException != null) throw SendException; + if (sendException != null) throw sendException; return ValueTask.FromResult(new TunnelMessage { Wake = new WakeResponse() }); } public Task StartAsync(string binPath, Speaker.OnReceiveDelegate messageHandler, Speaker.OnErrorDelegate errorHandler, CancellationToken ct = default) - { - throw new NotImplementedException(); - } + => throw new NotImplementedException(); - public Task StopAsync(CancellationToken ct = default) - { - return Task.CompletedTask; - } + public Task StopAsync(CancellationToken ct = default) => Task.CompletedTask; public Task SendMessage(ManagerMessage message, CancellationToken ct = default) - { - throw new NotImplementedException(); - } + => throw new NotImplementedException(); - public ValueTask DisposeAsync() - { - return ValueTask.CompletedTask; - } + public ValueTask DisposeAsync() => ValueTask.CompletedTask; } internal class FakeSystemResumeMonitor : ISystemResumeMonitor { public event EventHandler? Resumed; - public void Raise() - { - Resumed?.Invoke(this, EventArgs.Empty); - } + public void Raise() => Resumed?.Invoke(this, EventArgs.Empty); public void Dispose() { @@ -65,67 +51,43 @@ internal class FakeManagerRpc : IManagerRpc public event IManagerRpc.OnReceiveHandler? OnReceive; #pragma warning restore CS0067 - public Task StopAsync(CancellationToken cancellationToken) - { - return Task.CompletedTask; - } - - public Task ExecuteAsync(CancellationToken stoppingToken) - { - return Task.CompletedTask; - } - - public Task BroadcastAsync(ServiceMessage message, CancellationToken ct = default) - { - return Task.CompletedTask; - } - - public ValueTask DisposeAsync() - { - return ValueTask.CompletedTask; - } + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + public Task ExecuteAsync(CancellationToken stoppingToken) => Task.CompletedTask; + public Task BroadcastAsync(ServiceMessage message, CancellationToken ct = default) => Task.CompletedTask; + public ValueTask DisposeAsync() => ValueTask.CompletedTask; } internal class FakeDownloader : IDownloader { public Task StartDownloadAsync(HttpRequestMessage req, string destinationPath, IDownloadValidator validator, CancellationToken ct = default) - { - throw new NotImplementedException(); - } + => throw new NotImplementedException(); } internal class FakeTelemetryEnricher : ITelemetryEnricher { - public StartRequest EnrichStartRequest(StartRequest original) - { - return original; - } + public StartRequest EnrichStartRequest(StartRequest original) => original; } -#endregion - [TestFixture] public class ManagerTest { private static Manager NewManager(ITunnelSupervisor tunnelSupervisor, ISystemResumeMonitor? resumeMonitor = null) - { - return new Manager(Options.Create(new ManagerConfig()), NullLogger.Instance, new FakeDownloader(), + => new(Options.Create(new ManagerConfig()), NullLogger.Instance, new FakeDownloader(), tunnelSupervisor, new FakeManagerRpc(), new FakeTelemetryEnricher(), resumeMonitor ?? new FakeSystemResumeMonitor()); - } [Test(Description = "Send a wake request to the tunnel on system resume")] [CancelAfter(30_000)] public async Task SendsWakeRequestOnResume(CancellationToken ct) { - var supervisor = new FakeTunnelSupervisor { NegotiatedVersion = new RpcVersion(1, 3) }; + var supervisor = new FakeTunnelSupervisor(new RpcVersion(1, 3)); var monitor = new FakeSystemResumeMonitor(); using var manager = NewManager(supervisor, monitor); monitor.Raise(); - await supervisor.Sent.Task.WaitAsync(ct); + Assert.That(supervisor.SentRequests[0].MsgCase, Is.EqualTo(ManagerMessage.MsgOneofCase.Wake)); } @@ -134,10 +96,7 @@ public async Task SendsWakeRequestOnResume(CancellationToken ct) [CancelAfter(30_000)] public async Task SkipsWakeRequestWhenUnsupported(string? version, CancellationToken ct) { - var supervisor = new FakeTunnelSupervisor - { - NegotiatedVersion = version == null ? null : RpcVersion.Parse(version), - }; + var supervisor = new FakeTunnelSupervisor(version == null ? null : RpcVersion.Parse(version)); using var manager = NewManager(supervisor); await manager.SendWakeRequest(ct); @@ -149,11 +108,7 @@ public async Task SkipsWakeRequestWhenUnsupported(string? version, CancellationT [CancelAfter(30_000)] public async Task IgnoresWakeRequestFailure(CancellationToken ct) { - var supervisor = new FakeTunnelSupervisor - { - NegotiatedVersion = new RpcVersion(1, 3), - SendException = new InvalidOperationException("TunnelSupervisor is not running"), - }; + var supervisor = new FakeTunnelSupervisor(new RpcVersion(1, 3), new InvalidOperationException("not running")); using var manager = NewManager(supervisor); await manager.SendWakeRequest(ct); diff --git a/Vpn.Proto/RpcVersion.cs b/Vpn.Proto/RpcVersion.cs index 0a0c797..5f00a59 100644 --- a/Vpn.Proto/RpcVersion.cs +++ b/Vpn.Proto/RpcVersion.cs @@ -64,12 +64,11 @@ public override string ToString() } /// - /// Returns true if a peer with this version supports a feature introduced in the given version. Major - /// versions must match exactly. + /// Returns true if this version is equal to or newer than the other version. /// - public bool SupportsFeature(RpcVersion featureVersion) + public bool IsAtLeast(RpcVersion other) { - return Major == featureVersion.Major && Minor >= featureVersion.Minor; + return Major > other.Major || (Major == other.Major && Minor >= other.Minor); } #region RpcVersion Equality diff --git a/Vpn.Service/Manager.cs b/Vpn.Service/Manager.cs index 1d8cb44..22619bf 100644 --- a/Vpn.Service/Manager.cs +++ b/Vpn.Service/Manager.cs @@ -92,7 +92,7 @@ private void HandleSystemResumed(object? sender, EventArgs e) public async Task SendWakeRequest(CancellationToken ct = default) { var version = _tunnelSupervisor.NegotiatedVersion; - if (version is null || !version.SupportsFeature(WakeMinimumTunnelRpcVersion)) + if (version is null || !version.IsAtLeast(WakeMinimumTunnelRpcVersion)) { _logger.LogDebug("Skipping wake request, tunnel is not running or version {Version} does not support it", version); From 0e260ce897665d974c02bc1241d3897e797eb2de Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 14 Jul 2026 21:53:28 +0000 Subject: [PATCH 4/4] refactor: guarantee wake request send never throws --- Vpn.Service/Manager.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Vpn.Service/Manager.cs b/Vpn.Service/Manager.cs index 22619bf..9c3d396 100644 --- a/Vpn.Service/Manager.cs +++ b/Vpn.Service/Manager.cs @@ -81,7 +81,8 @@ public async Task StopAsync(CancellationToken ct = default) private void HandleSystemResumed(object? sender, EventArgs e) { - // Runs on the SystemEvents broadcast thread, so send in the background. + // Fire-and-forget is safe: SendWakeRequest logs and swallows all + // failures, and must not block the SystemEvents broadcast thread. _ = SendWakeRequest(); } @@ -91,16 +92,16 @@ private void HandleSystemResumed(object? sender, EventArgs e) /// public async Task SendWakeRequest(CancellationToken ct = default) { - var version = _tunnelSupervisor.NegotiatedVersion; - if (version is null || !version.IsAtLeast(WakeMinimumTunnelRpcVersion)) - { - _logger.LogDebug("Skipping wake request, tunnel is not running or version {Version} does not support it", - version); - return; - } - try { + var version = _tunnelSupervisor.NegotiatedVersion; + if (version is null || !version.IsAtLeast(WakeMinimumTunnelRpcVersion)) + { + _logger.LogDebug( + "Skipping wake request, tunnel is not running or version {Version} does not support it", version); + return; + } + _logger.LogInformation("Sending wake request to tunnel after system resume"); using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); cts.CancelAfter(WakeReplyTimeout);