Skip to content

[P1] Protect Client access token reads and writes from data races #131

Description

@UjjwalGupta49

Summary

Client.accessToken is read and written without synchronization. When session generation or token renewal overlaps with normal API requests, the race detector reports concurrent access to the field.

This can happen in a realistic client lifecycle: one goroutine renews an expired token while other goroutines continue making API calls through the same Client.

Cause

SetAccessToken writes directly to the mutable field:

GenerateSession and RenewAccessToken call that setter after successful responses:

At the same time, each request helper reads accessToken while constructing the authorization header:

InvalidateAccessToken also reads the field directly:

The test suite makes the race reproducible because parallel API subtests share one client:

Impact

Concurrent unsynchronized access is a Go data race. It makes the behavior undefined under the Go memory model and may cause requests made during token rotation to use inconsistent or stale authentication state.

Running:

go test -race -count=1 ./...

reports races between SetAccessToken and the request helpers. Running the same suite with -parallel=1 passes, which isolates the failure to concurrent access.

Suggested fix

Keep the change local to Client:

  1. Add a small sync.RWMutex that protects accessToken.
  2. Lock writes in SetAccessToken.
  3. Read a single token snapshot under a read lock before building request headers or invalidating the current token.
  4. Update the tests so token-mutating cases do not accidentally share mutable authentication state with unrelated parallel subtests, or give each parallel case its own client.
  5. Keep the race-detector command in validation for this behavior.

An atomic container would also work, but a mutex is simpler here and makes ownership explicit.

Why this is pattern-aligned

The token is already owned by Client, and all affected reads flow through the existing request helpers. Protecting the field at that boundary keeps the fix in the files that already own authentication state.

This does not require new middleware, a separate token manager, changes to public request APIs, or broad lifecycle abstractions. The normal request path remains the same: take a token snapshot, construct the existing header, and send the request.

Suggested tests

  • Run go test -race -count=1 ./... and confirm the existing race is gone.
  • Add a focused test that rotates the access token while requests read it.
  • Verify authorization headers contain one complete token value.
  • Keep the existing non-race test suite passing.

Acceptance criteria

  • Concurrent token renewal/session generation and request construction do not trigger the race detector.
  • SetAccessToken, request authorization, and access-token invalidation use synchronized access.
  • Existing public APIs and authentication behavior remain unchanged.
  • go test ./..., go vet ./..., and go test -race ./... pass.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions