From 36dfc63bdca04eb40ec687441ca6535207983f18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deniz=20=C4=B0rgin?= Date: Sat, 17 Oct 2026 14:57:12 +0300 Subject: [PATCH 1/2] refactor: the launcher runs on its seams' clock, and shipped src bans the system clock (RT-E2 C) PosixServerLadder, the pump release and the taskkill tree kill now measure their bounds on LauncherSeams.Time; BoundedWait loses its system-clock overload. BannedSymbols.Clock.txt (14 entries) applies to the two shipped src projects only, with five counted composition roots exempted narrowly. --- BannedSymbols.Clock.txt | 14 +++ Directory.Build.props | 4 + docs/architecture/client-runtime.md | 8 +- docs/architecture/platform-and-packaging.md | 2 + .../OpenCodeServer.BackgroundService.cs | 9 +- .../Platform/Windows/TaskkillTreeKill.cs | 5 +- .../Hosting/Launcher/Internal/BoundedWait.cs | 15 +-- .../Launcher/Internal/LauncherSeams.cs | 9 +- .../Launcher/Internal/OwnedServerChild.cs | 2 +- .../Launcher/Internal/PipeOutputPump.cs | 7 +- .../Launcher/Internal/PosixServerChild.cs | 2 +- .../Launcher/Internal/PosixServerLadder.cs | 20 ++-- .../Internal/Transport/Pipeline.cs | 2 + .../Launcher/OpenCodeServerLifecycleTests.cs | 4 +- .../Launcher/OpenCodeServerReadinessTests.cs | 7 ++ .../Hosting/Launcher/OwnedServerChildTests.cs | 41 +++++++ .../Launcher/PosixServerLadderTests.cs | 108 +++++++++++++++++- .../Platform/Windows/TaskkillTreeKillTests.cs | 51 ++++++++- ...Suppression_In_The_Repository.verified.txt | 5 +- 19 files changed, 275 insertions(+), 40 deletions(-) create mode 100644 BannedSymbols.Clock.txt diff --git a/BannedSymbols.Clock.txt b/BannedSymbols.Clock.txt new file mode 100644 index 00000000..bd765b2d --- /dev/null +++ b/BannedSymbols.Clock.txt @@ -0,0 +1,14 @@ +M:System.Threading.Tasks.Task.Delay(System.TimeSpan);Use the injected TimeProvider overload +M:System.Threading.Tasks.Task.Delay(System.TimeSpan,System.Threading.CancellationToken);Use the injected TimeProvider overload +M:System.Threading.Tasks.Task.Delay(System.Int32);Use the injected TimeProvider overload +M:System.Threading.Tasks.Task.Delay(System.Int32,System.Threading.CancellationToken);Use the injected TimeProvider overload +M:System.Threading.CancellationTokenSource.#ctor(System.TimeSpan);Use a cancellation deadline on the injected TimeProvider +M:System.Threading.CancellationTokenSource.#ctor(System.Int32);Use a cancellation deadline on the injected TimeProvider +M:System.Threading.CancellationTokenSource.CancelAfter(System.TimeSpan);Use a cancellation deadline on the injected TimeProvider +M:System.Threading.CancellationTokenSource.CancelAfter(System.Int32);Use a cancellation deadline on the injected TimeProvider +T:System.Diagnostics.Stopwatch;Use GetTimestamp and GetElapsedTime on the injected TimeProvider +P:System.DateTime.UtcNow;Use the injected TimeProvider (GetUtcNow or GetTimestamp) +P:System.DateTimeOffset.UtcNow;Use the injected TimeProvider (GetUtcNow or GetTimestamp) +P:System.Environment.TickCount;Use the injected TimeProvider (GetUtcNow or GetTimestamp) +P:System.Environment.TickCount64;Use the injected TimeProvider (GetUtcNow or GetTimestamp) +P:System.TimeProvider.System;Choose the clock only at a counted composition root and inject it elsewhere diff --git a/Directory.Build.props b/Directory.Build.props index 75980aa9..8aca4b91 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -118,6 +118,10 @@ + + + + diff --git a/docs/architecture/client-runtime.md b/docs/architecture/client-runtime.md index 4dba6f63..5c824e6e 100644 --- a/docs/architecture/client-runtime.md +++ b/docs/architecture/client-runtime.md @@ -617,8 +617,11 @@ completion; only the absence of those outcomes is a readiness timeout. An early- drain still gives the first readiness line priority. Cancellation arriving during cleanup does not replace the failure already selected. +Readiness, drain, both disposal ladders, background reap, reader release and tree-kill bounds run +on `LauncherSeams.Time`, chosen once by the standalone launcher's composition root. + Disposal is a ladder, bounded at every step so it never hangs the caller. On Windows it follows -upstream's rungs on upstream's clock: the grace (`GracefulShutdownTimeout`, default 3 seconds — the +upstream's rungs and timing: the grace (`GracefulShutdownTimeout`, default 3 seconds — the reference client's own force-kill window) starts with the first rung and bounds it. 1. A root that already exited is left alone: `taskkill` finds only live processes, so on an exited @@ -934,6 +937,9 @@ stamps read UTC, and handoff expiry reads UTC epoch milliseconds; neither is a m elapsed-time policy. Each request deadline arms after probe headers, after ticket pipeline construction, or at entry to the separate shutdown fallback, respectively. +`BannedSymbols.Clock.txt` bans the system clock in shipped src outside the five counted +composition roots: launcher, client pipeline, Discover, Stop and Ensure. + Where the SDK deliberately differs from the pinned chain, it differs here, each for a recorded reason (ADR-0031): diff --git a/docs/architecture/platform-and-packaging.md b/docs/architecture/platform-and-packaging.md index c74af6b6..fc37d999 100644 --- a/docs/architecture/platform-and-packaging.md +++ b/docs/architecture/platform-and-packaging.md @@ -46,6 +46,8 @@ document. A polyfilled member whose downlevel behaviour is a hazard is banned in `BannedSymbols.txt`, which names its replacement. `CancellationTokenSource.CancelAsync` is one: below .NET 8 it spins the calling worker until a queued `Cancel` starts, and concurrent callers stall the thread pool, so product and test code cancel through the internal `CancelOnWorkerAsync`. +The src-only `BannedSymbols.Clock.txt` enforces clock injection +([client-runtime.md](client-runtime.md)). ### WebSocket support diff --git a/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs b/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs index b20e9d78..62bdda1a 100644 --- a/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs +++ b/src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs @@ -21,8 +21,9 @@ public partial class OpenCodeServer OpenCodeServerDiscoverOptions? options = null, CancellationToken cancellationToken = default) { - // The discovery composition root chooses the clock for its info probe. +#pragma warning disable RS0030 // The discovery composition root chooses the system clock and passes it to its info probe. var time = TimeProvider.System; +#pragma warning restore RS0030 return DiscoverWithSeamsAsync(options, PlatformProbe(ServiceTiming.Default, time), cancellationToken); } @@ -86,8 +87,9 @@ public static Task StopAsync( CancellationToken cancellationToken = default) { var timing = ServiceTiming.Default; - // The stop composition root shares one clock across its probe, handoff and terminator. +#pragma warning disable RS0030 // The stop composition root chooses the system clock; its probe, handoff and terminator share it. var time = TimeProvider.System; +#pragma warning restore RS0030 var fileSystem = new ServiceFileSystem(); var ptyShutdown = new ServicePtyShutdown(); var stopper = new ServiceStopper( @@ -120,8 +122,9 @@ public static Task EnsureAsync( OpenCodeServerEnsureOptions? options = null, CancellationToken cancellationToken = default) { - // The ensure composition root shares one clock across the election and its contenders. +#pragma warning disable RS0030 // The ensure composition root chooses the system clock; election collaborators and spawned contenders receive it. var time = TimeProvider.System; +#pragma warning restore RS0030 return EnsureWithSeamsAsync(options, ServiceTiming.Default, new ServiceContenderSpawner(new PosixSpawn(), new WindowsSpawn(), time), time, cancellationToken); } diff --git a/src/OpenCode.Sdk/Hosting/Internal/Platform/Windows/TaskkillTreeKill.cs b/src/OpenCode.Sdk/Hosting/Internal/Platform/Windows/TaskkillTreeKill.cs index e45e3cf0..89e2146f 100644 --- a/src/OpenCode.Sdk/Hosting/Internal/Platform/Windows/TaskkillTreeKill.cs +++ b/src/OpenCode.Sdk/Hosting/Internal/Platform/Windows/TaskkillTreeKill.cs @@ -15,7 +15,8 @@ namespace OpenCode.Sdk.Internal; /// /// The spawn the kill runs through. /// The exit watch the kill's own exit is awaited through. -internal sealed class TaskkillTreeKill(IWindowsSpawn spawn, IWindowsChildExit exits) : IWindowsTreeKill +/// The clock for the supplied tree-kill bound. +internal sealed class TaskkillTreeKill(IWindowsSpawn spawn, IWindowsChildExit exits, TimeProvider time) : IWindowsTreeKill { /// /// Gets the absolute path to the system taskkill.exe. The system folder is read from the @@ -59,7 +60,7 @@ public async Task KillTreeAsync(int processId, TimeSpan bound) using (watch) { - if (!await BoundedWait.CompletesWithinAsync(watch.Exited, bound).ConfigureAwait(false)) + if (!await BoundedWait.CompletesWithinAsync(watch.Exited, bound, time).ConfigureAwait(false)) { // Ended, not awaited: the watch releases its handle once it is gone, and the // caller's own bound is not stretched by the time that takes. diff --git a/src/OpenCode.Sdk/Hosting/Launcher/Internal/BoundedWait.cs b/src/OpenCode.Sdk/Hosting/Launcher/Internal/BoundedWait.cs index 20644abf..019fb4db 100644 --- a/src/OpenCode.Sdk/Hosting/Launcher/Internal/BoundedWait.cs +++ b/src/OpenCode.Sdk/Hosting/Launcher/Internal/BoundedWait.cs @@ -3,20 +3,11 @@ namespace OpenCode.Sdk.Internal; /// Waits for a task under a bound, reporting an expired bound instead of raising it. internal static class BoundedWait { - /// - /// Waits at most for to end. A task that - /// faulted or was canceled has ended too: the wait reports that and raises nothing, and how the - /// task ended stays on the task for its owner to read. - /// - /// The awaited task. - /// How long the caller waits. - /// True when the task ended inside the bound, however it ended. - public static Task CompletesWithinAsync(Task task, TimeSpan bound) => - CompletesWithinAsync(task, bound, TimeProvider.System); - /// /// Waits at most , measured on , for - /// to end, as does. + /// to end. A faulted or canceled task has ended too, as + /// reports: the wait raises nothing, and how the task ended + /// stays on the task for its owner to read. /// /// The awaited task. /// How long the caller waits. diff --git a/src/OpenCode.Sdk/Hosting/Launcher/Internal/LauncherSeams.cs b/src/OpenCode.Sdk/Hosting/Launcher/Internal/LauncherSeams.cs index 6d9d4fa9..45c22b7c 100644 --- a/src/OpenCode.Sdk/Hosting/Launcher/Internal/LauncherSeams.cs +++ b/src/OpenCode.Sdk/Hosting/Launcher/Internal/LauncherSeams.cs @@ -31,7 +31,7 @@ internal sealed record LauncherSeams /// Gets the POSIX exit status. public required IChildExitStatus Exits { get; init; } - /// Gets the clock for the readiness bound and the server's early-end drain. + /// Gets the clock for readiness, drain, both disposal ladders, background reap, reader release, and tree-kill bounds. public required TimeProvider Time { get; init; } /// Gets an optional token-bound readiness wait substitution; null uses the ordinary task wait. @@ -49,16 +49,19 @@ public static LauncherSeams ForCurrentProcess() { var windowsSpawn = new WindowsSpawn(); var windowsExits = new WindowsChildExit(); +#pragma warning disable RS0030 // The standalone launcher composition root chooses the system clock; child bounds receive this provider. + var time = TimeProvider.System; +#pragma warning restore RS0030 return new LauncherSeams { WindowsSpawn = windowsSpawn, Job = KillOnCloseJob.ForCurrentProcess, WindowsExits = windowsExits, - TreeKill = new TaskkillTreeKill(windowsSpawn, windowsExits), + TreeKill = new TaskkillTreeKill(windowsSpawn, windowsExits, time), Spawn = new PosixSpawn(), Signals = new ProcessGroupSignal(), Exits = new ChildExitStatusReader(), - Time = TimeProvider.System, + Time = time, HostEnvironment = SnapshotEnvironment(), }; } diff --git a/src/OpenCode.Sdk/Hosting/Launcher/Internal/OwnedServerChild.cs b/src/OpenCode.Sdk/Hosting/Launcher/Internal/OwnedServerChild.cs index f7a0b32c..cc28c1dc 100644 --- a/src/OpenCode.Sdk/Hosting/Launcher/Internal/OwnedServerChild.cs +++ b/src/OpenCode.Sdk/Hosting/Launcher/Internal/OwnedServerChild.cs @@ -195,7 +195,7 @@ public async ValueTask DisposeAsync() await lease.DisposeAsync().ConfigureAwait(false); } - await _process.Output.ReleaseAsync().ConfigureAwait(false); + await _process.Output.ReleaseAsync(_time).ConfigureAwait(false); } finally { diff --git a/src/OpenCode.Sdk/Hosting/Launcher/Internal/PipeOutputPump.cs b/src/OpenCode.Sdk/Hosting/Launcher/Internal/PipeOutputPump.cs index c89bbaa6..e7e082d2 100644 --- a/src/OpenCode.Sdk/Hosting/Launcher/Internal/PipeOutputPump.cs +++ b/src/OpenCode.Sdk/Hosting/Launcher/Internal/PipeOutputPump.cs @@ -13,7 +13,7 @@ internal sealed class PipeOutputPump /// covers a runtime whose pipe read neither the token nor the closed stream reaches, and a /// release never hangs its caller. /// - private static readonly TimeSpan ReleaseBound = TimeSpan.FromSeconds(5); + internal static readonly TimeSpan ReleaseBound = TimeSpan.FromSeconds(5); private readonly PipeLineReader _standardOutput; private readonly PipeLineReader? _standardError; @@ -65,8 +65,9 @@ public async Task DrainAsync(TimeSpan bound, TimeProvider time) /// Ends every reader, whether or not its pipe reached end-of-stream, and closes the pipes. /// After it completes no reader is pending. /// + /// The owned child's clock for the release bound. /// A task that completes once the readers are released. - public async Task ReleaseAsync() + public async Task ReleaseAsync(TimeProvider time) { await _standardOutput.DisposeAsync().ConfigureAwait(false); if (_standardError is not null) @@ -74,7 +75,7 @@ public async Task ReleaseAsync() await _standardError.DisposeAsync().ConfigureAwait(false); } - _ = await BoundedWait.CompletesWithinAsync(ReadersEnded, ReleaseBound).ConfigureAwait(false); + _ = await BoundedWait.CompletesWithinAsync(ReadersEnded, ReleaseBound, time).ConfigureAwait(false); } /// Whether an ended reader reached end-of-stream; one whose line handler failed did not. diff --git a/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerChild.cs b/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerChild.cs index 03b0a526..826c752b 100644 --- a/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerChild.cs +++ b/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerChild.cs @@ -106,7 +106,7 @@ private static async Task AdoptAsync( ?? throw new InvalidOperationException("The server's standard output was routed to a pipe, but the spawn returned no read end."); var pump = PipeOutputPump.Start(standardOutput, spawned.StandardError, start.OnStandardOutput, start.OnStandardError); watch = PosixChildExitWatch.Start(spawned.ProcessId, seams.Exits, seams.Signals, childrenReapedAutomatically); - var ladder = new PosixServerLadder(spawned.ProcessId, watch, seams.Signals, PosixServerLadder.ForcedExitTimeout); + var ladder = new PosixServerLadder(spawned.ProcessId, watch, seams.Signals, PosixServerLadder.ForcedExitTimeout, seams.Time); return new OwnedServerChild( new OwnedServerProcess { diff --git a/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerLadder.cs b/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerLadder.cs index c2d5ffb1..7b86cc8f 100644 --- a/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerLadder.cs +++ b/src/OpenCode.Sdk/Hosting/Launcher/Internal/PosixServerLadder.cs @@ -1,5 +1,3 @@ -using System.Diagnostics; - namespace OpenCode.Sdk.Internal; /// @@ -33,21 +31,25 @@ internal sealed class PosixServerLadder : IServerLadder private readonly PosixChildExitWatch _watch; private readonly IProcessGroupSignal _signals; private readonly TimeSpan _forcedExitTimeout; + private readonly TimeProvider _time; /// Initializes the ladder for one server. /// The server's pid, which names its group. /// The server's exit watch, which also guards the pid fallback. /// The signal seam. /// How long the last rung waits; the shipped value is . - public PosixServerLadder(int processGroupId, PosixChildExitWatch watch, IProcessGroupSignal signals, TimeSpan forcedExitTimeout) + /// The clock for the shared grace and forced-root wait. + public PosixServerLadder(int processGroupId, PosixChildExitWatch watch, IProcessGroupSignal signals, TimeSpan forcedExitTimeout, TimeProvider time) { ArgumentNullException.ThrowIfNull(watch); ArgumentNullException.ThrowIfNull(signals); + ArgumentNullException.ThrowIfNull(time); _processGroupId = processGroupId; _watch = watch; _signals = signals; _forcedExitTimeout = forcedExitTimeout; + _time = time; } /// Runs the ladder; bounded by the grace plus the forced-exit wait. @@ -92,7 +94,7 @@ private bool TrySignal(ProcessSignal signal) => private async Task GroupEndedWithinAsync(TimeSpan grace) { - var elapsed = Stopwatch.StartNew(); + var started = _time.GetTimestamp(); if (!await RootExitedWithinAsync(grace).ConfigureAwait(false)) { return false; @@ -101,13 +103,17 @@ private async Task GroupEndedWithinAsync(TimeSpan grace) var interval = FirstProbeInterval; while (_signals.ProbeGroup(_processGroupId) is not SignalDelivery.NoSuchTarget) { - var remaining = grace - elapsed.Elapsed; + var remaining = grace - _time.GetElapsedTime(started); if (remaining <= TimeSpan.Zero) { return false; } - await Task.Delay(Shorter(interval, remaining)).ConfigureAwait(false); +#if NET + await Task.Delay(Shorter(interval, remaining), _time).ConfigureAwait(false); +#else + await _time.Delay(Shorter(interval, remaining)).ConfigureAwait(false); +#endif interval = Shorter(interval + interval, LongestProbeInterval); } @@ -118,7 +124,7 @@ private async Task RootExitedWithinAsync(TimeSpan window) { try { - _ = await _watch.Exited.WaitAsync(window).ConfigureAwait(false); + _ = await _watch.Exited.WaitAsync(window, _time).ConfigureAwait(false); return true; } catch (TimeoutException) diff --git a/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs b/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs index 81d42784..1ed99668 100644 --- a/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs +++ b/src/OpenCode.Sdk/Internal/Transport/Pipeline.cs @@ -69,7 +69,9 @@ internal Pipeline(HttpClient httpClient, bool ownsHttpClient, OpenCodeClientOpti _framer = framer ?? new ServerSentEventFramer(); _clientNetworkTimeout = networkTimeout ?? options.NetworkTimeout; _eventStreamIdleWindow = options.EventStreamIdleTimeout ?? StreamReconnect.DefaultIdleWindow; +#pragma warning disable RS0030 // The client pipeline composition root chooses the system clock only when no internal provider was supplied. _time = time ?? TimeProvider.System; +#pragma warning restore RS0030 _lifetimeToken = _lifetime.Token; // The options are read exactly once, here: the policies hold an immutable snapshot, diff --git a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerLifecycleTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerLifecycleTests.cs index 87d67c3d..dc0d68bf 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerLifecycleTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerLifecycleTests.cs @@ -635,7 +635,7 @@ public async Task DisposeAsync_Should_Return_Promptly_When_A_Descendant_Holds_St holderRunningAtClose = ProcessObservation.IsRunning(holderId.Value); var clock = Stopwatch.StartNew(); disposal = server.DisposeAsync().AsTask(); - closed = await BoundedWait.CompletesWithinAsync(disposal, CloseHangGuard); + closed = await BoundedWait.CompletesWithinAsync(disposal, CloseHangGuard, TimeProvider.System); elapsed = clock.Elapsed; if (closed) { @@ -667,7 +667,7 @@ public async Task DisposeAsync_Should_Return_Promptly_When_A_Descendant_Holds_St else if (disposal is { IsCompleted: false }) { // A close still waiting for end-of-stream gets it from the holder's end and finishes. - _ = await BoundedWait.CompletesWithinAsync(disposal, CloseHangGuard); + _ = await BoundedWait.CompletesWithinAsync(disposal, CloseHangGuard, TimeProvider.System); } } diff --git a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerReadinessTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerReadinessTests.cs index 0b1e790a..e48bacd4 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerReadinessTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OpenCodeServerReadinessTests.cs @@ -298,6 +298,13 @@ private static async Task AdvanceEarlyEndAsync( await draining.WaitAsync(SdkTestSupport.HangGuard.Bound, cancellationToken); time.Advance(OwnedServerChild.EarlyEndBound); } + + var releasing = time.WaitForArmedAsync(PipeOutputPump.ReleaseBound, SdkTestSupport.HangGuard.Bound, count: 1); + if (await Task.WhenAny(Task.WhenAll(startingTasks), releasing).WaitAsync(SdkTestSupport.HangGuard.Bound, cancellationToken) == releasing) + { + await releasing.WaitAsync(SdkTestSupport.HangGuard.Bound, cancellationToken); + time.Advance(PipeOutputPump.ReleaseBound); + } } private static LauncherSeams ObserveSpawn( diff --git a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OwnedServerChildTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OwnedServerChildTests.cs index 181b3ff3..1891dc04 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OwnedServerChildTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/OwnedServerChildTests.cs @@ -345,6 +345,47 @@ public async Task ResolveRecordedReadinessAsync_Should_Drain_Closed_Stdout_Only_ } } + [Test] + public async Task DisposeAsync_Should_Bound_Pump_Release_On_The_Owned_Clock() + { + var time = new ManualTimeProvider(); + using var readerGate = new ManualResetEventSlim(); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var stdout = new MemoryStream(Encoding.UTF8.GetBytes(BodyLine + "\n")); + var pump = PipeOutputPump.Start(stdout, null, line => + { + _ = entered.TrySetResult(); + readerGate.Wait(); + }, IgnoreLine); + var exited = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var child = ReadinessChild(pump, exited, Substitute.For(), time); + Task? releasing = null; + try + { + await entered.Task.WaitAsync(HangGuard.Bound); + releasing = child.DisposeAsync().AsTask(); + _ = await Task.WhenAny(releasing, time.WaitForArmedAsync(PipeOutputPump.ReleaseBound, HangGuard.Bound)); + await Assert.That(time.ArmedIn(PipeOutputPump.ReleaseBound)).IsEqualTo(1); + time.Advance(PipeOutputPump.ReleaseBound - TimeSpan.FromMilliseconds(1)); + await Assert.That(releasing.IsCompleted).IsFalse(); + time.Advance(TimeSpan.FromMilliseconds(1)); + await releasing.WaitAsync(HangGuard.Bound); + await Assert.That(pump.ReadersEnded.IsCompleted).IsFalse(); + } + finally + { + readerGate.Set(); + await pump.ReadersEnded.WaitAsync(HangGuard.Bound); + if (releasing is not null) + { + await releasing.WaitAsync(HangGuard.Bound); + } + + await child.DisposeAsync(); + _ = await child.Closed.WaitAsync(HangGuard.Bound); + } + } + private static async Task ClosedStdoutChildAsync( TaskCompletionSource exited, IServerLadder ladder, TimeProvider time) { diff --git a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/PosixServerLadderTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/PosixServerLadderTests.cs index 00913292..a7bf2a98 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/Launcher/PosixServerLadderTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/Launcher/PosixServerLadderTests.cs @@ -2,6 +2,7 @@ using NSubstitute; using OpenCode.Sdk.Internal; using OpenCode.Sdk.Tests.Support; +using TUnit.Assertions.Enums; namespace OpenCode.Sdk.Tests.Hosting.Launcher; @@ -238,6 +239,111 @@ public async Task RunAsync_Should_Fall_Back_To_The_Pid_When_The_Group_Refuses_Th await Assert.That(await ChildExitObservation.WithinAsync(watch, ExitObservationGuard)).IsEqualTo(Terminated); } + [Test] + public async Task RunAsync_Should_Spend_The_Root_Grace_On_The_Supplied_Clock() + { + var time = new ManualTimeProvider(); + using var exits = new ScriptedChildExit(); + var signals = Signals(exits, exitOnTerminate: false); + var watch = PosixChildExitWatch.Start(Pid, exits, signals, childrenReapedAutomatically: false); + var ladder = new PosixServerLadder(Pid, watch, signals, PosixServerLadder.ForcedExitTimeout, time); + var running = ladder.RunAsync(LongGrace); + try + { + await time.WaitForArmedAsync(LongGrace, HangGuard.Bound); + await Assert.That(time.ArmedIn(LongGrace)).IsEqualTo(1); + time.Advance(LongGrace - TimeSpan.FromTicks(1)); + _ = signals.DidNotReceive().SignalGroup(Pid, ProcessSignal.Kill); + time.Advance(TimeSpan.FromTicks(1)); + await running.WaitAsync(HangGuard.Bound); + await Assert.That(GroupSignals(signals)).IsEquivalentTo( + [ProcessSignal.Terminate, ProcessSignal.Kill], CollectionOrdering.Matching); + await Assert.That(await ChildExitObservation.WithinAsync(watch, ExitObservationGuard)).IsEqualTo(Killed); + } + finally + { + exits.Exit(Killed); + _ = await ChildExitObservation.WithinAsync(watch, ExitObservationGuard); + time.Advance(LongGrace + PosixServerLadder.ForcedExitTimeout); + await running.WaitAsync(HangGuard.Bound); + } + } + + [Test] + public async Task RunAsync_Should_Probe_Only_For_The_Grace_Remaining_On_The_Supplied_Clock() + { + var time = new ManualTimeProvider(); + using var exits = new ScriptedChildExit(); + var signals = Signals(exits); + var watch = PosixChildExitWatch.Start(Pid, exits, signals, childrenReapedAutomatically: false); + exits.Exit(new ChildExitStatus { ExitCode = 3 }); + _ = await ChildExitObservation.WithinAsync(watch, ExitObservationGuard); + var groupAlive = true; + var firstProbe = true; + _ = signals.ProbeGroup(Pid).Returns(_ => + { + if (firstProbe) + { + firstProbe = false; + time.Advance(LongGrace - TimeSpan.FromMilliseconds(1)); + } + + return groupAlive ? SignalDelivery.Delivered : SignalDelivery.NoSuchTarget; + }); + var ladder = new PosixServerLadder(Pid, watch, signals, PosixServerLadder.ForcedExitTimeout, time); + var running = ladder.RunAsync(LongGrace); + try + { + await time.WaitForArmedAsync(TimeSpan.FromMilliseconds(1), HangGuard.Bound); + await Assert.That(time.ArmedIn(TimeSpan.FromMilliseconds(1))).IsEqualTo(1); + _ = signals.DidNotReceive().SignalGroup(Pid, ProcessSignal.Kill); + time.Advance(TimeSpan.FromMilliseconds(1)); + await running.WaitAsync(HangGuard.Bound); + await Assert.That(GroupSignals(signals)).IsEquivalentTo( + [ProcessSignal.Terminate, ProcessSignal.Kill], CollectionOrdering.Matching); + } + finally + { + groupAlive = false; + time.Advance(LongGrace); + await running.WaitAsync(HangGuard.Bound); + } + } + + [Test] + public async Task RunAsync_Should_Bound_The_Forced_Root_Wait_On_The_Supplied_Clock() + { + var time = new ManualTimeProvider(); + using var exits = new ScriptedChildExit(); + var signals = Substitute.For(); + _ = signals.SignalGroup(Pid, Arg.Any()).Returns(SignalDelivery.Delivered); + var watch = PosixChildExitWatch.Start(Pid, exits, signals, childrenReapedAutomatically: false); + var ladder = new PosixServerLadder(Pid, watch, signals, PosixServerLadder.ForcedExitTimeout, time); + var running = ladder.RunAsync(TimeSpan.Zero); + try + { + await time.WaitForArmedAsync(PosixServerLadder.ForcedExitTimeout, HangGuard.Bound); + await Assert.That(time.ArmedIn(PosixServerLadder.ForcedExitTimeout)).IsEqualTo(1); + time.Advance(PosixServerLadder.ForcedExitTimeout - TimeSpan.FromMilliseconds(1)); + await Assert.That(running.IsCompleted).IsFalse(); + time.Advance(TimeSpan.FromMilliseconds(1)); + await running.WaitAsync(HangGuard.Bound); + await Assert.That(watch.Exited.IsCompleted).IsFalse(); + await Assert.That(GroupSignals(signals)).IsEquivalentTo( + [ProcessSignal.Terminate, ProcessSignal.Kill], CollectionOrdering.Matching); + } + finally + { + exits.Exit(Killed); + _ = await ChildExitObservation.WithinAsync(watch, ExitObservationGuard); + await running.WaitAsync(HangGuard.Bound); + } + } + + private static IEnumerable GroupSignals(IProcessGroupSignal signals) => + signals.ReceivedCalls().Where(call => call.GetMethodInfo().Name == nameof(IProcessGroupSignal.SignalGroup)) + .Select(call => call.GetArguments().OfType().Single()); + /// /// A substitute whose group and pid signals reach the scripted root: SIGTERM ends it with /// signal 15 unless is false, and SIGKILL ends it with @@ -268,6 +374,6 @@ SignalDelivery Deliver(ProcessSignal signal) private static PosixServerLadder Ladder(ScriptedChildExit exits, IProcessGroupSignal signals, out PosixChildExitWatch watch) { watch = PosixChildExitWatch.Start(Pid, exits, signals, childrenReapedAutomatically: false); - return new PosixServerLadder(Pid, watch, signals, PosixServerLadder.ForcedExitTimeout); + return new PosixServerLadder(Pid, watch, signals, PosixServerLadder.ForcedExitTimeout, TimeProvider.System); } } diff --git a/tests/OpenCode.Sdk.Tests/Hosting/Platform/Windows/TaskkillTreeKillTests.cs b/tests/OpenCode.Sdk.Tests/Hosting/Platform/Windows/TaskkillTreeKillTests.cs index 2faada21..a4f50d19 100644 --- a/tests/OpenCode.Sdk.Tests/Hosting/Platform/Windows/TaskkillTreeKillTests.cs +++ b/tests/OpenCode.Sdk.Tests/Hosting/Platform/Windows/TaskkillTreeKillTests.cs @@ -1,4 +1,6 @@ using System.Diagnostics; +using Microsoft.Win32.SafeHandles; +using NSubstitute; using OpenCode.Sdk.Internal; using OpenCode.Sdk.Tests.Harness; using OpenCode.Sdk.Tests.Support; @@ -92,7 +94,7 @@ public async Task KillTreeAsync_Should_Report_Failed_When_The_Root_Already_Exite public async Task KillTreeAsync_Should_Run_The_System_Taskkill_With_No_Handle_Of_The_Host(CancellationToken cancellationToken) { var recording = new RecordingSpawn(); - var outcome = await new TaskkillTreeKill(recording, new WindowsChildExit()).KillTreeAsync(4242, Bound).WaitAsync(cancellationToken); + var outcome = await new TaskkillTreeKill(recording, new WindowsChildExit(), TimeProvider.System).KillTreeAsync(4242, Bound).WaitAsync(cancellationToken); var request = recording.Requests.Single(); await Assert.That(outcome).IsEqualTo(TreeKillOutcome.Failed); @@ -121,7 +123,7 @@ public async Task KillTreeAsync_Should_End_A_Run_That_Outlives_Its_Bound(Cancell var bound = TimeSpan.FromMilliseconds(500); var run = Stopwatch.StartNew(); - var outcome = await new TaskkillTreeKill(hanging, new WindowsChildExit()).KillTreeAsync(4242, bound); + var outcome = await new TaskkillTreeKill(hanging, new WindowsChildExit(), TimeProvider.System).KillTreeAsync(4242, bound); var elapsed = run.Elapsed; await Assert.That(outcome).IsEqualTo(TreeKillOutcome.TimedOut); @@ -130,7 +132,50 @@ public async Task KillTreeAsync_Should_End_A_Run_That_Outlives_Its_Bound(Cancell await Assert.That(hanging.Held!.ExitCode).IsEqualTo(1); } - private static TaskkillTreeKill Kill() => new(new WindowsSpawn(), new WindowsChildExit()); + [Test] + [Timeout(120_000)] + public async Task KillTreeAsync_Should_Run_The_Supplied_Bound_On_The_Supplied_Clock(CancellationToken cancellationToken) + { + if (!OperatingSystem.IsWindows()) + { + BranchReport.Print("POSIX — the launcher ends its server through its process group, never a tree kill"); + return; + } + + var time = new ManualTimeProvider(); + var hanging = new LingeringSpawn(); + var watching = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var exits = Substitute.For(); + _ = exits.Watch(Arg.Any()).Returns(call => + { + var watch = new WindowsChildExit().Watch(call.Arg()); + _ = watching.TrySetResult(); + return watch; + }); + var killing = new TaskkillTreeKill(hanging, exits, time).KillTreeAsync(4242, Bound); + try + { + await Assert.That(killing.IsCompleted).IsFalse(); + await watching.Task.WaitAsync(cancellationToken); + await time.WaitForArmedAsync(Bound, HangGuard.Bound); + await Assert.That(time.ArmedIn(Bound)).IsEqualTo(1); + time.Advance(Bound - TimeSpan.FromMilliseconds(1)); + await Assert.That(killing.IsCompleted).IsFalse(); + time.Advance(TimeSpan.FromMilliseconds(1)); + await Assert.That(await killing.WaitAsync(cancellationToken)).IsEqualTo(TreeKillOutcome.TimedOut); + // The watch starts only after LingeringSpawn holds the spawned process's native handle. + var held = hanging.Held!; + await Assert.That(await ProcessObservation.ObserveExitWithinAsync(held, Bound, cancellationToken)).IsTrue(); + await Assert.That(held.ExitCode).IsEqualTo(1); + } + finally + { + hanging.Dispose(); + _ = await killing.WaitAsync(cancellationToken); + } + } + + private static TaskkillTreeKill Kill() => new(new WindowsSpawn(), new WindowsChildExit(), TimeProvider.System); /// A spawn that records each request and starts nothing. private sealed class RecordingSpawn : IWindowsSpawn diff --git a/tests/OpenCode.Sdk.Tools.Tests/Snapshots/SuppressionLedgerTests.Ledger_Should_Count_Every_Suppression_In_The_Repository.verified.txt b/tests/OpenCode.Sdk.Tools.Tests/Snapshots/SuppressionLedgerTests.Ledger_Should_Count_Every_Suppression_In_The_Repository.verified.txt index c0b4f091..48a78ce4 100644 --- a/tests/OpenCode.Sdk.Tools.Tests/Snapshots/SuppressionLedgerTests.Ledger_Should_Count_Every_Suppression_In_The_Repository.verified.txt +++ b/tests/OpenCode.Sdk.Tools.Tests/Snapshots/SuppressionLedgerTests.Ledger_Should_Count_Every_Suppression_In_The_Repository.verified.txt @@ -5,7 +5,7 @@ build-switch: 12 editorconfig: 158 globalconfig: 1 no-warn: 18 -pragma: 21 +pragma: 26 slopwatch-ignore: 7 slopwatch-suppress: 31 @@ -201,12 +201,15 @@ no-warn tests/OpenCode.Sdk.Tests/OpenCode.Sdk.Tests.csproj OPENCODE003 pragma src/OpenCode.Sdk/Common/Internal/ExperimentalDiagnostics.cs S1075 pragma src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Handoff/PtyHandoffSidecar.cs MA0045 pragma src/OpenCode.Sdk/Hosting/BackgroundService/Internal/Registration/StrictJson.cs S1244 +pragma src/OpenCode.Sdk/Hosting/BackgroundService/OpenCodeServer.BackgroundService.cs RS0030 x3 pragma src/OpenCode.Sdk/Hosting/Internal/Platform/Posix/PosixChildExitWatch.cs VSTHRD003 pragma src/OpenCode.Sdk/Hosting/Internal/Platform/Windows/WindowsExitWatch.cs S6640 pragma src/OpenCode.Sdk/Hosting/Internal/Platform/Windows/WindowsExitWatch.cs VSTHRD003 +pragma src/OpenCode.Sdk/Hosting/Launcher/Internal/LauncherSeams.cs RS0030 pragma src/OpenCode.Sdk/Hosting/Launcher/Internal/PipeLineReader.cs VSTHRD003 pragma src/OpenCode.Sdk/Hosting/Launcher/Internal/WindowsServerChild.cs MA0045 pragma src/OpenCode.Sdk/Internal/Terminal/TerminalReceivePump.cs VSTHRD003 +pragma src/OpenCode.Sdk/Internal/Transport/Pipeline.cs RS0030 pragma tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Ensure/ServiceEnsurerTests.cs MA0045 pragma tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffScenario.cs MA0045 pragma tests/OpenCode.Sdk.Tests/Hosting/BackgroundService/Handoff/ServicePtyHandoffTests.cs MA0045 From 28eb395a3c9571abc05cc5af3e4981c535dac076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deniz=20=C4=B0rgin?= Date: Sat, 17 Oct 2026 14:57:12 +0300 Subject: [PATCH 2/2] chore: the local-time bans name the injected clock for shipped src --- BannedSymbols.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BannedSymbols.txt b/BannedSymbols.txt index 796426b3..d4567b89 100644 --- a/BannedSymbols.txt +++ b/BannedSymbols.txt @@ -1,6 +1,6 @@ # https://github.com/dotnet/roslyn-analyzers/blob/master/src/Microsoft.CodeAnalysis.BannedApiAnalyzers/BannedApiAnalyzers.Help.md -P:System.DateTime.Now;Use System.DateTime.UtcNow instead -P:System.DateTimeOffset.Now;Use System.DateTimeOffset.UtcNow instead +P:System.DateTime.Now;Use the injected TimeProvider (src) or DateTime.UtcNow (tests) +P:System.DateTimeOffset.Now;Use the injected TimeProvider (src) or DateTimeOffset.UtcNow (tests) P:System.DateTimeOffset.DateTime;Use System.DateTimeOffset.UtcDateTime instead M:Polyfills.Polyfill.CancelAsync(System.Threading.CancellationTokenSource);Spins a worker until a queued Cancel starts and stalls the pool under concurrency; use OpenCode.Sdk.Internal.CancellationTokenSourceExtensions.CancelOnWorkerAsync M:Polyfills.Polyfill.Kill(System.Diagnostics.Process,System.Boolean);Polyfill walks the tree itself: on Unix it shells out to ps, kills only the root without an error when ps fails, and SIGKILLs descendants by bare pid; end a tree with taskkill /T (OpenCode.Sdk.Internal.TaskkillTreeKill, or TestProcessTreeKill in tests)