Skip to content

Add CancellationToken and async hydration to GitStatusCache#1914

Merged
tyrielv merged 1 commit intomicrosoft:masterfrom
tyrielv:tyrielv/hydration-status-3-cancellation
Mar 18, 2026
Merged

Add CancellationToken and async hydration to GitStatusCache#1914
tyrielv merged 1 commit intomicrosoft:masterfrom
tyrielv:tyrielv/hydration-status-3-cancellation

Conversation

@tyrielv
Copy link
Contributor

@tyrielv tyrielv commented Mar 17, 2026

  • Add CancellationToken parameter to CreateSummary (default: none)
  • Check cancellation between each phase of summary computation
  • Separate OperationCanceledException catch returns invalid summary
  • GitStatusCache creates CancellationTokenSource, cancels on Shutdown
  • Run UpdateHydrationSummary in parallel with TryRebuildStatusCache via Task.Run — independent operations should not delay each other

- Add CancellationToken parameter to CreateSummary (default: none)
- Check cancellation between each phase of summary computation
- Separate OperationCanceledException catch returns invalid summary
- GitStatusCache creates CancellationTokenSource, cancels on Shutdown
- Run UpdateHydrationSummary in parallel with TryRebuildStatusCache
  via Task.Run — independent operations should not delay each other

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tyrielv tyrielv force-pushed the tyrielv/hydration-status-3-cancellation branch from ff67270 to e88b403 Compare March 17, 2026 15:48
Copy link

@KeithIsSleeping KeithIsSleeping left a comment

Choose a reason for hiding this comment

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

see below

@KeithIsSleeping
Copy link

(Corrected re-post — the original review had rendering issues with backtick escaping)

Clean cancellation support and a good parallelization win. Two suggestions:

  1. Race condition on activeHydrationTask — The field is written on the background thread (this.activeHydrationTask = hydrationTask in RebuildStatusCacheIfNeeded) and read in Dispose() on the shutdown path. There's no synchronization between these. If Dispose is called between Task.Run() and the assignment, or if two rebuilds overlap, the stored reference may be stale. Consider Interlocked.Exchange or protecting with a lock.

  2. Dispose() waits with no timeoutthis.activeHydrationTask.Wait() in Dispose could block the main shutdown path indefinitely if the hydration summary hangs (e.g., waiting on a git process). Consider adding a bounded wait: this.activeHydrationTask.Wait(TimeSpan.FromSeconds(5)).

The cancellation-vs-error distinction is well handled — returning an invalid summary with no Error lets the caller log at Info level instead of Warning, and correctly avoids triggering the circuit breaker from PR #1913. The parallel execution makes sense since hydration summary and git status are independent operations.

Minor: a couple of extra blank lines around cancellationToken.ThrowIfCancellationRequested() calls — cleanup nit.

@tyrielv tyrielv merged commit 2db7629 into microsoft:master Mar 18, 2026
49 checks passed
tyrielv added a commit to tyrielv/VFSForGit that referenced this pull request Mar 18, 2026
Resolve conflicts in EnlistmentHydrationSummary.cs and GitStatusCache.cs
by combining ITracer + Stopwatch timing from this branch with
CancellationToken support from PR microsoft#1914.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tyrielv tyrielv deleted the tyrielv/hydration-status-3-cancellation branch March 18, 2026 20: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.

3 participants