Skip to content

fix: Avoid mutating live network requests - #8650

Merged
philprime merged 8 commits into
mainfrom
philprime/network-instrumentation-8519
Aug 6, 2026
Merged

fix: Avoid mutating live network requests#8650
philprime merged 8 commits into
mainfrom
philprime/network-instrumentation-8519

Conversation

@philprime

@philprime philprime commented Aug 3, 2026

Copy link
Copy Markdown
Member

📜 Description

Copy URLSessionTask.currentRequest before adding trace propagation headers and install the copied request through setCurrentRequest:. This prevents Sentry from mutating a request that CFNetwork may be reading concurrently.

Add regression coverage that verifies a mutable current request remains unchanged while the replacement request receives the trace headers.

💡 Motivation and Context

Network instrumentation previously mutated mutable currentRequest instances in place. Repeated or concurrent task resumes could race CFNetwork's access to the same request header storage and crash in CoreFoundation or CFNetwork.

The Swift network tracker conversion does not remove this path because header injection still delegates to SentryTracePropagation.

Fixes #8519

💚 How did you test it?

  • make format
  • make analyze
  • make build-ios FOR_AGENTS=true
  • make build-macos FOR_AGENTS=true
  • make test-ios FOR_AGENTS=true ONLY_TESTING=SentryTests/SentryTracePropagationTests,SentryTests/SentryNetworkTrackerTests

The focused iOS run executed 100 tests with 0 failures. The equivalent macOS test command is currently blocked by unrelated test-target compilation errors in SentryOptionsDictionaryTests, SentrySpotlightTransportTests, and SentryTransportFactoryTests on the base branch.

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.
  • If I added a new public API, I also added it to the SentryObjC wrapper.

@philprime philprime self-assigned this Aug 3, 2026
@philprime philprime added the run-full-ci Allows gated GitHub Action workflows to run for a labelled pull request label Aug 3, 2026
Copy the task's current request before adding trace propagation headers so CFNetwork never observes concurrent in-place mutations.

Fixes #8519
@philprime
philprime force-pushed the philprime/network-instrumentation-8519 branch from 00b2ec6 to 005ad55 Compare August 3, 2026 09:53
@philprime
philprime changed the base branch from philprime/network-tracker-conversion to main August 3, 2026 09:53
@philprime
philprime marked this pull request as ready for review August 3, 2026 09:55
Comment thread Sources/Sentry/SentryTracePropagation.m
@sentry

sentry Bot commented Aug 3, 2026

Copy link
Copy Markdown

📲 Install Builds

iOS

🔗 App Name App ID Version Configuration
SDK-Size io.sentry.sample.SDK-Size 9.25.0 (1) Release

⚙️ sentry-cocoa Build Distribution Settings

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1235.51 ms 1264.82 ms 29.31 ms
Size 24.14 KiB 1.27 MiB 1.25 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dcb456a 1220.98 ms 1246.74 ms 25.76 ms
952f116 1221.55 ms 1248.72 ms 27.17 ms
706c4c0 1213.06 ms 1252.94 ms 39.88 ms
b6b0d1d 1236.69 ms 1270.22 ms 33.52 ms
8693a8f 1222.37 ms 1249.31 ms 26.95 ms
52f26a1 1227.29 ms 1261.50 ms 34.21 ms
b6fa517 1218.83 ms 1257.47 ms 38.63 ms
7f28cd4 1215.77 ms 1242.98 ms 27.21 ms
5df460d 1230.49 ms 1260.69 ms 30.20 ms
f2e9a12 1223.86 ms 1264.56 ms 40.70 ms

App size

Revision Plain With Sentry Diff
dcb456a 24.14 KiB 1.22 MiB 1.20 MiB
952f116 24.14 KiB 1.24 MiB 1.21 MiB
706c4c0 24.14 KiB 1.26 MiB 1.23 MiB
b6b0d1d 24.14 KiB 1.18 MiB 1.16 MiB
8693a8f 24.14 KiB 1.16 MiB 1.13 MiB
52f26a1 24.14 KiB 1.19 MiB 1.17 MiB
b6fa517 24.14 KiB 1.14 MiB 1.12 MiB
7f28cd4 24.14 KiB 1.23 MiB 1.21 MiB
5df460d 24.14 KiB 1.22 MiB 1.20 MiB
f2e9a12 24.14 KiB 1.27 MiB 1.25 MiB

Previous results on branch: philprime/network-instrumentation-8519

Startup times

Revision Plain With Sentry Diff
7d22285 1211.29 ms 1255.36 ms 44.06 ms
65f5b6f 1206.22 ms 1244.58 ms 38.36 ms
be2121d 1233.62 ms 1265.92 ms 32.29 ms
dca42ff 1205.02 ms 1254.36 ms 49.34 ms

App size

Revision Plain With Sentry Diff
7d22285 24.14 KiB 1.26 MiB 1.23 MiB
65f5b6f 24.14 KiB 1.27 MiB 1.25 MiB
be2121d 24.14 KiB 1.27 MiB 1.25 MiB
dca42ff 24.14 KiB 1.26 MiB 1.23 MiB

@philprime
philprime marked this pull request as draft August 3, 2026 10:47
Model setCurrentRequest: on the download and upload task mocks to match
real NSURLSession task behavior, and add tests asserting trace headers
are applied for both task types.

Refs #8519
…trumentation-8519

# Conflicts:
#	Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerIntegrationTestServerTests.swift
@philprime
philprime marked this pull request as ready for review August 5, 2026 12:44

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a7cde10. Configure here.

@philipphofmann philipphofmann left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM thanks

Comment thread CHANGELOG.md Outdated
@philprime
philprime enabled auto-merge (squash) August 6, 2026 12:56
@philprime
philprime merged commit 56dbf49 into main Aug 6, 2026
251 of 253 checks passed
@philprime
philprime deleted the philprime/network-instrumentation-8519 branch August 6, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved run-full-ci Allows gated GitHub Action workflows to run for a labelled pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash: network instrumentation mutates a live NSURLSessionTask.currentRequest in place, racing CFNetwork (SIGSEGV in SentryTracePropagation)

2 participants