Skip to content

Bound credential operations and prevent authentication resource contention - #2082

Draft
Tyrie Vella (tyrielv) wants to merge 2 commits into
microsoft:vnextfrom
tyrielv:tyrielv/auth-lock-contention
Draft

Bound credential operations and prevent authentication resource contention#2082
Tyrie Vella (tyrielv) wants to merge 2 commits into
microsoft:vnextfrom
tyrielv:tyrielv/auth-lock-contention

Conversation

@tyrielv

@tyrielv Tyrie Vella (tyrielv) commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Problem and Context

Runtime credential helpers can wait indefinitely during object downloads and background prefetch.

A missed Git Credential Manager prompt can block maintenance while it holds the shared prefetch lock.

Credential work can also hold shared HTTP capacity. Concurrent authentication failures can then block healthy hydration requests.

Cancellation previously stopped at the HTTP retry layer. It could not interrupt credential gates or credential-helper processes.

This PR supersedes #2046. It also uses the shared process-wait helper from merged PR #2045.

Changes

Bound runtime credential operations

  • Apply a 120-second default timeout to credential fill, approve, reject, and reload operations.
  • Read gvfs.credential-timeout-seconds through RetryConfig.
  • Treat values of zero or less as the escape hatch for the historical unbounded wait.
  • Reject configured values that overflow the millisecond conversion.
  • Make the credential serialization gate wait at least as long as the credential operation.
  • Stop retries and endpoint fallback after a terminal credential timeout.
  • Kill the full process tree after timeout or cancellation.
  • Reuse the shared WaitForExitWithCancellation helper from Stream large -z git output instead of buffering it against the cap #2045.

Prevent authentication resource contention

  • Pass CancellationToken through HTTP requests, GitAuthentication, ICredentialStore, and GitProcess.
  • Cancel initialization waits and credential-gate waits.
  • Prevent a second credential helper after a credential-gate timeout.
  • Release the HTTP connection permit before credential rejection when the feature flag is enabled.
  • Gate early release with the off-by-default gvfs.release-connection-before-credential-reject setting.
  • Prevent a second permit release after cancellation or another exception.
  • Publish connection configuration atomically and emit rollout telemetry.

Preserve credential routing

  • Use the enlistment .git directory when it exists.
  • Run outside the enlistment before clone creates that directory.
  • Propagate timeout and cancellation values through both routes.

Add test coverage

  • Cover timeout configuration, overflow rejection, timeout classification, reject bounds, and endpoint fallback.
  • Cover cancellation, credential-gate contention, post-cancellation gate reuse, and initialization waits.
  • Cover early permit release, disabled behavior, and exact single-release behavior.
  • Run the full unit suite: 1,025 passed, 0 failed, and 12 expected tests ignored.

@tyrielv
Tyrie Vella (tyrielv) force-pushed the tyrielv/auth-lock-contention branch from c2b26d1 to 840201a Compare August 18, 2026 20:51
@tyrielv
Tyrie Vella (tyrielv) changed the base branch from master to vnext August 18, 2026 20:52
@tyrielv Tyrie Vella (tyrielv) mentioned this pull request Sep 2, 2026
Open
@tyrielv Tyrie Vella (tyrielv) changed the title Auth concurrency: release HTTP pool slot before credential reject; thread cancellation to the credential path (stacked on #2046) Bound credential operations and prevent authentication resource contention Sep 22, 2026
@tyrielv Tyrie Vella (tyrielv) mentioned this pull request Sep 22, 2026
Closed
@tyrielv Tyrie Vella (tyrielv) mentioned this pull request Sep 24, 2026
Merged
Tyrie Vella (tyrielv) and others added 2 commits September 29, 2026 15:55
@tyrielv
Bound runtime credential fetch so prefetch can't hang on auth
759d237 
The runtime credential path (HttpRequestor.SendRequest ->
GitAuthentication.TryGetCredentials/RejectCredentials ->
TryCallGitCredential) called git-credential with timeoutMs = -1, so
Process.WaitForExit(-1) waited forever. When a GCM auth popup was missed
(e.g. behind another window), the mount's background maintenance
PrefetchStep blocked indefinitely while holding the shared
prefetch-commits-trees.lock, which in turn blocked a user-initiated
`gvfs prefetch`.

The mount startup auth path was already bounded via credentialTimeoutMs;
this extends the same bound to every runtime credential invocation:

- TryGetCredentials takes credentialTimeoutMs (default
  DefaultCredentialTimeoutMs) and plumbs it to TryCallGitCredential.
- RejectCredentials, which reloads the credential on the 401-retry leg,
  takes and plumbs the same timeout (this leg is the actual stale-token
  hang path and was otherwise still unbounded).
- ApproveCredentials, RejectCredentials and the ICredentialStore
  store/delete operations are bounded too. `git credential approve` and
  `git credential reject` previously ran with timeoutMs = -1 while
  holding gitAuthLock, so a stalled helper could still pin the prefetch
  lock forever even after the fill leg was bounded.
- HttpRequestor exposes a protected virtual CredentialTimeoutMs and
  passes it to TryGetCredentials, RejectCredentials and
  ApproveCredentials.

The bound is generous (120s) rather than the 30s default: the mount's
requestor is shared by the background maintenance prefetch, interactive
on-demand hydration, and the user-initiated prefetch/clone verbs, where a
human may legitimately take longer than 30s to answer a GCM cold-start /
MFA / smartcard prompt. 120s still bounds the hang while being long
enough not to cut off a prompt the user is actively answering.

The value lives on RetryConfig, which is already loaded once from git
config and already passed into the HttpRequestor constructor alongside
MaxRetries and Timeout. It is overridable via
gvfs.credential-timeout-seconds; 0 or less restores the old unbounded
wait as a field escape hatch. Reading it here rather than inside the
requestor keeps requestor construction free of config I/O: a per-instance
read would spawn `git config` on the mount startup path, and
GetFromConfig itself runs unbounded, which is exactly the class of
unbounded git invocation this change exists to remove.

The credential serialization gate now waits at least as long as the fetch
it is serializing. It previously waited a fixed 60s, and on expiry fell
through and spawned a second credential fetch. With a 120s fetch bound
that guaranteed a second, competing GCM prompt in exactly the slow-prompt
case the longer bound exists to tolerate.

A timed-out fetch no longer asks the caller to retry. SendRequest
previously returned shouldRetry: true for every credential failure, so a
timeout burned the whole RetryWrapper budget (up to MaxAttempts x 120s),
re-prompting the user each time. TryGetCredentials now reports whether
the failure was a timeout, and SendRequest sets shouldRetry accordingly;
genuine auth failures still retry as before.

On timeout the git process tree is killed, not just git.exe. Killing only
git.exe left the credential helper child alive, holding the credential
store and showing orphaned prompt UI. The kill is now followed by a
bounded wait so the async stdout/stderr readers flush before their
buffers are read.

The timeout is reported as a distinct CredentialFetchTimedOut telemetry
event with structured timeoutMs and RepoUrl fields, rather than only as
warning message text. This is what makes the 120s choice measurable in
the field: how often the bound fires, and whether a timeout is followed
by a successful fetch (a prompt that was cut off) or not (a hang that was
prevented).

On timeout the fetch fails, backoff engages, the download gives up, and
the lock is released instead of hanging forever.

Tests: MockGitProcess now records the timeout passed to each git
invocation, so tests can assert the bound is actually plumbed rather than
just that a failure message appears. The timeout test asserts the
observed timeout and the rendered "within 1 seconds" message; reverting
the plumbing makes it fail (verified by mutation). Adds a test that the
401-reject leg bounds both the credential reload and the erase, a test
that only genuine timeouts are reported as timeouts so a real auth
failure still retries (also mutation-verified), and RetryConfig coverage
for the default, configured, and unbounded-escape-hatch values.

Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv
Prevent authentication resource contention and propagate cancellation
e65cc6e 
This stacked change uses the shared process-wait helper from microsoft#2045.

Pass cancellation through authentication and credential-helper operations. Stop the helper process tree when cancellation occurs.

Release HTTP capacity before credential rejection when the off-by-default feature flag is enabled. Prevent duplicate permit releases.

Preserve terminal credential timeouts across endpoint selection. Prevent concurrent helpers after a credential-gate timeout.

Publish connection configuration atomically and emit rollout telemetry. Add tests for contention, cancellation, timeout propagation, and gate reuse.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tyrielv