Skip to content

Make NetworkingClient.headers thread-safe - #9

Merged
logansease merged 1 commit into
mainfrom
logan/thread-safe-headers
Jul 23, 2026
Merged

Make NetworkingClient.headers thread-safe#9
logansease merged 1 commit into
mainfrom
logan/thread-safe-headers

Conversation

@logansease

Copy link
Copy Markdown
Contributor

headers was a plain stored dictionary read on every request build (and on retries from URLSession/Combine threads) while being rewritten on auth token refresh from arbitrary threads. Swift Dictionary is not thread-safe, so this raced and corrupted the CoW buffer, producing the Crashlytics crashes in NetworkingClient.addHeaders/removeHeaders and NetworkingRequest.__ivar_destroyer (EXC_BAD_ACCESS / EXC_BREAKPOINT / SIGABRT).

  • back headers with an NSLock-protected private store; the public property keeps the exact same get/set surface, so existing consumers compile and behave unchanged
  • add withHeaders(_:) for callers that need an atomic read-modify-write (e.g. merging auth headers) instead of racy get-modify-set
  • add regression tests; the concurrent hammer test segfaults (signal 11) against the previous implementation and passes with the lock

Copilot AI review requested due to automatic review settings July 22, 2026 18:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a production crash caused by concurrent reads/writes to NetworkingClient.headers by making header storage thread-safe and adding regression tests to prevent reintroducing the data race.

Changes:

  • Replaced the stored headers dictionary with a lock-protected private backing store while preserving the existing headers get/set API surface.
  • Added withHeaders(_:) to support atomic read-modify-write updates for callers that need it (e.g., auth header merges).
  • Added concurrency regression tests that exercise the previously-crashing access patterns.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Sources/Networking/NetworkingClient.swift Introduces an NSLock-protected backing store for headers and adds withHeaders(_:) for atomic mutations.
Tests/NetworkingTests/HeadersThreadSafetyTests.swift Adds regression tests that stress concurrent reads/writes and validate atomicity/value semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

headers was a plain stored dictionary read on every request build (and on
retries from URLSession/Combine threads) while being rewritten on auth token
refresh from arbitrary threads. Swift Dictionary is not thread-safe, so this
raced and corrupted the CoW buffer, producing the Crashlytics crashes in
NetworkingClient.addHeaders/removeHeaders and NetworkingRequest.__ivar_destroyer
(EXC_BAD_ACCESS / EXC_BREAKPOINT / SIGABRT).

- back headers with an NSLock-protected private store; the public property
  keeps the exact same get/set surface, so existing consumers compile and
  behave unchanged
- add withHeaders(_:) for callers that need an atomic read-modify-write
  (e.g. merging auth headers) instead of racy get-modify-set
- add regression tests; the concurrent hammer test segfaults (signal 11)
  against the previous implementation and passes with the lock

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@logansease
logansease force-pushed the logan/thread-safe-headers branch from cbfd82c to c8a6aca Compare July 22, 2026 18:44
@logansease
logansease requested a review from btresedder July 22, 2026 18:45
@logansease
logansease merged commit 50f65cd into main Jul 23, 2026
4 checks passed
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.

3 participants