Skip to content

Use Stopwatch for LockableContext acquire timeouts#1772

Merged
kevin-montrose merged 3 commits into
microsoft:mainfrom
unsafePtr:fix/lockablecontext-stopwatch-timeout
May 18, 2026
Merged

Use Stopwatch for LockableContext acquire timeouts#1772
kevin-montrose merged 3 commits into
microsoft:mainfrom
unsafePtr:fix/lockablecontext-stopwatch-timeout

Conversation

@unsafePtr
Copy link
Copy Markdown
Contributor

DoManualTryLock and DoManualTryPromoteLock use DateTime.UtcNow.Ticks deltas to enforce the caller-supplied TimeSpan timeout. A wall-clock adjustment (NTP step, manual change) lands the comparison on a perturbed value: a backward step keeps retrying past the intended deadline, a forward step gives up early. Both methods are pure local timeout loops — no protocol or persistence concerns — so monotonic time is the right primitive.

Switch to Stopwatch.GetTimestamp() / Stopwatch.GetElapsedTime for the elapsed-time math. Behavior under stable clocks is unchanged; the timeout contract is now enforced against a clock that can't move sideways.

Same NTP-resilience reasoning was applied recently in dotnet/runtime#127303 for EventCounter's polling timer.

Changes

  • DoManualTryLock and DoManualTryPromoteLock in LockableContext.cs capture Stopwatch.GetTimestamp() at entry and gate retries on Stopwatch.GetElapsedTime(startTimestamp) > timeout.

Test plan

  • TryLockTimeSpanLimitTest, TryLockCancellationTest, TryPromoteLockTimeSpanLimitTest, TryPromoteLockCancellationTest pass on net8.0 and net10.0.
  • Tsavorite.core builds clean.
  • CI

Copilot AI review requested due to automatic review settings May 5, 2026 23:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Tsavorite's manual lock-acquisition retry loops to measure elapsed timeout using Stopwatch instead of DateTime.UtcNow, so lock and lock-promotion timeouts are based on monotonic time and are no longer affected by wall-clock adjustments.

Changes:

  • Replaced wall-clock timeout tracking in DoManualTryLock with Stopwatch.GetTimestamp() / Stopwatch.GetElapsedTime.
  • Replaced wall-clock timeout tracking in DoManualTryPromoteLock with the same monotonic timing pattern.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread libs/storage/Tsavorite/cs/src/core/ClientSession/LockableContext.cs Outdated
Comment thread libs/storage/Tsavorite/cs/src/core/ClientSession/LockableContext.cs Outdated
Comment thread libs/storage/Tsavorite/cs/src/core/ClientSession/LockableContext.cs Outdated
@unsafePtr unsafePtr requested a review from kevin-montrose May 11, 2026 10:46
@kevin-montrose
Copy link
Copy Markdown
Contributor

This LGTM - please fix conflicts and get a green CI run.

@kevin-montrose kevin-montrose merged commit 4cbaa99 into microsoft:main May 18, 2026
381 of 383 checks passed
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.

3 participants