Fix: Session replay touch tracking race condition (#4548)

Touch tracker adds events on the main thread, but reads it from a background thread, this could lead to a race condition.

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixes
+
+- Session replay touch tracking race condition (#4548)
+
 ### Features
 
 - Add in_foreground app context to transactions (#4561)
@@ -11,6 +15,7 @@
 
 - impr: Speed up getBinaryImages V2 (#4539). Follow up on (#4435)
 - Make SentryId Sendable (#4553)
+
 ## 8.41.0
 
 ### Features
@@ -26,6 +31,10 @@
 - Finish TTFD when not calling reportFullyDisplayed before binding a new transaction to the scope (#4526).
 - Session replay opacity animation masking (#4532)
 
+### Improvements
+
+- Expose `Sentry._Hybrid` explicit module (#4440)
+
 ## 8.41.0-beta.1
 
 ### Features

Sources/Swift/Integrations/SessionReplay/SentryTouchTracker.swift

@@ -1,5 +1,6 @@
 import Foundation
 #if (os(iOS) || os(tvOS)) && !SENTRY_NO_UIKIT
+@_implementationOnly import _SentryPrivate
 import UIKit
 
 @objcMembers
@@ -34,39 +35,51 @@ class SentryTouchTracker: NSObject {
      * will ever have the same pointer.
      */
     private var trackedTouches = [UITouch: TouchInfo]()
+    private let dispatchQueue: SentryDispatchQueueWrapper
     private var touchId = 1
     private let dateProvider: SentryCurrentDateProvider
     private let scale: CGAffineTransform
 
-    init(dateProvider: SentryCurrentDateProvider, scale: Float) {
+    init(dateProvider: SentryCurrentDateProvider, scale: Float, dispatchQueue: SentryDispatchQueueWrapper) {
         self.dateProvider = dateProvider
         self.scale = CGAffineTransform(scaleX: CGFloat(scale), y: CGFloat(scale))
+        self.dispatchQueue = dispatchQueue
+    }
+
+    convenience init(dateProvider: SentryCurrentDateProvider, scale: Float) {
+        // SentryTouchTracker has it own dispatch queue instead of using the one
+        // from Dependency container to avoid the bottleneck of sharing the same
+        // queue with the rest of the SDK.
+        self.init(dateProvider: dateProvider, scale: scale, dispatchQueue: SentryDispatchQueueWrapper())
     }
 
     func trackTouchFrom(event: UIEvent) {
         guard let touches = event.allTouches else { return }
-        for touch in touches {
-            guard touch.phase == .began || touch.phase == .ended || touch.phase == .moved || touch.phase == .cancelled else { continue }
-            let info = trackedTouches[touch] ?? TouchInfo(id: touchId++)
-            let position = touch.location(in: nil).applying(scale)
-            let newEvent = TouchEvent(x: position.x, y: position.y, timestamp: event.timestamp, phase: touch.phase.toRRWebTouchPhase())
-
-            switch touch.phase {
-            case .began:
-                info.startEvent = newEvent
-            case .ended, .cancelled:
-                info.endEvent = newEvent
-            case .moved:
-                // If the distance between two points is smaller than 10 points, we don't record the second movement.
-                // iOS event polling is fast and will capture any movement; we don't need this granularity for replay.
-                if let last = info.moveEvents.last, touchesDelta(last.point, position) < 10 { continue }
-                info.moveEvents.append(newEvent)
-                debounceEvents(in: info)
-            default:
-                continue
+        let timestamp = event.timestamp
+
+        dispatchQueue.dispatchAsync { [self] in
+            for touch in touches {
+                guard touch.phase == .began || touch.phase == .ended || touch.phase == .moved || touch.phase == .cancelled else { continue }
+                let info = trackedTouches[touch] ?? TouchInfo(id: touchId++)
+                let position = touch.location(in: nil).applying(scale)
+                let newEvent = TouchEvent(x: position.x, y: position.y, timestamp: timestamp, phase: touch.phase.toRRWebTouchPhase())
+
+                switch touch.phase {
+                case .began:
+                    info.startEvent = newEvent
+                case .ended, .cancelled:
+                    info.endEvent = newEvent
+                case .moved:
+                    // If the distance between two points is smaller than 10 points, we don't record the second movement.
+                    // iOS event polling is fast and will capture any movement; we don't need this granularity for replay.
+                    if let last = info.moveEvents.last, touchesDelta(last.point, position) < 10 { continue }
+                    info.moveEvents.append(newEvent)
+                    self.debounceEvents(in: info)
+                default:
+                    continue
+                }
+                trackedTouches[touch] = info
             }
-
-            trackedTouches[touch] = info
         }
     }
 
@@ -103,9 +116,11 @@ class SentryTouchTracker: NSObject {
 
         return abs(abAngle - bcAngle) < 0.05 || abs(abAngle - (2 * .pi - bcAngle)) < 0.05
     }
-
+    
     func flushFinishedEvents() {
-        trackedTouches = trackedTouches.filter { $0.value.endEvent == nil }
+        dispatchQueue.dispatchSync { [self] in
+            trackedTouches = trackedTouches.filter { $0.value.endEvent == nil }
+        }
     }
 
     func replayEvents(from: Date, until: Date) -> [SentryRRWebEvent] {
@@ -116,7 +131,12 @@ class SentryTouchTracker: NSObject {
 
         var result = [SentryRRWebEvent]()
 
-        for info in trackedTouches.values {
+        var touches = [TouchInfo]()
+        dispatchQueue.dispatchSync { [self] in
+            touches = Array(trackedTouches.values)
+        }
+
+        for info in touches {
             if let infoStart = info.startEvent, infoStart.timestamp >= startTimeInterval && infoStart.timestamp <= endTimeInterval {
                 result.append(RRWebTouchEvent(timestamp: now.addingTimeInterval(infoStart.timestamp - uptime), touchId: info.id, x: Float(infoStart.x), y: Float(infoStart.y), phase: .start))
             }

Tests/SentryTests/Integrations/SessionReplay/SentryTouchTrackerTests.swift

@@ -66,8 +66,8 @@ class SentryTouchTrackerTests: XCTestCase {
 
     private var referenceDate = Date(timeIntervalSinceReferenceDate: 0)
 
-    func getSut() -> SentryTouchTracker {
-        return SentryTouchTracker(dateProvider: dateprovider, scale: 1)
+    func getSut(dispatchQueue: SentryDispatchQueueWrapper = TestSentryDispatchQueueWrapper()) -> SentryTouchTracker {
+        return SentryTouchTracker(dateProvider: dateprovider, scale: 1, dispatchQueue: dispatchQueue)
     }
 
     func testTrackTouchFromEvent() {
@@ -300,5 +300,38 @@ class SentryTouchTrackerTests: XCTestCase {
         XCTAssertEqual(positions?.count, 3)
     }
 
+    func testLock() {
+        let sut = getSut(dispatchQueue: SentryDispatchQueueWrapper())
+
+        let addExp = expectation(description: "add")
+        let removeExp = expectation(description: "remove")
+        let readExp = expectation(description: "read")
+
+        DispatchQueue.global().async {
+            for i in 0..<50_000 {
+                let event = MockUIEvent(timestamp: Double(i))
+                let touch = MockUITouch(phase: .ended, location: CGPoint(x: 100, y: 100))
+                event.addTouch(touch)
+                sut.trackTouchFrom(event: event)
+            }
+            addExp.fulfill()
+        }
+
+        DispatchQueue.global().async {
+            for _ in 0..<50_000 {
+                sut.flushFinishedEvents()
+            }
+            removeExp.fulfill()
+        }
+
+        DispatchQueue.global().async {
+            for _ in 0..<50_000 {
+                _ = sut.replayEvents(from: self.referenceDate, until: self.referenceDate.addingTimeInterval(10_000.0))
+            }
+            readExp.fulfill()
+        }
+
+        wait(for: [addExp, removeExp, readExp], timeout: 1)
+    }
 }
 #endif