Skip to content

claude_oauth: narrow refresh_lock scope to just the token refresh (held across whole attempt incl. 300s back-off) #72

Description

@amondnet

Summary

In forward_claude_oauth() (src/adapters/anthropic.rs), the per-account refresh_lock is acquired at the top of the per-account loop iteration and held for the account's entire attempt — credential resolution, the initial upstream POST, the PauseSame branch's tokio::time::sleep(delay) (retry-after, capped at ~300s), and the retry POST — not just the token-refresh operation.

AccountPool::refresh_lock() (src/accounts.rs) is documented as serializing only "token refreshes for one account", and the method is named accordingly. The actual hold scope is strictly broader than that documented intent.

Impact

Because select_order applies session affinity (x-claude-code-session-id), concurrent requests from the same session route to the same account and serialize on this lock. A second concurrent same-account request blocks at the lock acquire for the first request's full attempt — including an up-to-300s retry-after back-off sleep that has nothing to do with token refresh. This is a latency/throughput bug relative to the lock's stated purpose; it does not corrupt output.

Found by an internal review pass and confirmed by an independent validator (RECALL bias, adjusted confidence ~72). Related to the intentionally-deferred storm-control follow-up.

Suggested fix

Narrow the critical section to only the force_refresh() call (the sole operation that genuinely needs cross-request serialization to avoid concurrent credential-file writes). Credential resolution, the initial POST, and the PauseSame back-off sleep should run without holding the lock. Add a regression test asserting two concurrent same-account requests are not serialized across the retry-after sleep.

Location

  • src/adapters/anthropic.rsforward_claude_oauth(), lock acquired at the top of the for index in order loop.
  • src/accounts.rsAccountPool::refresh_lock() doc/intent.

Deferred from the PR #70 review as a focused concurrency change rather than an inline fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Fields

    No fields configured for Refactor.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions