Repository navigation
[RT-E2] refactor: terminals run on the client's TimeProvider - #432
Merged
Merged
Conversation
ITerminalDeadlineFactory, TerminalDeadlineFactory and TerminalCancellationSource are gone. ConnectionSnapshot carries the client's clock; PTY and persistent PTY sessions pass it to the terminal core, its send deadlines and both graceful close bounds, which stay independent. New manual-clock tests prove the admission bound, the close-output bound, the two-budget case and the CancelOnWorkerAsync contract.
The disposed-deadline row fires a callback queued after Dispose, not one that overlaps it, and the barrier race now carries a registration so the callback's Cancel runs work under the lock while Dispose contends for it.
This was referenced Oct 17, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The PTY and persistent-PTY terminal families now run their send deadlines and graceful-close bounds on the client's
TimeProvider. This is RT-E2 PR B of three. A (#429, background service andTimedCancellation) has merged; C (the launcher and the clock ban) follows.Closes
ITerminalDeadlineFactoryduplicatedTimeProvider, and the close bounds bypassed it.ITerminalDeadlineFactoryandTerminalDeadlineFactoryare deleted.TerminalCancellationSourceonly forwarded to aCancellationTokenSource. It is deleted; raw sources withCancelOnWorkerAsyncreplace it.CancelOnWorkerAsynccontract had no test. It has one now.The change
ConnectionSnapshotcarries the client's clock fromPipeline.PtyClient.ConnectAsyncand the persistent attach pass it to the sessions, thenTerminalSocketCore,TerminalSendQueueand each send deadline, as required internal parameters with no System fallback.TimedCancellation.Createfrom [RT-E2] refactor: the background service runs on an injected TimeProvider #429, so the downlevel timer is released on an early dispose.ConnectionSnapshotTests,CancellationIdentityTests, and three benchmarks, which passTimeProvider.System; the ban in PR C coverssrconly. U2-07's real-timer races belong to TST-E2b and are not rewritten here.Proof
ManualTimeProvider. Each is killed by a compiling product mutant that keeps the signature, on net10.0 and net472:TerminalSendQueueTests: the admission bound, the close-output bound, and the two-budget case;CancellationTokenSourceExtensionsTests:CancelOnWorkerAsyncreturns while a callback blocks, and completes only after it.TerminalSocketCoreTests' queued-expiry test threw aTimeoutExceptioninside anAsyncLocalchange callback. An exception there triggersEnvironment.FailFastand takes the whole test host down. The callback no longer throws: it records the wait, and the test asserts it on the test thread. A never-expire mutant now fails the test normally.AsyncLocalcallback. Under full-suite scheduling the callback could fire on an unrelated continuation carrying the same execution context, so the first call released the gate early and the test timed out. The hold now runs in the core's own post-send step (TerminalSendActions.OnSent), on the first call's path, after the physical send and before the gate is released. It is deterministic.tools/jscpd/tests.baseline.jsonmakes 24 one-for-one swaps of pre-existing clones that the compile-forced clock argument re-fingerprinted, plus one removal of a clone this branch eliminated, so it goes from 655 to 654 entries. The 2 clones the branch introduced were refactored into helpers (CreateGatedCloseQueue,AssertAdmissionReleasedAsync). Consolidating the 24 pre-existing arranges is TST-E4's work, not a product PR's.410dc16d23b0fb80→c2a7b415518d2198,b1f194514a39741f→ba50dc522914af62,bb9c534e42d8fbec→ceed549c863ce378,80cfe1c4da15558e→7cb3773f240a7d3b,614c8cb7e327b799→ba1b81c7dacd299e,eb7e225d5bfe0159→7baca2d0e7c78a56,9111880caf330ba5→2fcfde38c702a305,d9ac5bae878d22b3→63ccee6cade2a08c,b107b4bdff30ec7a→3081885a80a22be3,cbf3676e991f1146→0fcc7cca1b6402dd,85079fc139806e83→4202e901bcb4f8ba,70bd27545e44e77c→f37319ef3f0e2c88,dee72b571774c9d5→210bbefc7e3a3805,ca8b61da9bd303bb→58df2dce6a57ddc3,2ad053ce77d6dfb2→a5830a7e1b5d7975,064bde5a422105ed→13fa62aa808a28cb,3bccb11c8653a298→a644bdfbe6ec0044,a7b53cc19a267d21→50c75dddf97e272d,0d1c1e62f027669f→93c7ffbef646d5bd,e45d1d2fb94c859b→813356530d4e26c5,c3312d7a632e3665→ee67ec3f21f2f9ce,33df1e6ccf3c7b58→030445ef25b69be7,ceefd01b00d7a785→8845f6d375146134,9c41941b90bc5dc4→2d01f1bea1f9ea7e; removeda86ca42b60a2d816ReservedNamePolicy.InternalTypeNamesdrops the three deleted types.OpenCode.Sdk.Tools.Testsproject 2008/2008; Terminal, Ptys, PersistentPtys and Threading green on net10.0 and net472.PublicApi.verified.txtis unchanged.315ed80f(this branch on master62d82f0a): slopwatch, build, both format passes, clone-detector, verify-clones, 10,457 tests including net472 with 0 failed, and election. An earlier run caught the racy hold described above. The second commit, which only renames two rows and adds a registration, was rerun narrowly:TimedCancellationTests4/4 on net10.0 and 5/5 on net472, the wholeOpenCode.Sdk.Tools.Testsproject 2005/2005, slopwatch, format and clones.FailFastabove, now fixed.TimedCancellationand this switch, which the maintainer approved for lock and race code: "the design is sound", no blocker or should-fix. Two nits are recorded, not fixed:Disposewould skipbase.Disposeonly if_timer.Dispose()threw, which BCLTimer.Disposedoes not;TimedCancellationTestsrows claimed more than they proved. This one is fixed in this PR's second commit: they are renamed to a callback queued afterDisposeand a race carrying a registration, so the callback'sCancelruns work under the lock whileDisposecontends.Need help on this PR? Tag
@codesmith-botwith what you need.