Skip to content

Conversation

@philprime
Copy link
Member

📜 Description

This PR fixes a crash caused by accessing UITouch instances from a background thread in . The fix extracts all necessary data from UITouch objects on the main thread before dispatching to the background queue, then processes only thread-safe data structures on the background thread.

Changes:

  • Added ExtractedTouchData struct to hold thread-safe touch information
  • Changed trackedTouches dictionary to use ObjectIdentifier as key instead of UITouch
  • Extract touch.location, touch.phase, and ObjectIdentifier on calling thread (main thread)
  • Process only thread-safe data structures (structs, enums, value types) on background queue

💡 Motivation and Context

Fixes #6231

The issue was that trackTouchFrom(event:) dispatched to a background queue and then accessed UITouch properties (touch.location(in: nil) and touch.phase) from that background thread. This violates UIKit's thread-safety requirements and caused EXC_BAD_ACCESS crashes.

The stack trace from the issue showed:

EXC_BAD_ACCESS: Attempted to dereference garbage pointer 0x400000000000bad0.
...
2  REDACTED +0xcbebc0                closure #1 in SentryTouchTracker.trackTouchFrom(event:)
...
5  libdispatch.dylib +0x1ad8       __dispatch_call_block_and_release

💚 How did you test it?

  • All 12 existing tests in SentryTouchTrackerTests pass successfully
  • Verified no UIKit objects cross the thread boundary by code review
  • All UITouch accesses now happen on the calling thread before dispatching

📝 Checklist

  • I added tests to verify the changes. (Existing tests verify behavior remains unchanged)
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed. (No doc changes needed - internal fix)
  • I updated the wizard if needed. (No wizard changes 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.

Extract UITouch data on main thread before dispatching to background queue
to prevent EXC_BAD_ACCESS crashes when accessing UIKit objects from
background threads.

- Added ExtractedTouchData struct to hold thread-safe touch information
- Changed trackedTouches dictionary to use ObjectIdentifier as key instead of UITouch
- Extract touch.location, touch.phase, and ObjectIdentifier on calling thread
- Process only thread-safe data structures on background queue

Fixes #6231
@philprime philprime changed the title Fix UITouch background thread access in SentryTouchTracker fix: Fix UITouch background thread access in SentryTouchTracker Oct 29, 2025
@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.097%. Comparing base (5fce94f) to head (e6a3249).
⚠️ Report is 1 commits behind head on v8.x.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              v8.x     #6584       +/-   ##
=============================================
+ Coverage   85.196%   86.097%   +0.901%     
=============================================
  Files          441       441               
  Lines        27426     27470       +44     
  Branches     10368     11933     +1565     
=============================================
+ Hits         23366     23651      +285     
+ Misses        4018      3773      -245     
- Partials        42        46        +4     
Files with missing lines Coverage Δ
...ntegrations/SessionReplay/SentryTouchTracker.swift 100.000% <100.000%> (ø)

... and 33 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5fce94f...e6a3249. Read the comment docs.

- Added testObjectIdentifierCollision_NewTouchGetsNewId to demonstrate the bug
- Fix prevents corrupted touch data when UITouch memory is reused
- When .began phase is detected, always create new TouchInfo
- Updated CHANGELOG.md with combined entry for both fixes
cursor[bot]

This comment was marked as outdated.

- Removed trackedTouches dictionary entirely
- Use single allTouchInfos array with O(n) lookup
- Store ObjectIdentifier in TouchInfo for matching
- Eliminates collision/orphaning complexity
- Reduced cyclomatic complexity (no more linter warnings)
- All 14 tests pass
@github-actions
Copy link
Contributor

github-actions bot commented Oct 29, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1222.43 ms 1246.13 ms 23.69 ms
Size 23.75 KiB 992.25 KiB 968.50 KiB

Baseline results on branch: v8.x

Startup times

Revision Plain With Sentry Diff
ab82dac 1213.12 ms 1240.92 ms 27.80 ms
c11a8e0 1203.00 ms 1223.23 ms 20.23 ms
ab82dac 1249.73 ms 1272.69 ms 22.96 ms
41834f1 1235.15 ms 1256.31 ms 21.17 ms
f76f6bf 1207.70 ms 1233.27 ms 25.57 ms
b66be9b 1218.22 ms 1244.19 ms 25.96 ms
5fce94f 1226.31 ms 1246.82 ms 20.50 ms
a7a0a2b 1218.61 ms 1248.69 ms 30.08 ms
e537c90 1226.22 ms 1256.64 ms 30.41 ms
3af1ae9 1225.60 ms 1252.65 ms 27.05 ms

App size

Revision Plain With Sentry Diff
ab82dac 23.75 KiB 991.86 KiB 968.11 KiB
c11a8e0 23.75 KiB 991.86 KiB 968.12 KiB
ab82dac 23.75 KiB 991.85 KiB 968.10 KiB
41834f1 23.75 KiB 991.88 KiB 968.13 KiB
f76f6bf 23.74 KiB 981.30 KiB 957.56 KiB
b66be9b 23.75 KiB 996.03 KiB 972.28 KiB
5fce94f 23.75 KiB 991.62 KiB 967.87 KiB
a7a0a2b 23.75 KiB 996.04 KiB 972.29 KiB
e537c90 23.75 KiB 992.03 KiB 968.28 KiB
3af1ae9 23.74 KiB 981.29 KiB 957.55 KiB

Previous results on branch: philprime/removeuitouch-backgruond

Startup times

Revision Plain With Sentry Diff
1568203 1212.58 ms 1241.50 ms 28.92 ms

App size

Revision Plain With Sentry Diff
1568203 23.75 KiB 989.86 KiB 966.11 KiB

Copy link
Contributor

@itaybre itaybre left a comment

Choose a reason for hiding this comment

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

lgtm

@philprime philprime enabled auto-merge (squash) October 29, 2025 13:35
@philprime philprime disabled auto-merge October 29, 2025 13:37
Address PR feedback - use dictionary for O(1) lookup instead of O(n) array search.

- trackedTouches: O(1) dictionary lookup for active touches
- orphanedTouches: array for collision-replaced touches
- When .began collides, move old touch to orphanedTouches before replacing
- Extract processTouchEvent method to reduce cyclomatic complexity
- replayEvents returns both active + orphaned touches
- Maintains performance while preserving all events
- All 14 tests pass
@philprime philprime enabled auto-merge (squash) October 29, 2025 16:31
@itaybre
Copy link
Contributor

itaybre commented Oct 30, 2025

CI is being very slow, I am backporting some fixes on #6604 to help this PR and the later release

@philprime philprime merged commit b79b552 into v8.x Oct 30, 2025
296 of 319 checks passed
@philprime philprime deleted the philprime/removeuitouch-backgruond branch October 30, 2025 00:39
itaybre added a commit that referenced this pull request Oct 30, 2025
* test: Ensure test is server running (#6300)

Ensure that the test server is running with a retry mechanism to avoid
flakiness in CI.

* ci: Add v8.x branch to workflows (#6321)

* chore: Explain v8 branch (#6323)

Add decision log entry for v8 branch and explain how to release from it.

* ci(v8): Bump Xcode from 26.0 to 26.0.1 (#6394)

* docs: Add note to README with reference to v9 on main branch (#6402)

* fix: Wrong Frame Delay when becoming active (#6393)

The SDK reported false frame delay statistics when it moved from the
background to the foreground, which also led to falsely reported app
hangs.

Fixes GH-6345

* fix(session-replay): Add detection for potential PII leaks disabling session replay (#6389)

* release: 8.57.0

* chore: Bump simulators to 26.1 (#6578)

* fix: Fix crash when last replay info is missing some keys (#6577)

* fix: Fix crash when last replay info is missing keys

* Update changelog

* fix: Disable SessionSentryReplayIntegration if the environment is unsafe (#6573)

* fix: Disable SessionSentryReplayIntegration if the environment is unsafe

* Simplify shouldEnableSessionReplay

* Rename test

* Add log message

* Update changelog

* Safely unwrap SentryOptions

* fix: Fix UITouch background thread access in SentryTouchTracker (#6584)

* release: 8.57.1

* Fix merge issues

* Fix another merge issue

* More merge conflicts

* Add SentryThreadInspector again

* Fix tests on iOS 26

* Add `enableSessionReplayInUnreliableEnvironment`

---------

Co-authored-by: Philipp Hofmann <[email protected]>
Co-authored-by: Philip Niedertscheider <[email protected]>
Co-authored-by: getsentry-bot <[email protected]>
Co-authored-by: getsentry-bot <[email protected]>
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.

5 participants