From 73eb46b1fcc8955947a68fa4d1226a35b3181ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deniz=20=C4=B0rgin?= Date: Sat, 17 Oct 2026 09:28:52 +0300 Subject: [PATCH] refactor: the background service runs on an injected TimeProvider (RT-E2 A) IServiceClock and ServiceClock are gone. Discover, Stop and Ensure each choose TimeProvider.System once, as a composition root, and pass it to the info probe, the PTY handoff, the stopper and terminator, the election and the contenders. UTC reads stay UTC at the same read points, and every former CancelAfter is armed at its old point, so no bound moves. New manual-clock tests prove each newly reachable bound (probe, ticket, fallback, election poll, terminator rungs, contender backoff). --- docs/architecture/client-runtime.md | 11 +- .../Internal/Abstractions/IServiceClock.cs | 11 -- .../Internal/Contender/ServiceContender.cs | 10 +- .../Contender/ServiceContenderSpawner.Unix.cs | 2 +- .../ServiceContenderSpawner.Windows.cs | 2 +- .../Contender/ServiceContenderSpawner.cs | 4 +- .../Internal/Discovery/ServiceInfoProbe.cs | 6 +- .../Internal/Ensure/ServiceElection.cs | 18 ++- .../Internal/Ensure/ServiceElectionSeams.cs | 4 +- .../Internal/Ensure/ServiceEnsurer.cs | 6 +- .../Internal/Handoff/ServicePtyHandoff.cs | 16 +-- .../Internal/ServiceClock.cs | 8 -- .../Internal/Stop/ServiceStopper.cs | 5 +- .../Internal/Stop/ServiceTerminator.cs | 8 +- .../OpenCodeServer.BackgroundService.cs | 39 ++++-- .../Internal/Threading/TimedCancellation.cs | 67 +++++++++ .../Internal/Transport/Pipeline.cs | 4 +- .../DiscoveryMode.cs | 2 +- .../OpenCode.Sdk.ServiceFixture/EnsureMode.cs | 1 + .../Harness/EnsureServiceContext.cs | 2 +- .../ServiceContenderSpawnerRequestTests.cs | 2 +- .../Contender/ServiceContenderSpawnerTests.cs | 40 +++++- .../Contender/ServiceContenderTests.cs | 2 +- .../OpenCodeServerDiscoveryLiveTests.cs | 2 +- .../Discovery/ServiceInfoProbeTests.cs | 41 +++++- .../Ensure/ServiceEnsurerTests.cs | 66 ++++++++- .../Handoff/ServicePtyHandoffScenario.cs | 8 +- .../Handoff/ServicePtyHandoffTests.cs | 84 ++++++++++- .../Stop/ServiceStopperTests.cs | 60 +++++++- .../Support/ManualTimeProvider.cs | 5 +- .../Threading/TimedCancellationTests.cs | 130 ++++++++++++++++++ tests/Shared/ContenderLedger.cs | 2 +- .../Generator/Binding/ReservedNamePolicy.cs | 3 +- 33 files changed, 574 insertions(+), 97 deletions(-) delete mode 100644 src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Abstractions/IServiceClock.cs delete mode 100644 src/OpenCode.Sdk/Hosting/BackgroundService/Internal/ServiceClock.cs create mode 100644 src/OpenCode.Sdk/Internal/Threading/TimedCancellation.cs create mode 100644 tests/OpenCode.Sdk.Tests/Threading/TimedCancellationTests.cs diff --git a/docs/architecture/client-runtime.md b/docs/architecture/client-runtime.md index f9fe297c..5774020d 100644 --- a/docs/architecture/client-runtime.md +++ b/docs/architecture/client-runtime.md @@ -1,6 +1,6 @@ # Client Runtime Architecture -Date: 2026-10-16 +Date: 2026-10-17 Canonical current rules for client construction, transport ownership, API errors, streams, and the local server launcher. Protocol and generated-model rules live in @@ -922,6 +922,15 @@ write all run inside `opencode serve --service` (`server-process.ts`), and the e loser's exit 0 the way upstream's does. The CLI's `reconnect` and `restart` have no door of their own; a caller composes them from `StopAsync` and `EnsureAsync`. +Each background-service door chooses `TimeProvider.System` once at its composition root: +Discover supplies its probe; Stop shares it across the probe, handoff and terminator; Ensure +shares it across the election collaborators and spawned contenders. Internal collaborators require +that provider, including the handoff ticket's request pipeline. Request deadlines, election and +termination polls, and contender exit backoff run on it. Election deadline comparisons and spawn +stamps read UTC, and handoff expiry reads UTC epoch milliseconds; neither is a monotonic +elapsed-time policy. Each request deadline arms after probe headers, after ticket pipeline +construction, or at entry to the separate shutdown fallback, respectively. + Where the SDK deliberately differs from the pinned chain, it differs here, each for a recorded reason (ADR-0031): diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Abstractions/IServiceClock.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Abstractions/IServiceClock.cs deleted file mode 100644 index 0f01aaf9..00000000 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Abstractions/IServiceClock.cs +++ /dev/null @@ -1,11 +0,0 @@ -namespace OpenCode.Sdk.Internal; - -/// -/// The wall clock the Ensure election loop reads for its PromiseTimeout deadline, spawn -/// delay, and timeout counter. Behind a seam so tests drive the loop without waiting. -/// -internal interface IServiceClock -{ - /// Gets the current UTC time. - public DateTimeOffset UtcNow { get; } -} diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContender.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContender.cs index c741a423..9b9bd244 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContender.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContender.cs @@ -49,6 +49,7 @@ internal sealed class ServiceContender : IServiceContender private readonly Lock _gate = new(); private readonly Stream _stderr; private readonly SafeProcessHandle? _process; + private readonly TimeProvider _time; /// The handoff ticket's UTF-8 bytes, matched in the raw tail so a cut cannot split a match. private readonly byte[]? _redact; @@ -67,13 +68,14 @@ internal sealed class ServiceContender : IServiceContender /// How the process ended, once a poll learned it; unknown when another reaper took the pid first. private ChildExitStatus? _exit; - public ServiceContender(int processId, Stream stderr, SafeProcessHandle? process, string? redact) + public ServiceContender(int processId, Stream stderr, SafeProcessHandle? process, string? redact, TimeProvider time) { ArgumentNullException.ThrowIfNull(stderr); ProcessId = processId; _stderr = stderr; _process = process; + _time = time; _redact = string.IsNullOrEmpty(redact) ? null : Encoding.UTF8.GetBytes(redact); _retainLimit = StderrLimit + (_redact is null ? 0 : _redact.Length - 1); @@ -437,7 +439,11 @@ private async Task WatchExitAsync() } } - await Task.Delay(delay).ConfigureAwait(false); +#if NET + await Task.Delay(delay, _time).ConfigureAwait(false); +#else + await _time.Delay(delay).ConfigureAwait(false); +#endif delay = delay + delay < ExitWatchCap ? delay + delay : ExitWatchCap; } } diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Unix.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Unix.cs index 61447090..77664bef 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Unix.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Unix.cs @@ -40,6 +40,6 @@ private ServiceContender SpawnUnix( // the contender owns it from here. var stderr = child.StandardError ?? throw new InvalidOperationException("The contender's standard error was routed to a pipe, but the spawn returned no read end."); - return new ServiceContender(child.ProcessId, stderr, null, redaction); + return new ServiceContender(child.ProcessId, stderr, null, redaction, _time); } } diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Windows.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Windows.cs index 5ac4f363..9dc70453 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Windows.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.Windows.cs @@ -50,6 +50,6 @@ private ServiceContender SpawnWindows( throw new InvalidOperationException("The contender's standard error was routed to a pipe, but the spawn returned no read end."); } - return new ServiceContender(child.ProcessId, stderr, child.Process, redaction); + return new ServiceContender(child.ProcessId, stderr, child.Process, redaction, _time); } } diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.cs index 35c9ae09..ac75f37d 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Contender/ServiceContenderSpawner.cs @@ -11,13 +11,15 @@ namespace OpenCode.Sdk.Internal; /// /// The POSIX spawn the Unix arm starts contenders through. /// The Windows spawn the Windows arm starts contenders through. -internal sealed partial class ServiceContenderSpawner(IPosixSpawn posixSpawn, IWindowsSpawn windowsSpawn) : IServiceContenderSpawner +/// The clock the contenders' exit backoff runs on. +internal sealed partial class ServiceContenderSpawner(IPosixSpawn posixSpawn, IWindowsSpawn windowsSpawn, TimeProvider time) : IServiceContenderSpawner { /// The PTY-handoff variable whose value is secret-bearing: removed when empty, redacted from diagnostics when present. private const string HandoffVariable = "OPENCODE_PTY_HANDOFF"; private readonly IPosixSpawn _posixSpawn = posixSpawn; private readonly IWindowsSpawn _windowsSpawn = windowsSpawn; + private readonly TimeProvider _time = time; /// public IServiceContender Spawn(IServiceContenderSpawner.ContenderStartInfo startInfo) => Start(startInfo); diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Discovery/ServiceInfoProbe.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Discovery/ServiceInfoProbe.cs index c38794fe..bd21fe5e 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Discovery/ServiceInfoProbe.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Discovery/ServiceInfoProbe.cs @@ -11,7 +11,7 @@ namespace OpenCode.Sdk.Internal; /// generated info model has a different contract. Discovery keeps its own request bound and never /// forwards credentials on redirects. /// -internal sealed class ServiceInfoProbe(ServiceTiming timing, IServiceProbeTransport transport) : IServiceInfoProbe +internal sealed class ServiceInfoProbe(ServiceTiming timing, IServiceProbeTransport transport, TimeProvider time) : IServiceInfoProbe { /// The pin's info route, resolved against the authority only, as new URL("/api/info", info.url) does. private const string InfoPath = "/api/info"; @@ -35,8 +35,8 @@ public async Task ProbeAsync(ServiceRegistration registratio } // The internal bound and the caller's token are separate: only the caller's propagates. - using var bound = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - bound.CancelAfter(timing.RequestTimeout); + using var deadline = TimedCancellation.Create(timing.RequestTimeout, time); + using var bound = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, deadline.Token); HttpStatusCode status; byte[] body; diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElection.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElection.cs index 82e19996..10027b8b 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElection.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElection.cs @@ -32,13 +32,13 @@ internal sealed class ServiceElection(ServiceElectionSeams seams, EnsureRequest /// The bound expired, the service failed, its health protocol is incompatible at a matching version, a contender failed with none live, or a spawn failed. public async Task RunAsync(CancellationToken cancellationToken) { - var deadline = seams.Clock.UtcNow + seams.Timing.PromiseTimeout; + var deadline = seams.Time.GetUtcNow() + seams.Timing.PromiseTimeout; try { while (true) { cancellationToken.ThrowIfCancellationRequested(); - if (seams.Clock.UtcNow >= deadline) + if (seams.Time.GetUtcNow() >= deadline) { throw _failure ?? (_lastReplaceFailure is null ? new OpenCodeServerException(ServiceEnsurer.TimeoutMessage) @@ -67,7 +67,11 @@ public async Task RunAsync(CancellationToken cancellationTo await HarvestAndMaybeSpawnAsync(registration is not null, cancellationToken).ConfigureAwait(false); } - await Task.Delay(seams.Timing.PollInterval, cancellationToken).ConfigureAwait(false); +#if NET + await Task.Delay(seams.Timing.PollInterval, seams.Time, cancellationToken).ConfigureAwait(false); +#else + await seams.Time.Delay(seams.Timing.PollInterval, cancellationToken).ConfigureAwait(false); +#endif } } finally @@ -107,7 +111,7 @@ private async Task RecoverUnresponsiveAsync(ServiceRegistration? registration, S ReleaseStoppedContenders(registration.ProcessId); _timeoutIdentity = null; _timeoutCount = 0; - _lastSpawn = seams.Clock.UtcNow - _spawnDelay; + _lastSpawn = seams.Time.GetUtcNow() - _spawnDelay; } /// @@ -231,7 +235,7 @@ private async Task HarvestAndMaybeSpawnAsync(bool registered, CancellationToken { if (_lastSpawn is null && registered) { - _lastSpawn = seams.Clock.UtcNow; + _lastSpawn = seams.Time.GetUtcNow(); } var finished = _contenders.Where(static contender => contender.Finished).ToList(); @@ -255,11 +259,11 @@ private async Task HarvestAndMaybeSpawnAsync(bool registered, CancellationToken // One candidate plus one lock probe for pre-lock stalls. After a failure, the survivors // finish without replacements that could hide the error until the bound. - if (_failure is null && _contenders.Count < 2 && (_lastSpawn is not { } spawned || seams.Clock.UtcNow - spawned >= _spawnDelay)) + if (_failure is null && _contenders.Count < 2 && (_lastSpawn is not { } spawned || seams.Time.GetUtcNow() - spawned >= _spawnDelay)) { Announce(OpenCodeServerEnsureReason.Missing, previousVersion: null); _contenders.Add(await SpawnAsync(cancellationToken).ConfigureAwait(false)); - _lastSpawn = seams.Clock.UtcNow; + _lastSpawn = seams.Time.GetUtcNow(); } } diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElectionSeams.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElectionSeams.cs index 45b464b0..681dc07e 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElectionSeams.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceElectionSeams.cs @@ -9,7 +9,7 @@ namespace OpenCode.Sdk.Internal; /// The persistent-terminal handoff sidecar. /// The stop ladder the recovery and the replacement end a daemon with. /// The registration and service-config file access. -/// The wall clock the deadline and the spawn delay are measured on. +/// The clock for polling and the UTC deadline and spawn stamps. /// The launcher's executable resolution for the command's first entry. /// The lifecycle timing. internal sealed record ServiceElectionSeams( @@ -18,6 +18,6 @@ internal sealed record ServiceElectionSeams( IServicePtyHandoff Handoff, ServiceTerminator Terminator, IServiceFileSystem FileSystem, - IServiceClock Clock, + TimeProvider Time, ExecutableResolver Executables, ServiceTiming Timing); diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceEnsurer.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceEnsurer.cs index 8adcdba7..7980a5e7 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceEnsurer.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Ensure/ServiceEnsurer.cs @@ -13,7 +13,7 @@ internal sealed class ServiceEnsurer( IServiceContenderSpawner spawner, IServicePtyHandoff handoff, IServiceProcessControl processControl, - IServiceClock clock, + TimeProvider time, ExecutableResolver executableResolver, ServiceTiming timing) { @@ -59,9 +59,9 @@ public async Task EnsureAsync(OpenCodeServerEnsureOptions? probe, spawner, handoff, - new ServiceTerminator(fileSystem, processControl, timing), + new ServiceTerminator(fileSystem, processControl, timing, time), fileSystem, - clock, + time, executableResolver, timing); return await new ServiceElection(seams, request, paths).RunAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Handoff/ServicePtyHandoff.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Handoff/ServicePtyHandoff.cs index 094a456b..0007abdb 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Handoff/ServicePtyHandoff.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Handoff/ServicePtyHandoff.cs @@ -14,7 +14,7 @@ namespace OpenCode.Sdk.Internal; /// but the caller's cancellation replaces the service without a handoff. /// Time comes from the injected clock so every expiry rule is testable without waiting. /// -internal sealed class ServicePtyHandoff(IServiceFileSystem fileSystem, IServiceClock clock, IServicePtyShutdown ptyShutdown) +internal sealed class ServicePtyHandoff(IServiceFileSystem fileSystem, TimeProvider time, IServicePtyShutdown ptyShutdown) : IServicePtyHandoff { /// The sidecar suffix the pinned client keeps beside the registration. @@ -140,7 +140,7 @@ public Task ClearAsync(string registrationFile, CancellationToken cancellationTo /// The handoff route through the SDK's own request pipeline — the transport, credential, and /// status mapping every generated door uses — with the answer kept raw, under the request bound. /// - private static async Task RequestTicketAsync( + private async Task RequestTicketAsync( ServiceRegistration registration, TimeSpan timeout, CancellationToken cancellationToken) @@ -149,9 +149,9 @@ private static async Task RequestTicketAsync( { Endpoint = registration.Endpoint, Password = registration.Password, - }); - using var bound = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - bound.CancelAfter(timeout); + }, time); + using var deadline = TimedCancellation.Create(timeout, time); + using var bound = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, deadline.Token); return await pipeline .ExecuteAsync("persistentPty.handoff", HttpMethod.Post, OpenCodeRoutes.PersistentPtys.Handoff, ServiceHandoffTicketAdapter.Instance, options: null, bound.Token) .ConfigureAwait(false); @@ -193,8 +193,8 @@ private PtyHandoffSidecar Parse(ServiceHandoffTicketResponse answer, ServiceRegi "The pinned client's shutdown .catch(() => {}) is best-effort (prepare in pty-handoff.ts): endpoint refusal or a failed shutdown leads to a null sidecar and replacement without these terminals.")] private async Task ShutdownAsync(ServiceRegistration registration, TimeSpan timeout, CancellationToken cancellationToken) { - using var bound = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - bound.CancelAfter(timeout); + using var deadline = TimedCancellation.Create(timeout, time); + using var bound = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, deadline.Token); try { await ptyShutdown.ShutdownAsync(registration, bound.Token).ConfigureAwait(false); @@ -319,5 +319,5 @@ private static bool Matches(PtyHandoffSidecar sidecar, ServiceRegistration regis sidecar.SourcePid == registration.ProcessId && string.Equals(sidecar.SourceUrl, registration.Url, StringComparison.Ordinal); - private double NowMilliseconds() => clock.UtcNow.ToUnixTimeMilliseconds(); + private double NowMilliseconds() => time.GetUtcNow().ToUnixTimeMilliseconds(); } diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/ServiceClock.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/ServiceClock.cs deleted file mode 100644 index 2d88501a..00000000 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/ServiceClock.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace OpenCode.Sdk.Internal; - -/// The production over . -internal sealed class ServiceClock : IServiceClock -{ - /// - public DateTimeOffset UtcNow => DateTimeOffset.UtcNow; -} diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceStopper.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceStopper.cs index a58a4379..f7f56ade 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceStopper.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceStopper.cs @@ -18,7 +18,8 @@ internal sealed class ServiceStopper( IServicePtyShutdown ptyShutdown, IServicePtyHandoff ptyHandoff, IServiceProcessControl processControl, - ServiceTiming timing) + ServiceTiming timing, + TimeProvider time) { /// Stops the registered service a selection points at. /// The caller's options; null means every default. @@ -50,7 +51,7 @@ public async Task StopAsync(OpenCodeServerStopOptions? options, CancellationToke await ClearSidecarAsync(paths.RegistrationFile, cancellationToken).ConfigureAwait(false); if (registration is not null) { - await new ServiceTerminator(fileSystem, processControl, timing) + await new ServiceTerminator(fileSystem, processControl, timing, time) .TerminateAsync(registration, paths.RegistrationFile, cancellationToken) .ConfigureAwait(false); } diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceTerminator.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceTerminator.cs index d9966a54..c5e4fdb8 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceTerminator.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Stop/ServiceTerminator.cs @@ -15,7 +15,7 @@ namespace OpenCode.Sdk.Internal; /// reused reads as the registered process being gone. The one failure is a process still running /// after the kill rung; the registration then stays. /// -internal sealed class ServiceTerminator(IServiceFileSystem fileSystem, IServiceProcessControl processControl, ServiceTiming timing) +internal sealed class ServiceTerminator(IServiceFileSystem fileSystem, IServiceProcessControl processControl, ServiceTiming timing, TimeProvider time) { /// Ends the registered process and removes its registration. /// The registration as it was read. @@ -87,7 +87,11 @@ private async Task WaitForExitAsync(ProcessIdentity target, CancellationTo if (attempt < timing.StopPollAttempts) { - await Task.Delay(timing.StopPollInterval, cancellationToken).ConfigureAwait(false); +#if NET + await Task.Delay(timing.StopPollInterval, time, cancellationToken).ConfigureAwait(false); +#else + await time.Delay(timing.StopPollInterval, cancellationToken).ConfigureAwait(false); +#endif } } diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs index f8982c90..b20e9d78 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs @@ -19,8 +19,12 @@ public partial class OpenCodeServer /// No user home directory resolves for an XDG fallback. public static Task DiscoverAsync( OpenCodeServerDiscoverOptions? options = null, - CancellationToken cancellationToken = default) => - DiscoverWithSeamsAsync(options, PlatformProbe(ServiceTiming.Default), cancellationToken); + CancellationToken cancellationToken = default) + { + // The discovery composition root chooses the clock for its info probe. + var time = TimeProvider.System; + return DiscoverWithSeamsAsync(options, PlatformProbe(ServiceTiming.Default, time), cancellationToken); + } /// /// over an injected probe, the seam the isolated test executable uses @@ -44,8 +48,8 @@ public partial class OpenCodeServer } /// The info probe over the platform's loopback transport and socket connect. - private static ServiceInfoProbe PlatformProbe(ServiceTiming timing) => - new(timing, new LoopbackTransport(new LoopbackSocketConnector())); + private static ServiceInfoProbe PlatformProbe(ServiceTiming timing, TimeProvider time) => + new(timing, new LoopbackTransport(new LoopbackSocketConnector()), time); /// /// The non-owning handle over a registered service. Discover and the Ensure election return only @@ -82,16 +86,19 @@ public static Task StopAsync( CancellationToken cancellationToken = default) { var timing = ServiceTiming.Default; + // The stop composition root shares one clock across its probe, handoff and terminator. + var time = TimeProvider.System; var fileSystem = new ServiceFileSystem(); var ptyShutdown = new ServicePtyShutdown(); var stopper = new ServiceStopper( new ServiceEnvironment(), fileSystem, - PlatformProbe(timing), + PlatformProbe(timing, time), ptyShutdown, - new ServicePtyHandoff(fileSystem, new ServiceClock(), ptyShutdown), + new ServicePtyHandoff(fileSystem, time, ptyShutdown), new ServiceProcessControl(), - timing); + timing, + time); return stopper.StopAsync(options, cancellationToken); } @@ -111,8 +118,12 @@ public static Task StopAsync( /// No user home directory resolves, the election timed out, a contender or the registered service failed to start, the registered service speaks an incompatible health protocol at a version the call accepts, a version mismatch was refused, or a spawned command could not be resolved. public static Task EnsureAsync( OpenCodeServerEnsureOptions? options = null, - CancellationToken cancellationToken = default) => - EnsureWithSeamsAsync(options, ServiceTiming.Default, new ServiceContenderSpawner(new PosixSpawn(), new WindowsSpawn()), cancellationToken); + CancellationToken cancellationToken = default) + { + // The ensure composition root shares one clock across the election and its contenders. + var time = TimeProvider.System; + return EnsureWithSeamsAsync(options, ServiceTiming.Default, new ServiceContenderSpawner(new PosixSpawn(), new WindowsSpawn(), time), time, cancellationToken); + } /// /// The seam-injected Ensure the tests use, kept out of the public option types the way upstream's @@ -125,27 +136,29 @@ public static Task EnsureAsync( /// The ensure options; null uses every default. /// The lifecycle timing; the public door passes . /// The contender spawn; the public door passes the platform spawner. + /// The clock shared by the election collaborators. /// The caller's token; its cancellation propagates. /// A non-owning handle over the ready service. internal static async Task EnsureWithSeamsAsync( OpenCodeServerEnsureOptions? options, ServiceTiming timing, IServiceContenderSpawner spawner, + TimeProvider time, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(timing); ArgumentNullException.ThrowIfNull(spawner); + ArgumentNullException.ThrowIfNull(time); var fileSystem = new ServiceFileSystem(); - var clock = new ServiceClock(); var ensurer = new ServiceEnsurer( new ServiceEnvironment(), fileSystem, - PlatformProbe(timing), + PlatformProbe(timing, time), spawner, - new ServicePtyHandoff(fileSystem, clock, new ServicePtyShutdown()), + new ServicePtyHandoff(fileSystem, time, new ServicePtyShutdown()), new ServiceProcessControl(), - clock, + time, new ExecutableResolver(ExecutableSearchEnvironment.ForCurrentProcess()), timing); var registration = await ensurer.EnsureAsync(options, cancellationToken).ConfigureAwait(false); diff --git a/src/OpenCode.Sdk/Internal/Threading/TimedCancellation.cs b/src/OpenCode.Sdk/Internal/Threading/TimedCancellation.cs new file mode 100644 index 00000000..de0fe41e --- /dev/null +++ b/src/OpenCode.Sdk/Internal/Threading/TimedCancellation.cs @@ -0,0 +1,67 @@ +namespace OpenCode.Sdk.Internal; + +/// Creates provider-driven deadlines that release their timers when disposed early. +/// +/// The downlevel BCL extension releases its separate timer only on cancellation, so its timer +/// outlives a source disposed before the deadline. The modern constructor owns its timer already. +/// After disposal both paths promise only that a late timer callback never throws; whether the +/// token then reads as canceled is unspecified (the modern constructor may cancel it). +/// +internal static class TimedCancellation +{ + /// Creates a deadline whose source owns the provider's timer until disposal. + /// How long until cancellation. + /// The clock that drives the deadline. + /// A source that cancels at the deadline and releases its timer on disposal. + public static CancellationTokenSource Create(TimeSpan timeout, TimeProvider time) => +#if NET + new(timeout, time); +#else + new TimerCancellationTokenSource(timeout, time); +#endif + +#if !NET + /// Owns the timer that the downlevel BCL extension leaves armed after early disposal. + private sealed class TimerCancellationTokenSource : CancellationTokenSource + { + private readonly Lock _gate = new(); + private readonly ITimer? _timer; + private bool _disposed; + + public TimerCancellationTokenSource(TimeSpan timeout, TimeProvider time) => + _timer = time.CreateTimer(static state => + { + // The timer's state is always its owning source. + var source = (TimerCancellationTokenSource)state; + // The lock prevents cancel/dispose overlap; callbacks run under it, so waiting on another thread disposing this deadline would deadlock (nothing in the SDK does that). + lock (source._gate) + { + if (!source._disposed) + { + source.Cancel(); + } + } + }, this, timeout, Timeout.InfiniteTimeSpan); + + protected override void Dispose(bool disposing) + { + if (disposing) + { + lock (_gate) + { + if (_disposed) + { + return; + } + + _disposed = true; + } + + _timer?.Dispose(); + } + + base.Dispose(disposing); + } + } +#endif +} diff --git a/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs b/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs index aae87803..92be2404 100644 --- a/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs +++ b/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs @@ -104,7 +104,7 @@ internal Pipeline(HttpClient httpClient, bool ownsHttpClient, OpenCodeClientOpti /// private bool CredentialSent => Connection.Authorization is not null; - public static Pipeline Create(OpenCodeClientOptions options) + public static Pipeline Create(OpenCodeClientOptions options, TimeProvider? time = null) { ArgumentNullException.ThrowIfNull(options); @@ -118,7 +118,7 @@ public static Pipeline Create(OpenCodeClientOptions options) var httpClient = TransportPolicy.CreateOwnedHttpClient(options.Endpoint); try { - return new Pipeline(httpClient, ownsHttpClient: true, options); + return new Pipeline(httpClient, ownsHttpClient: true, options, time: time); } catch { diff --git a/tests/OpenCode.Sdk.ServiceFixture/DiscoveryMode.cs b/tests/OpenCode.Sdk.ServiceFixture/DiscoveryMode.cs index 1a42682a..2ccacc01 100644 --- a/tests/OpenCode.Sdk.ServiceFixture/DiscoveryMode.cs +++ b/tests/OpenCode.Sdk.ServiceFixture/DiscoveryMode.cs @@ -28,7 +28,7 @@ public static async Task RunAsync(OpenCodeServerDiscoverOptions? options, T // a test reads the verdict, the time and the timeline are only diagnostics. var timing = requestTimeout is { } bound ? ServiceTiming.Default with { RequestTimeout = bound } : ServiceTiming.Default; var transport = new LoopbackTransport(new TimelineConnector(new LoopbackSocketConnector(), timeline)); - var probe = new RecordingProbe(new ServiceInfoProbe(timing, transport), timeline); + var probe = new RecordingProbe(new ServiceInfoProbe(timing, transport, TimeProvider.System), timeline); var stopwatch = Stopwatch.StartNew(); var server = await OpenCodeServer.DiscoverWithSeamsAsync(options, probe, CancellationToken.None).ConfigureAwait(false); await Console.Error diff --git a/tests/OpenCode.Sdk.ServiceFixture/EnsureMode.cs b/tests/OpenCode.Sdk.ServiceFixture/EnsureMode.cs index 1f2be714..d7b365b3 100644 --- a/tests/OpenCode.Sdk.ServiceFixture/EnsureMode.cs +++ b/tests/OpenCode.Sdk.ServiceFixture/EnsureMode.cs @@ -29,6 +29,7 @@ public static async Task RunAsync(string channel, string ledger) new OpenCodeServerEnsureOptions { Channel = channel }, ServiceTiming.Default, new ContenderLedger(new RealFileSystem(), ledger), + TimeProvider.System, CancellationToken.None).ConfigureAwait(false); await Console.Error .WriteLineAsync($"ensure took {stopwatch.ElapsedMilliseconds.ToString(CultureInfo.InvariantCulture)} ms") diff --git a/tests/OpenCode.Sdk.Tests/Harness/EnsureServiceContext.cs b/tests/OpenCode.Sdk.Tests/Harness/EnsureServiceContext.cs index 5b4d7852..182a647c 100644 --- a/tests/OpenCode.Sdk.Tests/Harness/EnsureServiceContext.cs +++ b/tests/OpenCode.Sdk.Tests/Harness/EnsureServiceContext.cs @@ -102,7 +102,7 @@ public Task EnsureAsync(OpenCodeServerEnsureOptions options, Can /// The non-owning handle Ensure returns. public async Task EnsureAsync(OpenCodeServerEnsureOptions options, ServiceTiming timing, CancellationToken cancellationToken) { - var server = await OpenCodeServer.EnsureWithSeamsAsync(options, timing, Ledger, cancellationToken).ConfigureAwait(false); + var server = await OpenCodeServer.EnsureWithSeamsAsync(options, timing, Ledger, TimeProvider.System, cancellationToken).ConfigureAwait(false); // The handle does not own the elected service; a service that fails the check is ended by // this context's teardown like every other one it recorded. diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerRequestTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerRequestTests.cs index 1c071a3c..4f6c6f29 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerRequestTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerRequestTests.cs @@ -157,7 +157,7 @@ public async Task Spawn_Should_Refuse_A_Missing_Stderr_Read_End_And_Close_The_Wi } } - private static ServiceContenderSpawner Spawner(RecordingSpawn recording) => new(recording.Posix, recording.Windows); + private static ServiceContenderSpawner Spawner(RecordingSpawn recording) => new(recording.Posix, recording.Windows, TimeProvider.System); private static IServiceContenderSpawner.ContenderStartInfo Start( ResolvedExecutable? executable = null, IReadOnlyDictionary? environment = null) => diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerTests.cs index 3b773d93..d89a6318 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderSpawnerTests.cs @@ -59,7 +59,45 @@ public sealed class ServiceContenderSpawnerTests private static readonly RealFileSystem FileSystem = new(); /// The shipped spawner over the shipped spawns; it holds no per-spawn state. - private static readonly ServiceContenderSpawner Spawner = new(new PosixSpawn(), new WindowsSpawn()); + private static readonly ServiceContenderSpawner Spawner = new(new PosixSpawn(), new WindowsSpawn(), TimeProvider.System); + + [Test] + [Timeout(120_000)] + public async Task Contender_Should_Back_Off_On_The_Injected_Clock(CancellationToken cancellationToken) + { + var time = new ManualTimeProvider(); + using var contender = new ServiceContenderSpawner(new PosixSpawn(), new WindowsSpawn(), time) + .Start(FixtureProbe("contender-probe", "daemon-sleep")); + try + { + _ = await LiveReadiness.WaitAsync( + _ => Task.FromResult(contender.Stderr), + tail => ReadyPid(tail) == contender.ProcessId, + "the contender's ready line before advancing its exit watch", + cancellationToken); + var delay = TimeSpan.FromMilliseconds(1); + for (var poll = 0; poll < 12; poll++) + { + await time.WaitForArmedAsync(delay, ExitBound); + time.Advance(delay - TimeSpan.FromTicks(1)); + await Assert.That(time.ArmedIn(TimeSpan.FromTicks(1))).IsEqualTo(1); + time.Advance(TimeSpan.FromTicks(1)); + delay = delay + delay < TimeSpan.FromSeconds(1) ? delay + delay : TimeSpan.FromSeconds(1); + } + + await time.WaitForArmedAsync(TimeSpan.FromSeconds(1), ExitBound); + await Assert.That(ProcessObservation.IsRunning(contender.ProcessId)).IsTrue(); + } + finally + { + ProcessObservation.KillIfRunning(contender.ProcessId); + time.Advance(TimeSpan.FromSeconds(1)); + _ = await WaitForTheProbeAsync(contender, "the killed contender to finish", CancellationToken.None) + .WaitAsync(ExitBound, CancellationToken.None); + await ProcessObservation.WaitForExitAsync(contender.ProcessId, CancellationToken.None).WaitAsync(ExitBound, CancellationToken.None); + contender.Dispose(); + } + } [Test] [Timeout(120_000)] diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderTests.cs index 368a2929..91958ed8 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Contender/ServiceContenderTests.cs @@ -137,7 +137,7 @@ public async Task Release_Should_Drop_The_Retained_Stderr() } private static ServiceContender Contender(string stderr = "", string? redact = Ticket, Stream? stream = null) => - new(Environment.ProcessId, stream ?? new MemoryStream(Encoding.UTF8.GetBytes(stderr)), process: null, redact); + new(Environment.ProcessId, stream ?? new MemoryStream(Encoding.UTF8.GetBytes(stderr)), process: null, redact, TimeProvider.System); /// Both target-framework read paths throw the same fault before any read suspends. private sealed class FaultingStderr(IOException failure) : MemoryStream diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/OpenCodeServerDiscoveryLiveTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/OpenCodeServerDiscoveryLiveTests.cs index 04cd4d53..d703612c 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/OpenCodeServerDiscoveryLiveTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/OpenCodeServerDiscoveryLiveTests.cs @@ -108,7 +108,7 @@ public async Task DiscoverAsync_Should_Apply_ExpectedVersion_Against_The_Accepte new OpenCodeServerDiscoverOptions { RegistrationFilePath = service.RegistrationFile, ExpectedVersion = service.Version }, cancellationToken); await using var _ = matching; - var probe = new RecordingProbe(new ServiceInfoProbe(ServiceTimingData.Patient, new LoopbackTransport(new LoopbackSocketConnector()))); + var probe = new RecordingProbe(new ServiceInfoProbe(ServiceTimingData.Patient, new LoopbackTransport(new LoopbackSocketConnector()), TimeProvider.System)); var mismatching = await OpenCodeServer.DiscoverWithSeamsAsync( new OpenCodeServerDiscoverOptions { RegistrationFilePath = service.RegistrationFile, ExpectedVersion = "0.0.0-never-1" }, probe, diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/ServiceInfoProbeTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/ServiceInfoProbeTests.cs index 5f09a0b7..50563eba 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/ServiceInfoProbeTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Discovery/ServiceInfoProbeTests.cs @@ -21,6 +21,39 @@ public sealed class ServiceInfoProbeTests { private const string Password = ServiceRegistrationData.Password; private static readonly ServiceTiming FastTiming = ServiceTiming.Default with { RequestTimeout = TimeSpan.FromMilliseconds(100) }; + private static readonly TimeSpan HangGuard = TimeSpan.FromSeconds(10); + + [Test] + public async Task ProbeAsync_Should_Expire_The_Request_On_The_Injected_Clock() + { + var time = new ManualTimeProvider(); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await using var server = LoopbackHttpServer.Start(_ => + { + entered.TrySetResult(); + return Json(HttpStatusCode.OK, ServiceInfoBodyData.Ready) with { KeepOpen = true }; + }); + using var caller = new CancellationTokenSource(); + var pending = new ServiceInfoProbe(ServiceTimingData.Patient, PlatformTransport(), time) + .ProbeAsync(Registration(server.Endpoint), caller.Token); + try + { + await entered.Task.WaitAsync(HangGuard); + await time.WaitForArmedAsync(ServiceTimingData.Patient.RequestTimeout, HangGuard); + time.Advance(ServiceTimingData.Patient.RequestTimeout - TimeSpan.FromTicks(1)); + await Assert.That(pending.IsCompleted).IsFalse(); + time.Advance(TimeSpan.FromTicks(1)); + + var result = await pending.WaitAsync(HangGuard); + await Assert.That(result.TimedOut).IsTrue(); + await Assert.That(result.IsService).IsFalse(); + } + finally + { + await caller.CancelOnWorkerAsync(); + server.ReleaseResponses(); + } + } [Test] public async Task ProbeAsync_Should_Report_A_Ready_Service_And_Send_The_Basic_Credential() @@ -223,7 +256,7 @@ public async Task ProbeAsync_Should_Report_A_Timeout_At_The_Injected_Bound() { await using var server = LoopbackHttpServer.Start(static _ => Json(HttpStatusCode.OK, ServiceInfoBodyData.Ready) with { KeepOpen = true }); - var result = await new ServiceInfoProbe(FastTiming, PlatformTransport()).ProbeAsync(Registration(server.Endpoint), CancellationToken.None); + var result = await new ServiceInfoProbe(FastTiming, PlatformTransport(), TimeProvider.System).ProbeAsync(Registration(server.Endpoint), CancellationToken.None); await Assert.That(result.TimedOut).IsTrue(); await Assert.That(result.IsService).IsFalse(); @@ -368,7 +401,7 @@ public async Task ProbeAsync_Should_Report_No_Service_For_A_Refusal_That_Reaches var endpoint = await ClosedEndpointAsync(); var transport = new BoundOutlastingTransport(PlatformTransport()); - var result = await new ServiceInfoProbe(FastTiming, transport).ProbeAsync(Registration(endpoint), CancellationToken.None); + var result = await new ServiceInfoProbe(FastTiming, transport, TimeProvider.System).ProbeAsync(Registration(endpoint), CancellationToken.None); await Assert.That(transport.Handler).IsNotNull(); await Assert.That(transport.Handler!.Refused).IsTrue(); @@ -388,7 +421,7 @@ public async Task ProbeAsync_Should_Rethrow_Caller_Cancellation_That_Arrives_Wit var transport = new LoopbackTransport(new ScriptedLoopbackConnector(caller, ConnectFailure(SocketError.ConnectionRefused))); var exception = await Assert - .That(async () => _ = await new ServiceInfoProbe(ServiceTimingData.Patient, transport).ProbeAsync(Registration(endpoint), caller.Token)) + .That(async () => _ = await new ServiceInfoProbe(ServiceTimingData.Patient, transport, TimeProvider.System).ProbeAsync(Registration(endpoint), caller.Token)) .Throws(); await Assert.That(exception!.CancellationToken).IsEqualTo(caller.Token); @@ -420,7 +453,7 @@ public async Task The_Result_Should_Never_Render_The_Credential() /// bound; the bound itself is proven once, with , against a kept-open /// response. /// - private static ServiceInfoProbe Probe() => new(ServiceTimingData.Patient, PlatformTransport()); + private static ServiceInfoProbe Probe() => new(ServiceTimingData.Patient, PlatformTransport(), TimeProvider.System); private static LoopbackTransport PlatformTransport() => new(new LoopbackSocketConnector()); diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Ensure/ServiceEnsurerTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Ensure/ServiceEnsurerTests.cs index c364c134..6acf8e70 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Ensure/ServiceEnsurerTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Ensure/ServiceEnsurerTests.cs @@ -6,6 +6,7 @@ using OpenCode.Sdk.Tests.Support; using Testably.Abstractions.Testing; using TUnit.Assertions.Enums; +using TimeProvider = System.TimeProvider; namespace OpenCode.Sdk.Tests.Hosting.BackgroundService.Ensure; @@ -47,7 +48,8 @@ public sealed class ServiceEnsurerTests private readonly IServiceContenderSpawner _spawner = Substitute.For(); private readonly IServicePtyHandoff _handoff = Substitute.For(); private readonly IServiceProcessControl _processControl = Substitute.For(); - private readonly IServiceClock _clock = Substitute.For(); + private readonly TimeProvider _clock = Substitute.For(); + private readonly ManualTimeProvider _time = new(Start); private readonly List _spawned = []; private readonly List _starts = []; private readonly HashSet _signalled = []; @@ -62,6 +64,8 @@ public ServiceEnsurerTests() _environment.GetEnvironmentVariable("XDG_CONFIG_HOME").Returns(Root("config")); // Bounded by default: an implementation that loops where it should refuse times out, never hangs. DeadlineAfter(clockReads: 5_000); + _clock.CreateTimer(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(call => _time.CreateTimer(call.Arg(), call.Arg(), call.ArgAt(2), call.ArgAt(3))); _handoff.EnvironmentAsync(Arg.Any(), Arg.Any?>(), Arg.Any()) .Returns(call => PassThrough(call.Arg?>())); _handoff.CompleteAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(Task.CompletedTask); @@ -87,6 +91,39 @@ public ServiceEnsurerTests() }); } + [Test] + public async Task EnsureAsync_Should_Poll_On_The_Injected_Clock() + { + var interval = TimeSpan.FromSeconds(1); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + SeedRegistered(); + var probes = 0; + _probe.ProbeAsync(Arg.Any(), Arg.Any()).Returns(_ => + { + entered.TrySetResult(); + return ++probes == 1 ? Waiting : Ready; + }); + using var caller = new CancellationTokenSource(); + var pending = Ensurer(Timing with { PollInterval = interval }).EnsureAsync(options: null, caller.Token); + try + { + await entered.Task.WaitAsync(TimeSpan.FromSeconds(10)); + await _time.WaitForArmedAsync(interval, TimeSpan.FromSeconds(10)); + _time.Advance(interval - TimeSpan.FromTicks(1)); + await Assert.That(probes).IsEqualTo(1); + await Assert.That(pending.IsCompleted).IsFalse(); + _time.Advance(TimeSpan.FromTicks(1)); + + var registration = await pending.WaitAsync(TimeSpan.FromSeconds(10)); + await Assert.That(registration.ProcessId).IsEqualTo(RegisteredPid); + await Assert.That(probes).IsEqualTo(2); + } + finally + { + await caller.CancelOnWorkerAsync(); + } + } + [Test] public async Task EnsureAsync_Should_Return_A_Ready_Compatible_Service_Without_Spawning() { @@ -319,7 +356,7 @@ public async Task EnsureAsync_Should_Reset_The_Spawn_Delay_When_A_Registered_Ser var reaped = false; var answered = false; SeedRegistered(); - _clock.UtcNow.Returns(_ => _now); + _clock.GetUtcNow().Returns(_ => _now); _probe.ProbeAsync(WithPid(RegisteredPid), Arg.Any()).Returns(_ => { _now += TimeSpan.FromSeconds(1); @@ -580,7 +617,7 @@ public async Task EnsureAsync_Should_Carry_The_Last_Swallowed_Replacement_Failur DeadlineAfter(clockReads: 40); var exception = await Assert - .That(async () => _ = await Ensurer().EnsureAsync( + .That(async () => _ = await EnsureWithStopPollsAsync( new OpenCodeServerEnsureOptions { ExpectedVersion = "2.0.4", VersionPolicy = OpenCodeServerVersionPolicy.Replace }, CancellationToken.None)) .Throws(); @@ -917,6 +954,25 @@ private ServiceEnsurer Ensurer(ServiceTiming? timing = null) => }), timing ?? Timing); + /// Preserves the scripted UTC reads while explicitly progressing the stop polls. + private async Task EnsureWithStopPollsAsync(OpenCodeServerEnsureOptions options, CancellationToken cancellationToken) + { + var pending = Ensurer().EnsureAsync(options, cancellationToken); + while (!pending.IsCompleted) + { + var armed = _time.WaitForArmedAsync(Timing.StopPollInterval, TimeSpan.FromSeconds(10)); + if (await Task.WhenAny(pending, armed) == pending) + { + break; + } + + await armed; + _time.Advance(Timing.StopPollInterval); + } + + return await pending; + } + private OpenCodeServerEnsureOptions Announcing(OpenCodeServerEnsureOptions? options = null) { options ??= new OpenCodeServerEnsureOptions(); @@ -928,7 +984,7 @@ private OpenCodeServerEnsureOptions Announcing(OpenCodeServerEnsureOptions? opti private void DeadlineAfter(int clockReads) { var reads = 0; - _clock.UtcNow.Returns(_ => ++reads > clockReads ? Start + Timing.PromiseTimeout : Start); + _clock.GetUtcNow().Returns(_ => ++reads > clockReads ? Start + Timing.PromiseTimeout : Start); } private static ServiceRegistration WithPid(int processId) => @@ -945,7 +1001,7 @@ private void AnswerRegistered(ServiceProbeResult result) => /// private void RefuseRegistered(TimeSpan step) { - _clock.UtcNow.Returns(_ => _now); + _clock.GetUtcNow().Returns(_ => _now); _probe.ProbeAsync(WithPid(RegisteredPid), Arg.Any()).Returns(_ => { _now += step; diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffScenario.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffScenario.cs index 89ed689f..9a76f13c 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffScenario.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffScenario.cs @@ -1,5 +1,4 @@ using System.Net; -using NSubstitute; using OpenCode.Sdk.Internal; using OpenCode.Sdk.Tests.Hosting.BackgroundService.Support; using OpenCode.Sdk.Tests.Support; @@ -25,14 +24,9 @@ internal sealed class ServicePtyHandoffScenario /// public static readonly TimeSpan RequestTimeout = ServiceTimingData.PatientRequestTimeout; - public ServicePtyHandoffScenario() - { - Clock.UtcNow.Returns(Now); - } - public MockFileSystem FileSystem { get; } = new(); - public IServiceClock Clock { get; } = Substitute.For(); + public ManualTimeProvider Clock { get; } = new(Now); public static ServiceRegistration Registration(Uri endpoint) => new(SourceId, Version, endpoint.ToString(), endpoint, RegisteredPid, Password); diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffTests.cs index 968c396a..6b551c15 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffTests.cs @@ -30,6 +30,75 @@ public sealed class ServicePtyHandoffTests private static readonly JsonElement Ticket = LoadTicket(); private readonly ServicePtyHandoffScenario _scenario = new(); + private static readonly TimeSpan HangGuard = TimeSpan.FromSeconds(10); + + [Test] + public async Task PrepareAsync_Should_Expire_The_Ticket_On_The_Injected_Clock() + { + _scenario.EnsureSidecarDirectory(); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await using var server = LoopbackHttpServer.Start(_ => Pending(entered)); + var shutdown = Substitute.For(); + shutdown.ShutdownAsync(Arg.Any(), Arg.Any()).Returns(Task.CompletedTask); + using var caller = new CancellationTokenSource(); + var registration = ServicePtyHandoffScenario.Registration(server.Endpoint); + var pending = new ServicePtyHandoff(new TestablyServiceFileSystem(_scenario.FileSystem), _scenario.Clock, shutdown) + .PrepareAsync(_scenario.RegistrationPath(), registration, RequestTimeout, caller.Token); + try + { + await entered.Task.WaitAsync(HangGuard); + await AdvanceToJustBeforeRequestTimeoutAsync(); + await Assert.That(pending.IsCompleted).IsFalse(); + _ = shutdown.DidNotReceiveWithAnyArgs().ShutdownAsync(default!, default); + _scenario.Clock.Advance(TimeSpan.FromTicks(1)); + await pending.WaitAsync(HangGuard); + + _ = shutdown.Received(1).ShutdownAsync(registration, Arg.Any()); + await AssertNullSidecarAfterExpiryAsync(); + } + finally + { + await caller.CancelOnWorkerAsync(); + server.ReleaseResponses(); + } + } + + [Test] + public async Task PrepareAsync_Should_Expire_The_Fallback_On_The_Injected_Clock() + { + _scenario.EnsureSidecarDirectory(); + await using var server = LoopbackHttpServer.Start(static _ => Json(HttpStatusCode.NotFound, "{}")); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var shutdown = Substitute.For(); + CancellationToken received = default; + shutdown.ShutdownAsync(Arg.Any(), Arg.Any()).Returns(async call => + { + received = call.Arg(); + var held = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var cancellation = received.Register(() => held.TrySetCanceled(received)); + entered.TrySetResult(); + await held.Task; + }); + using var caller = new CancellationTokenSource(); + var pending = new ServicePtyHandoff(new TestablyServiceFileSystem(_scenario.FileSystem), _scenario.Clock, shutdown) + .PrepareAsync(_scenario.RegistrationPath(), ServicePtyHandoffScenario.Registration(server.Endpoint), RequestTimeout, caller.Token); + try + { + await entered.Task.WaitAsync(HangGuard); + await AdvanceToJustBeforeRequestTimeoutAsync(); + await Assert.That(received.IsCancellationRequested).IsFalse(); + await Assert.That(pending.IsCompleted).IsFalse(); + _scenario.Clock.Advance(TimeSpan.FromTicks(1)); + await pending.WaitAsync(HangGuard); + + await Assert.That(received.IsCancellationRequested).IsTrue(); + await AssertNullSidecarAfterExpiryAsync(); + } + finally + { + await caller.CancelOnWorkerAsync(); + } + } [Test] public async Task PrepareAsync_Should_Publish_A_Sidecar_From_The_Handoff_Ticket() @@ -446,7 +515,7 @@ public async Task PrepareAsync_Should_Take_The_Fallback_When_The_Request_Bound_E var shutdown = new BoundOutlivingShutdown(); var registration = ServicePtyHandoffScenario.Registration(server.Endpoint); - await new ServicePtyHandoff(new TestablyServiceFileSystem(_scenario.FileSystem), _scenario.Clock, shutdown).PrepareAsync( + await new ServicePtyHandoff(new TestablyServiceFileSystem(_scenario.FileSystem), TimeProvider.System, shutdown).PrepareAsync( _scenario.RegistrationPath(), registration, TimeSpan.FromMilliseconds(200), CancellationToken.None); await Assert.That(_scenario.ReadSidecar()!.Handoff).IsNull(); @@ -571,6 +640,19 @@ public async Task EnvironmentAsync_Should_Adopt_When_The_Registration_Cannot_Be_ await Assert.That(overlay["OPENCODE_PTY_HANDOFF"]).IsEqualTo(Ticket.GetRawText()); } + private async Task AdvanceToJustBeforeRequestTimeoutAsync() + { + await _scenario.Clock.WaitForArmedAsync(RequestTimeout, HangGuard); + _scenario.Clock.Advance(RequestTimeout - TimeSpan.FromTicks(1)); + } + + private async Task AssertNullSidecarAfterExpiryAsync() + { + await Assert.That(_scenario.ReadSidecar()!.Handoff).IsNull(); + await Assert.That(_scenario.ReadSidecar()!.ExpiresAt) + .IsEqualTo(_scenario.Clock.GetUtcNow().ToUnixTimeMilliseconds() + 30_000d); + } + private static JsonElement LoadTicket() { using var document = JsonDocument.Parse(Fixtures.LoadJson("BackgroundService.pty-handoff-valid.json")); diff --git a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Stop/ServiceStopperTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Stop/ServiceStopperTests.cs index 2b26103e..20b355ff 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Stop/ServiceStopperTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Stop/ServiceStopperTests.cs @@ -6,6 +6,7 @@ using OpenCode.Sdk.Tests.Hosting.BackgroundService.Registration; using OpenCode.Sdk.Tests.Support; using Testably.Abstractions.Testing; +using TimeProvider = System.TimeProvider; namespace OpenCode.Sdk.Tests.Hosting.BackgroundService.Stop; @@ -42,7 +43,7 @@ public sealed class ServiceStopperTests private readonly IServiceInfoProbe _probe = Substitute.For(); private readonly IServicePtyShutdown _ptyShutdown = Substitute.For(); private readonly IServiceProcessControl _processControl = Substitute.For(); - private readonly IServiceClock _clock = Substitute.For(); + private readonly TimeProvider _clock = TimeProvider.System; public ServiceStopperTests() { @@ -55,6 +56,57 @@ public ServiceStopperTests() ProcessLeavesOn(ProcessSignal.Terminate); } + [Test] + [Arguments(false)] + [Arguments(true)] + public async Task TerminateAsync_Should_Poll_Each_Rung_On_The_Injected_Clock(bool killRung) + { + SeedModern(); + ProcessNeverLeaves(); + var time = new ManualTimeProvider(); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _processControl.TrySignal(Live, Arg.Any()).Returns(_ => + { + entered.TrySetResult(); + return true; + }); + var fileSystem = new TestablyServiceFileSystem(_fileSystem); + var registration = await ServiceRegistrationReader.TryReadAsync(fileSystem, SharedRegistrationPath(), CancellationToken.None); + var timing = Timing with { StopPollAttempts = 1, StopPollInterval = TimeSpan.FromSeconds(1) }; + using var caller = new CancellationTokenSource(); + var pending = new ServiceTerminator(fileSystem, _processControl, timing, time) + .TerminateAsync(registration!, SharedRegistrationPath(), caller.Token); + try + { + await entered.Task.WaitAsync(TimeSpan.FromSeconds(10)); + await time.WaitForArmedAsync(timing.StopPollInterval, TimeSpan.FromSeconds(10)); + if (killRung) + { + time.Advance(timing.StopPollInterval); + await time.WaitForArmedAsync(timing.StopPollInterval, TimeSpan.FromSeconds(10)); + _ = _processControl.Received(1).TrySignal(Live, ProcessSignal.Kill); + } + + time.Advance(timing.StopPollInterval - TimeSpan.FromTicks(1)); + await Assert.That(pending.IsCompleted).IsFalse(); + if (!killRung) + { + _ = _processControl.DidNotReceive().TrySignal(Live, ProcessSignal.Kill); + } + + // The next look sees the process gone; no real-time wait can decide this outcome. + ProcessIs(null); + time.Advance(TimeSpan.FromTicks(1)); + await pending.WaitAsync(TimeSpan.FromSeconds(10)); + await AssertSharedRegistrationAbsentAsync(); + _ = _processControl.Received(killRung ? 1 : 0).TrySignal(Live, ProcessSignal.Kill); + } + finally + { + await caller.CancelOnWorkerAsync(); + } + } + [Test] public async Task StopAsync_Should_Complete_And_Clear_The_Sidecar_When_The_Registration_Is_Missing() { @@ -423,7 +475,8 @@ public async Task StopAsync_Should_Still_Terminate_When_The_Sidecar_Cannot_Be_Re _ptyShutdown, new ServicePtyHandoff(fileSystem, _clock, _ptyShutdown), _processControl, - Timing); + Timing, + _clock); await stopper.StopAsync(options: null, CancellationToken.None); @@ -559,7 +612,8 @@ private ServiceStopper Stopper() => _ptyShutdown, new ServicePtyHandoff(new TestablyServiceFileSystem(_fileSystem), _clock, _ptyShutdown), _processControl, - Timing); + Timing, + _clock); private void Answer(ServiceProbeResult result) => _probe.ProbeAsync(Arg.Any(), Arg.Any()).Returns(result); diff --git a/tests/OpenCode.Sdk.Tests/Support/ManualTimeProvider.cs b/tests/OpenCode.Sdk.Tests/Support/ManualTimeProvider.cs index fa66dcfb..e96cf48d 100644 --- a/tests/OpenCode.Sdk.Tests/Support/ManualTimeProvider.cs +++ b/tests/OpenCode.Sdk.Tests/Support/ManualTimeProvider.cs @@ -15,7 +15,10 @@ internal sealed class ManualTimeProvider : TimeProvider private readonly List _timers = []; private readonly Channel _created = Channel.CreateUnbounded(); private readonly List<(DateTimeOffset Due, int Count, TaskCompletionSource Armed)> _armedWaiters = []; - private DateTimeOffset _now = new(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); + private DateTimeOffset _now; + + public ManualTimeProvider(DateTimeOffset? start = null) => + _now = start ?? new DateTimeOffset(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); public override TimeZoneInfo LocalTimeZone => TimeZoneInfo.Utc; diff --git a/tests/OpenCode.Sdk.Tests/Threading/TimedCancellationTests.cs b/tests/OpenCode.Sdk.Tests/Threading/TimedCancellationTests.cs new file mode 100644 index 00000000..570b6498 --- /dev/null +++ b/tests/OpenCode.Sdk.Tests/Threading/TimedCancellationTests.cs @@ -0,0 +1,130 @@ +using OpenCode.Sdk.Internal; +using OpenCode.Sdk.Tests.Support; + +namespace OpenCode.Sdk.Tests.Threading; + +public sealed class TimedCancellationTests +{ + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(2); + + [Test] + public async Task Create_Should_Disarm_The_Timer_When_Disposed_Before_The_Deadline() + { + var time = new ManualTimeProvider(); + using var deadline = TimedCancellation.Create(Timeout, time); + deadline.Dispose(); + + await Assert.That(time.ArmedIn(Timeout)).IsEqualTo(0); + deadline.Dispose(); + } + + [Test] + public async Task Create_Should_Cancel_At_The_Deadline_And_Not_One_Tick_Before() + { + var time = new ManualTimeProvider(); + using var deadline = TimedCancellation.Create(Timeout, time); + var cancellations = 0; + using var registration = deadline.Token.Register(() => cancellations++); + + time.Advance(Timeout - TimeSpan.FromTicks(1)); + await Assert.That(deadline.IsCancellationRequested).IsFalse(); + await Assert.That(cancellations).IsEqualTo(0); + + time.Advance(TimeSpan.FromTicks(1)); + await Assert.That(deadline.IsCancellationRequested).IsTrue(); + await Assert.That(cancellations).IsEqualTo(1); + } + +#if !NET + [Test] + public async Task Create_Should_Ignore_An_In_Flight_Timer_Callback_After_Disposal() + { + var time = new InFlightTimeProvider(); + using var deadline = TimedCancellation.Create(Timeout, time); + deadline.Dispose(); + + // A disposed deadline promises only that a late callback never throws; whether its token + // then reads as canceled is unspecified, as with the modern constructor. + await Assert.That(time.Fire).ThrowsNothing(); + } +#endif + + [Test] + public async Task Create_Should_Allow_Timer_Cancellation_To_Race_With_Disposal() + { + for (var round = 0; round < 1_000; round++) + { + var time = new InFlightTimeProvider(); + using var deadline = TimedCancellation.Create(Timeout, time); + using var barrier = new Barrier(2); + var firing = Task.Run(() => + { + var met = barrier.SignalAndWait(HangGuard.Bound); + time.Fire(); + return met; + }); + var disposing = Task.Run(() => + { + var met = barrier.SignalAndWait(HangGuard.Bound); + deadline.Dispose(); + return met; + }); + + var rendezvous = await Task.WhenAll(firing, disposing).WaitAsync(HangGuard.Bound); + await Assert.That(rendezvous[0]).IsTrue(); + await Assert.That(rendezvous[1]).IsTrue(); + } + } + + [Test] + public async Task Create_Should_Allow_A_Cancellation_Callback_To_Dispose_Its_Own_Deadline() + { + var time = new ManualTimeProvider(); + using var deadline = TimedCancellation.Create(Timeout, time); + var token = deadline.Token; + var callbacksCompleted = 0; + using var registration = token.Register(() => + { + deadline.Dispose(); + callbacksCompleted++; + }); + + await Task.Run(() => time.Advance(Timeout)).WaitAsync(HangGuard.Bound); + + await Assert.That(token.IsCancellationRequested).IsTrue(); + await Assert.That(callbacksCompleted).IsEqualTo(1); + await Assert.That(time.ArmedIn(TimeSpan.Zero)).IsEqualTo(0); + } + + /// Retains a queued callback even after disposal, as a thread-pool timer can. + private sealed class InFlightTimeProvider : TimeProvider + { + private Action? _fire; + + public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) + { + var timer = new InFlightTimer(callback, state); + _fire = timer.Fire; + return timer; + } + + public void Fire() => _fire!(); + + private sealed class InFlightTimer(TimerCallback callback, object? state) : ITimer + { + private bool _disposed; + + public bool Change(TimeSpan dueTime, TimeSpan period) => !_disposed; + + public void Fire() => callback(state); + + public void Dispose() => _disposed = true; + + public ValueTask DisposeAsync() + { + Dispose(); + return default; + } + } + } +} diff --git a/tests/Shared/ContenderLedger.cs b/tests/Shared/ContenderLedger.cs index 80685077..6fe58aea 100644 --- a/tests/Shared/ContenderLedger.cs +++ b/tests/Shared/ContenderLedger.cs @@ -17,7 +17,7 @@ namespace OpenCode.Sdk.Tests.Harness; /// The ledger file. internal sealed class ContenderLedger(IFileSystem fileSystem, string path) : IServiceContenderSpawner { - private readonly ServiceContenderSpawner _spawner = new(new PosixSpawn(), new WindowsSpawn()); + private readonly ServiceContenderSpawner _spawner = new(new PosixSpawn(), new WindowsSpawn(), TimeProvider.System); private readonly Lock _gate = new(); public IServiceContender Spawn(IServiceContenderSpawner.ContenderStartInfo startInfo) diff --git a/tools/OpenCode.Sdk.Tools/Generator/Binding/ReservedNamePolicy.cs b/tools/OpenCode.Sdk.Tools/Generator/Binding/ReservedNamePolicy.cs index 739ee812..4754e508 100644 --- a/tools/OpenCode.Sdk.Tools/Generator/Binding/ReservedNamePolicy.cs +++ b/tools/OpenCode.Sdk.Tools/Generator/Binding/ReservedNamePolicy.cs @@ -149,7 +149,6 @@ internal static class ReservedNamePolicy "IProcessGroupSignal", "IProcessJob", "IServerLadder", - "IServiceClock", "IServiceContender", "IServiceContenderSpawner", "IServiceEnvironment", @@ -239,7 +238,6 @@ internal static class ReservedNamePolicy "ServerSentEvent", "ServerSentEventFramer", "ServerSentEventReader", - "ServiceClock", "ServiceConfigReader", "ServiceContender", "ServiceContenderSpawner", @@ -292,6 +290,7 @@ internal static class ReservedNamePolicy "TerminalSocketBounds", "TerminalSocketCore", "TerminalSubmission", + "TimedCancellation", "TransportPolicy", "TreeKillOutcome", "UnionDiscriminatorReader",