Skip to content

Recover from concurrent OAuth token cache writes - #1793

Merged
mihaimitrea-db merged 3 commits into
mainfrom
fix/u2m-refresh-cache-race
Aug 14, 2026
Merged

Recover from concurrent OAuth token cache writes#1793
mihaimitrea-db merged 3 commits into
mainfrom
fix/u2m-refresh-cache-race

Conversation

@mihaimitrea-db

Copy link
Copy Markdown
Contributor

Summary

Recovers U2M OAuth refreshes when another process wins a concurrent token cache write. The process whose write fails briefly reloads the cache and returns the fresh token stored by the winner.

Why

Multiple CLI or SDK processes can read the same cached OAuth token, refresh it concurrently, and then race to replace the cache entry. On macOS, concurrent Keychain writes can cause one process to fail with exit status 45, even though another process successfully stored a valid token.

This addresses databricks/cli#6051 without adding cross-platform process locks. It does not prevent concurrent refreshes; it prevents a losing cache write from becoming an authentication failure when a fresh winner is available.

What changed

Interface changes

None.

Behavioral changes

  • After a refreshed token fails to be stored, PersistentAuth reloads the cache immediately and then after 25, 50, 100, and 200 milliseconds.
  • A changed, valid cached token is returned when its expiry is within one minute of the token obtained by the current process.
  • The original cache update error is preserved when no suitable token appears within the bounded 375-millisecond recovery window.

Internal changes

  • Added freshness comparison and cache update recovery helpers.
  • Added tests covering delayed winner discovery, changed-token detection, validity, and expiry tolerance.
  • Added a bug-fix changelog entry.

How is this tested?

  • make test (race-enabled full unit test suite)
  • make lint
  • go test -count=1 ./credentials/u2m/...

The real macOS Keychain race has not yet been stress-tested on macOS. The unit test models the observed sequence: the cache write fails, the first reload still sees the original token, and a later reload sees the fresh token stored by another process.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
@mihaimitrea-db
mihaimitrea-db marked this pull request as ready for review August 5, 2026 14:54
Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
Comment thread credentials/u2m/persistent_auth.go Outdated
if cached.AccessToken == old.AccessToken || !cached.Valid() {
return false
}
if candidate.Expiry.IsZero() || cached.Expiry.IsZero() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we avoid skipping the expiry comparison when only one side is zero? If
candidate.Expiry is zero, this accepts a changed cached token expiring in one
minute because Valid() only checks whether it is valid now, so we can return a
token inside the five-minute refresh window.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
@mihaimitrea-db
mihaimitrea-db deployed to test-trigger-is August 14, 2026 09:25 — with GitHub Actions Active
@mihaimitrea-db
mihaimitrea-db deployed to test-trigger-is August 14, 2026 09:26 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-go

Inputs:

  • PR number: 1793
  • Commit SHA: 78caae1f4552886aa509c2c1791edd919adbed99

Checks will be approved automatically on success.

@mihaimitrea-db
mihaimitrea-db deployed to test-trigger-is August 14, 2026 09:27 — with GitHub Actions Active
@mihaimitrea-db
mihaimitrea-db added this pull request to the merge queue Aug 14, 2026
Merged via the queue into main with commit 2f8228c Aug 14, 2026
13 checks passed
@mihaimitrea-db
mihaimitrea-db deleted the fix/u2m-refresh-cache-race branch August 14, 2026 10:59
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.

2 participants