Recover from concurrent OAuth token cache writes - #1793
Merged
Conversation
Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
mihaimitrea-db
temporarily deployed
to
test-trigger-is
August 5, 2026 14:18 — with
GitHub Actions
Inactive
mihaimitrea-db
temporarily deployed
to
test-trigger-is
August 5, 2026 14:18 — with
GitHub Actions
Inactive
mihaimitrea-db
temporarily deployed
to
test-trigger-is
August 5, 2026 14:19 — with
GitHub Actions
Inactive
mihaimitrea-db
marked this pull request as ready for review
August 5, 2026 14:54
Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
mihaimitrea-db
temporarily deployed
to
test-trigger-is
August 5, 2026 15:02 — with
GitHub Actions
Inactive
mihaimitrea-db
temporarily deployed
to
test-trigger-is
August 5, 2026 15:03 — with
GitHub Actions
Inactive
mihaimitrea-db
temporarily deployed
to
test-trigger-is
August 5, 2026 15:04 — with
GitHub Actions
Inactive
mihaimitrea-db
requested review from
hectorcast-db and
parthban-db
and removed request for
hectorcast-db
August 5, 2026 16:22
parthban-db
approved these changes
Aug 13, 2026
| if cached.AccessToken == old.AccessToken || !cached.Valid() { | ||
| return false | ||
| } | ||
| if candidate.Expiry.IsZero() || cached.Expiry.IsZero() { |
Contributor
There was a problem hiding this comment.
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>
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
PersistentAuthreloads the cache immediately and then after 25, 50, 100, and 200 milliseconds.Internal changes
How is this tested?
make test(race-enabled full unit test suite)make lintgo 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.