fix: A crash for user interaction transactions (#3036)

Finishing multiple spans on different threads that cancel and
reschedule the idle timeout for user interaction transactions
could lead to EXC_BAD_ACCESS crashes. This is fixed now by
synchronizing the access to the idleTimeout block.

Fixes GH-3002

Author: philipphofmann

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixed
 
 - Fix crashes in profiling serialization race condition (#3018)
+- Fix a crash for user interaction transactions (#3036)
 
 ## 8.7.1
 

Sources/Sentry/SentryTracer.m

@@ -75,6 +75,7 @@ @implementation SentryTracer {
     NSMutableArray<id<SentrySpan>> *_children;
     BOOL _startTimeChanged;
     NSDate *_originalStartTimestamp;
+    NSObject *_idleTimeoutLock;
 
 #if SENTRY_HAS_UIKIT
     NSUInteger initTotalFrames;
@@ -125,6 +126,7 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti
 
     appStartMeasurement = [self getAppStartMeasurement];
 
+    _idleTimeoutLock = [[NSObject alloc] init];
     if ([self hasIdleTimeout]) {
         [self dispatchIdleTimeout];
     }
@@ -164,26 +166,28 @@ - (nullable SentryTracer *)tracer
 
 - (void)dispatchIdleTimeout
 {
-    if (_idleTimeoutBlock != nil) {
-        [_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock];
-    }
-    __weak SentryTracer *weakSelf = self;
-    _idleTimeoutBlock = [_configuration.dispatchQueueWrapper createDispatchBlock:^{
-        if (weakSelf == nil) {
-            SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not doing anything.");
-            return;
+    @synchronized(_idleTimeoutLock) {
+        if (_idleTimeoutBlock != NULL) {
+            [_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock];
+        }
+        __weak SentryTracer *weakSelf = self;
+        _idleTimeoutBlock = [_configuration.dispatchQueueWrapper createDispatchBlock:^{
+            if (weakSelf == nil) {
+                SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not doing anything.");
+                return;
+            }
+            [weakSelf finishInternal];
+        }];
+
+        if (_idleTimeoutBlock == NULL) {
+            SENTRY_LOG_WARN(@"Couldn't create idle time out block. Can't schedule idle timeout. "
+                            @"Finishing transaction");
+            // If the transaction has no children, the SDK will discard it.
+            [self finishInternal];
+        } else {
+            [_configuration.dispatchQueueWrapper dispatchAfter:_configuration.idleTimeout
+                                                         block:_idleTimeoutBlock];
         }
-        [weakSelf finishInternal];
-    }];
-
-    if (_idleTimeoutBlock == NULL) {
-        SENTRY_LOG_WARN(@"Couldn't create idle time out block. Can't schedule idle timeout. "
-                        @"Finishing transaction");
-        // If the transaction has no children, the SDK will discard it.
-        [self finishInternal];
-    } else {
-        [_configuration.dispatchQueueWrapper dispatchAfter:_configuration.idleTimeout
-                                                     block:_idleTimeoutBlock];
     }
 }
 
@@ -199,8 +203,10 @@ - (BOOL)isAutoGeneratedTransaction
 
 - (void)cancelIdleTimeout
 {
-    if ([self hasIdleTimeout]) {
-        [_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock];
+    @synchronized(_idleTimeoutLock) {
+        if ([self hasIdleTimeout]) {
+            [_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock];
+        }
     }
 }
 

Tests/SentryTests/Transaction/SentryTracerTests.swift

@@ -211,6 +211,15 @@ class SentryTracerTests: XCTestCase {
 
         XCTAssertFalse(fixture.timerWrapper.overrides.timer.isValid)
     }
+    
+    func testDeadlineTimer_MultipleSpansFinishedInParallel() {
+        let sut = fixture.getSut(idleTimeout: 0.01, dispatchQueueWrapper: SentryDispatchQueueWrapper())
+        
+        testConcurrentModifications(writeWork: { _ in
+            let child = sut.startChild(operation: self.fixture.transactionOperation)
+            child.finish()
+        })
+    }
 
     func testFinish_CheckDefaultStatus() {
         let sut = fixture.getSut()