Skip to content

Adopt TimeProvider for deterministic time control across NexNet core - #76

Merged
DJGosnell merged 23 commits into
masterfrom
75-adopt-timeprovider
May 27, 2026
Merged

DJGosnell merged 23 commits into
masterfrom
75-adopt-timeprovider

Conversation

@DJGosnell

@DJGosnell DJGosnell commented May 26, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Closes Adopt TimeProvider for deterministic time control across core #75
  • Routes app-logic time-dependent sites in src/NexNet/ through a single ConfigBase.Time surface so tests can drive time deterministically with FakeTimeProvider.
  • Removes the ad-hoc ServerNexusBase.TickCountOverride test seam in favor of the unified mechanism.
  • 21 commits on the branch, 13 implementation phases + REMEDIATE + Option A clarification, +515/-125 lines across 27 source files.

Reason for Change

The old code mixed Environment.TickCount64, new Timer(...), Task.Delay(int), DateTime.UtcNow, and Stopwatch.GetTimestamp() across the codebase. Time-dependent features (auth cache TTL, reconnect backoff, rate-limiter sliding windows, ping interval, idle pool reaping) could only be tested with real-time waits — slow and flaky. The TickCountOverride seam on ServerNexusBase was a one-off escape hatch.

TimeProvider (.NET 8+) is the BCL-blessed abstraction; pairing it with Microsoft.Extensions.TimeProvider.Testing FakeTimeProvider gives deterministic tests across every subsystem with a single config surface.

Impact

  • Default behavior unchanged. ConfigBase.Time defaults to TimeProvider.System, so any consumer who doesn't touch the new property sees zero behavioral or performance change.
  • The internal TimeProviderExtensions.GetTickCount64() extension on TimeProvider.System is Stopwatch-backed (~15-25 ns), preserving the existing Environment.TickCount64 cost profile.
  • Test count: 2628 → 2637 integration (+9 net: 1 auth-cache migration + 2 new auth-cache + 2 new rate-limiter + 1 pool idle reaping + 1 ping cadence + 1 relay reconnect + 1 reconnect-delay + 1 server watchdog; offset by 1 dead-code test removed by Phase 12). 149 generator tests unchanged.

Design clarification: app-logic time vs. wait-for-the-kernel

During the test-expansion pass, the test suite hung for 22 minutes in CI. Root cause: the initial migration routed every Task.Delay through TimeProvider, including ones that exist purely to give the OS real wall-clock time for real network operations. Faking those waits doesn't accelerate any real kernel work and deadlocks teardown when tests stop advancing FakeTimeProvider.

The PR ends with this clean split:

Routed through TimeProvider (app-logic time decisions — legitimately fakeable):

  • Reconnect backoff (NexusClient.cs:235, NexusListRelay.cs:153)
  • Pipe back-pressure polling (NexusPipeReader.cs:115)
  • All CreateTimer sites: ping timer, server watchdog, pool health check, rate-limit cleanup
  • All elapsed-duration tick math: auth cache TTL, session LastReceived, rate-limiter sliding window, pool _lastUsed

Kept on real time (wait for the OS network stack):

  • NexusSession.DisconnectDelay — TCP buffer flush grace period
  • NexusSession.Receiving.HandshakeTimeout — real handshake deadline
  • SocketTransport.ConnectionTimeout — TCP/Uds/Quic connect
  • WebSocketPipe close timeout — WebSocket close-ack deadline

Plan items implemented as specified

All 13 plan phases landed:

  1. ConfigBase.Time surface + internal GetTickCount64() helper.
  2. NexusClient ping timer, reconnect delay, idle-timeout calc.
  3. NexusServer watchdog timer (deferred from parameterless ctor to Configure()).
  4. NexusSession LastReceived and (later reverted) disconnect/handshake delays.
  5. NexusClientPool health timer + last-used timestamps.
  6. ConnectionRateLimiter ctor + sliding window + cleanup timer.
  7. ServerNexusBase auth cache via TimeProvider; TickCountOverride deleted; AuthCache_AttributeTtl_ExpiredEntry_ReChecks migrated to FakeTimeProvider.
  8. NexusPipeReader back-pressure delay.
  9. NexusCollectionManager → NexusListRelay threading.
  10. SocketTransport connect timeout (later reverted to real time).
  11. WebSocketPipe close timeout (later reverted to real time).
  12. Delete dead RegisteredInvocationState.Created field.
  13. Microsoft.Extensions.TimeProvider.Testing package added; new FakeTimeProvider-driven tests.

Deviations from plan implemented

  • ConfigBase.Time is { get; set; } not { get; init; } — matches every other ConfigBase property and lets tests override Time on an already-constructed config returned from helper methods. Documented in the property's XML doc.
  • Microsoft.Extensions.TimeProvider.Testing package added in phase 7 rather than phase 13.
  • NexusPipeReader.time is an optional ctor parameter (defaults to TimeProvider.System) while ConnectionRateLimiter.time is required — NexusPipeReader has 30+ direct test ctor sites and back-pressure timing isn't commonly fake-time-tested.
  • Four network-wait Task.Delay sites reverted to real time (the design clarification above).

Gaps in original plan implemented

Review surfaced two correctness pitfalls caused by the init → set deviation and four documentation gaps:

  • NexusClientPool.PooledClient reads Time dynamically via the shared pool config so _lastUsed writes stay consistent with PerformHealthAndIdleCheck reads even if Config.Time is mutated.
  • NexusClient._pingTimer construction deferred from ctor to TryConnectAsyncCore so the timer uses whatever TimeProvider is current at connect-time.
  • NexusServer.DisposeAsync now disposes _watchdogTimer (pre-existing oversight surfaced by the review).
  • ConfigBase.Time XML doc explains the "set before lifecycle methods" contract and what mutating it later does and doesn't affect.
  • INexusSession.DisconnectIfTimeout doc updated to reference Config.Time.GetTickCount64().

Additional FakeTimeProvider coverage delivered beyond the original plan:

  • PerIpWindow_FakeTimeProvider_ExpiredEntriesAllowNewConnections
  • BanExpiration_FakeTimeProvider_ExpiredBanReleasesIp
  • Pool_FakeTimeProvider_DisposesIdleClientsDeterministically
  • 7 existing NexusClientPoolTests migrated to FakeTimeProvider.Advance instead of Task.Delay (eliminates ~1.5s of real-time waits and several non-deterministic boundary assertions; also fixes the pre-existing Pool_RespectsMinIdleConnections Linux-CI flake).
  • AuthCache_AttributeTtl_SecondCallUsesCachedResult hardened with FakeTimeProvider.
  • Client_PingTimer_FiresAtConfiguredInterval_DrivenByFakeTime — precise multi-interval cadence.
  • RelayReconnect_500msBackoff_DrivenByFakeTime — pins the NexusListRelay 500ms boundary.
  • Client_ReconnectDelay_DrivenByFakeTime — exercises a 30-second DefaultReconnectionPolicy delay deterministically.
  • Server_ConnectionWatchdog_DisconnectsTimedOutSessions_DrivenByFakeTime — first direct coverage of NexusServer.ConnectionWatchdog.

Migration Steps

None — purely additive for external consumers. Add Time = fakeTime to your ServerConfig / ClientConfig object initializer to opt in.

Performance Considerations

  • TimeProvider.System.GetTimestamp() is Stopwatch-backed; GetTickCount64() extension adds one integer division per call. Net cost in LastReceived write path and watchdog read: ~15-25 ns each, dominated by the Stopwatch read.
  • All timers go through TimeProvider.CreateTimer which under TimeProvider.System wraps System.Threading.Timer — byte-equivalent default behavior.
  • No new allocations in steady-state code paths.

Security Considerations

  • ConfigBase.Time widens mutation surface (from init to set), but ConfigBase is the canonical mutable configuration class — every other property is already { get; set; }. No security boundary crossed; the time source is local to the server/client instance.
  • Microsoft.Extensions.TimeProvider.Testing is added only to NexNet.IntegrationTests (test-only). Production NexNet core has no new dependencies.

Breaking Changes

  • Consumer-facing: None. ConfigBase.Time is purely additive with a TimeProvider.System default.
  • Internal: ConnectionRateLimiter, NexusCollectionManager, NexusListRelay<T>, NexusPipeReader ctor signatures changed. All are internal types; all call sites (production + test) are updated within this PR.

Intentional non-scope

Issue #75's body lists MutexSlim (internal lock-contention machinery, no test value) and CoreLogger.Stopwatch (cosmetic diagnostic timestamps) as refactor sites. Both were intentionally skipped during DESIGN — refactoring either would touch load-bearing or low-value code with no test payoff. Recorded in workflow.md Decisions (2026-05-06 entries).

Follow-up work surfaced but not in this PR

  • B3 client-side timeout disconnect test — client pings race with FakeTimeProvider.Advance; server-side B4 covers the same architectural property safely.
  • B5 server DisconnectDelay and B6 WebSocketPipe close-timeout tests — would require raw-TCP/raw-WebSocket harness scaffolding for an unresponsive-peer scenario.
  • Additional ConnectionRateLimiter window scenarios beyond the two added here.

DJGosnell added 23 commits May 6, 2026 17:36
Phase 1 of issue #75. Introduces the dormant TimeProvider surface that
later phases route time-dependent behavior through.

- Add `TimeProvider Time { get; init; } = TimeProvider.System;` to
  ConfigBase so both ServerConfig and ClientConfig inherit a single
  configuration point. Default keeps behavior byte-equivalent to before.
- Add internal TimeProviderExtensions.GetTickCount64() — drop-in
  replacement for Environment.TickCount64 used by later phases for
  elapsed-duration math against existing long-typed tick fields
  (LastReceived, timeoutTicks, auth-cache ExpiresAtTicks, rate-limiter
  sliding window). Stopwatch-backed on TimeProvider.System (~15-25 ns).

No call sites changed yet. Subsequent phases switch sites individually.
…meProvider

Phase 2 of issue #75.

- _pingTimer: Timer -> ITimer; constructed via _config.Time.CreateTimer.
- Timer.Change call sites switched to TimeSpan overloads
  (Timeout.InfiniteTimeSpan, TimeSpan.FromMilliseconds(PingInterval)).
- Reconnect Task.Delay uses the TimeProvider overload.
- Idle timeoutTicks computed via _config.Time.GetTickCount64().
…eProvider

Phase 3 of issue #75.

- _watchdogTimer: Timer -> ITimer?. Removed from parameterless ctor;
  now constructed inside Configure() via _config.Time.CreateTimer so
  the configured TimeProvider drives it.
- Change call sites switched to TimeSpan overloads.
- ConnectionWatchdog timeoutTicks computed via _config.Time.GetTickCount64().
…gh TimeProvider

Phase 4 of issue #75.

- NexusSession.cs: disconnect delay Task.Delay uses _config.Time.
- NexusSession.Receiving.cs: handshake-timeout Task.Delay uses Config.Time.
- LastReceived = Config.Time.GetTickCount64() so the watchdog timeout
  math sees a consistent time source on both sides.
…imeProvider

Phase 5 of issue #75.

- _healthCheckTimer: Timer -> ITimer via _config.ClientConfig.Time.CreateTimer.
- PerformHealthAndIdleCheck reads now via TimeProvider.GetUtcNow().UtcDateTime,
  preserving the existing public DateTime LastUsed surface (no breaking
  type change).
- PooledClient now takes a TimeProvider in its ctor and sources both
  its initial _lastUsed and UpdateLastUsed() from it.
…sliding window

Phase 6 of issue #75.

- ConnectionRateLimiter ctor adds a required TimeProvider param.
- _cleanupTimer: Timer -> ITimer; cleanup-interval delays use TimeSpan
  overloads.
- TryAcquire/RecordViolation sliding-window 'now' values read via
  _time.GetTickCount64().
- NexusServer.Configure passes config.Time when instancing the limiter.
- All 14 direct test-side ctor sites updated to pass TimeProvider.System.
…ntOverride seam

Phase 7 of issue #75.

- Delete internal TickCountOverride field on ServerNexusBase.
- CheckAuthorization 'now' and ExpiresAtTicks computation now read from
  SessionContext.Session.Config.Time.GetTickCount64() — the configured
  TimeProvider is the single source of truth for cache lifetime math.
- AuthCache_AttributeTtl_ExpiredEntry_ReChecks migrated to FakeTimeProvider
  via Microsoft.Extensions.TimeProvider.Testing.

Deviation from plan recorded in workflow.md:
- ConfigBase.Time changed from { get; init; } to { get; set; } for
  consistency with the rest of ConfigBase (Logger, the int setters,
  etc.) and to allow tests to override Time on an already-constructed
  ServerConfig returned from helper methods.
- Microsoft.Extensions.TimeProvider.Testing PackageReference added now
  in phase 7 (not deferred to phase 13).
Phase 8 of issue #75.

- NexusPipeReader ctor gains an optional TimeProvider? time = null
  parameter (defaults to TimeProvider.System). The class is internal
  and tests construct it directly without caring about timing, so the
  optional default avoids churn across many test sites.
- Production NexusDuplexPipe passes _session.Config.Time explicitly so
  end-user-configured TimeProviders drive the back-pressure delay.
- The four-step int-ms back-pressure delay is wrapped in
  TimeSpan.FromMilliseconds before reaching Task.Delay(TimeSpan, TimeProvider).
Phase 9 of issue #75.

- NexusCollectionManager ctor adds a TimeProvider parameter, stored
  privately and passed through to NexusListRelay during ConfigureList.
- NexusListRelay ctor adds a TimeProvider parameter; its 500 ms
  reconnect-loop Task.Delay uses the TimeProvider overload.
- NexusClient and NexusServer construct the manager with _config.Time.
- Non-relay collection types (NexusListServer/Client/NexusBroadcastServer)
  have no time-dependent sites — verified by grep, no wiring needed.
Phase 10 of issue #75.

The TCP/Quic/Uds connect-timeout Task.Delay now uses the TimeProvider
overload with clientConfig.Time, so tests can advance fake time past
the connect timeout deterministically.
Phase 11 of issue #75.

WebSocket close-timeout Task.Delay now uses _config.Time, so tests
can fake-advance through the WhenAny race.
Phase 12 of issue #75.

The Created field was only ever assigned in SessionInvocationStateManager
and never read. Grep-confirmed dead code. Field declaration and
assignment removed. Not a TimeProvider migration - cleanup recorded as
a separate phase decision during DESIGN.
Phase 13 of issue #75 (final implementation phase).

Two new tests exercising the migrated auth-cache code path under
FakeTimeProvider:

- AuthCache_FakeTimeProvider_AdvanceJustBeforeTtl_StaysCached:
  advances 1999 ms (TTL is 2000 ms) and confirms the cache stays hot.
- AuthCache_FakeTimeProvider_MultipleExpiryCycles_ReChecksEachTime:
  walks three full TTL cycles and confirms OnAuthorize fires once per
  cycle.

Both deterministic, no real-time waits. The package reference for
Microsoft.Extensions.TimeProvider.Testing was added earlier in phase 7
when the original AuthCache_AttributeTtl_ExpiredEntry_ReChecks test
was migrated.
All 13 phases complete. 2630 integration tests + 149 generator tests
green. Transitioning to REVIEW phase.
…coverage

REMEDIATE phase resolving the 8 A-class findings from review.md.

Correctness fixes
- NexusClientPool.PooledClient now stores the pool config and reads
  Time dynamically (`_poolConfig.ClientConfig.Time`), keeping its
  _lastUsed writes consistent with PerformHealthAndIdleCheck reads if
  Config.Time is mutated post-construction.
- NexusClient._pingTimer construction deferred from the ctor to
  TryConnectAsyncCore so the timer always uses whatever TimeProvider
  is current at connect-time. Disposes any prior timer on reconnect.
- NexusServer.DisposeAsync now disposes _watchdogTimer (pre-existing
  oversight, surfaced by the review).

Documentation fixes
- ConfigBase.Time XML doc explains the "set before lifecycle methods"
  contract and what mutating it later does/doesn't affect.
- INexusSession.DisconnectIfTimeout doc updated from
  "Environment.TickCount64" to "Config.Time.GetTickCount64()".
- workflow.md gains a 2026-05-26 Decisions entry explaining the
  NexusPipeReader/ConnectionRateLimiter ctor-parameter asymmetry.

Test coverage additions
- PerIpWindow_FakeTimeProvider_ExpiredEntriesAllowNewConnections —
  sliding-window expiry under FakeTimeProvider.
- BanExpiration_FakeTimeProvider_ExpiredBanReleasesIp — ban duration
  expiry under FakeTimeProvider.
- Pool_FakeTimeProvider_DisposesIdleClientsDeterministically — pool
  idle reaping under FakeTimeProvider.

The reconnect-backoff and ping-interval test scenarios that were also
listed in the plan's Phase 13 require non-trivial scaffolding (custom
IReconnectionPolicy + wire-level inspection) and are deferred — they
do not block the migration's core goal.

Integration tests: 2630 → 2633. Generator tests: 149 (unchanged).
Second-pass sweep after a structured scan identified 16 actionable
opportunities. Of these, 7 pool tests were migrated to FakeTimeProvider
and 4 new tests added covering previously-untested time-driven sites.
B5 (server disconnect-delay) and B6 (WebSocket close timeout) require
raw-TCP harness scaffolding and are deferred. B3 (client-side timeout
disconnect) is intentionally skipped: client pings race with
FakeTimeProvider.Advance during synchronous timer callbacks. B4 covers
the same architectural property from the server side.

Migrations (A class) — replace real-time Task.Delay with fakeTime.Advance:
- Pool_DisposesIdleClientsAfterTimeout
- Pool_KeepsAtLeastOneClientDespiteIdleTimeout
- Pool_RespectsMinIdleConnections (also fixes the Linux-CI timing flake
  we hit earlier in REMEDIATE)
- Pool_MinIdleConnectionsDoesNotCreateNewClients
- Pool_MinIdleConnectionsWithZeroValue
- Pool_MaxIdleTimeConfiguration_ZeroValue
- Pool_HealthCheckTimer_ContinuesAfterExceptions
- AuthCache_AttributeTtl_SecondCallUsesCachedResult (hardened against
  tight TTL — no longer relies on test runtime < TTL)

These collectively eliminate ~1.7 s of cumulative real-time waits and
convert non-deterministic boundary assertions (Is.LessThan(N),
Is.GreaterThanOrEqualTo(N)) to exact equalities.

New tests (B class):
- Client_PingTimer_FiresAtConfiguredInterval_DrivenByFakeTime —
  precise cadence assertions (ping at t=1000, t=2000, none at t=0).
  Replaces the loose ClientSendsPing test that only verified
  'fires at least once.'
- RelayReconnect_500msBackoff_DrivenByFakeTime — first targeted
  coverage of NexusListRelay.cs:153's 500 ms reconnect delay.
- Client_ReconnectDelay_DrivenByFakeTime — exercises a 30-second
  DefaultReconnectionPolicy delay deterministically; previously
  impossible without bloating test runtime.
- Server_ConnectionWatchdog_DisconnectsTimedOutSessions_DrivenByFakeTime —
  first direct coverage of NexusServer.cs:402's watchdog disconnect
  path (previously only indirectly exercised by ReconnectsOnTimeout).

Infrastructure:
- NexusCollectionBaseTests.CreateRelayCollectionClientServers gains
  an optional TimeProvider? server2Time parameter for B7.

Test count: 2633 -> 2637 (+4 net; 7 migrations don't change count).
…n fakeTime delay

The 8 pool migrations and 4 new tests caused the integration suite to
hang because the tests' teardown paths (pool.DisposeAsync, test
fixture client/server teardown, or server-initiated disconnect) call
NexusSession.Close which awaits Task.Delay(DisconnectDelay,
_config.Time). When _config.Time is FakeTimeProvider and DisconnectDelay
is the default 200 ms, the await never completes without further
fakeTime.Advance — and the tests' assertion phase doesn't advance any
more.

Fix: every test that injects FakeTimeProvider into a config also sets
that config's DisconnectDelay = 0 so the disconnect path skips the
Task.Delay branch entirely (NexusSession.cs:306 `if (_config.DisconnectDelay > 0)`).

Affected tests:
- 7 migrated Pool_* tests + 1 existing Pool_FakeTimeProvider* test
- AuthCache_FakeTimeProvider_* (3 tests) and AuthCache_AttributeTtl_*
  ExpiredEntry_ReChecks/SecondCallUsesCachedResult
- Client_PingTimer_FiresAtConfiguredInterval_DrivenByFakeTime
- Client_ReconnectDelay_DrivenByFakeTime
- Server_ConnectionWatchdog_DisconnectsTimedOutSessions_DrivenByFakeTime
- RelayReconnect_500msBackoff_DrivenByFakeTime

Local full suite: 2637/2637 in 2m38s.

This was caught when CI hung at 22 minutes on the previous push.
…test hacks

Root-cause fix for the FakeTimeProvider test hang that the previous
commit only worked around. The initial migration routed every Task.Delay
through TimeProvider, including ones that exist purely to give the OS
real wall-clock time for real network operations. That conflated two
unrelated concepts:

- App-logic time decisions: reconnect backoff, ping interval, auth
  cache TTL, rate-limiter sliding window, pool idle reaping. Faking
  these makes tests deterministic — the right call.
- Wait for the kernel: TCP buffer flush before close, handshake
  deadline, connect timeout, WebSocket close-ack timeout. The real
  OS network stack runs on real wall-clock time; faking it doesn't
  accelerate any real work and just deadlocks teardown when tests
  stop advancing FakeTimeProvider.

Reverted (back to real Task.Delay):
- NexusSession.cs:311 — DisconnectDelay grace period for TCP flush
- NexusSession.Receiving.cs:17 — HandshakeTimeout handshake deadline
- SocketTransport.cs:96 — ConnectionTimeout TCP/Uds/Quic connect
- WebSocketPipe.cs:95 — WebSocket close-handshake timeout

Kept routed through TimeProvider (legitimate app-logic time):
- NexusClient.cs:235 — reconnect backoff
- NexusListRelay.cs:153 — relay reconnect backoff
- NexusPipeReader.cs:115 — back-pressure polling
- All CreateTimer sites (ping, watchdog, health-check, rate-limit
  cleanup)
- All GetTickCount64 sites (auth cache, LastReceived, sliding
  window, pool last-used)

Removed all 15 DisconnectDelay = 0 workarounds from FakeTimeProvider
tests (1 was a pre-existing legit config-validation test, untouched).

Local full suite: 2637/2637 in 2m39s, no hangs with --blame-hang-timeout 60s.
@DJGosnell
DJGosnell merged commit d5f84ad into master May 27, 2026
1 check passed
@DJGosnell
DJGosnell deleted the 75-adopt-timeprovider branch May 27, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt TimeProvider for deterministic time control across core

1 participant