fix: Crash in hasUnfinishedChildSpansToWaitFor (#3821)

Fix a crash in SentryTracer hasUnfinishedChildSpansToWaitFor by not
setting the shouldIgnoreWaitForChildrenCallback to nil when finishing
the tracer.

Fixes GH-3781

CHANGELOG.md

@@ -17,6 +17,7 @@ more about how to use the Metrics API.
 - Fixed certain views getting loaded twice when adding a child view controller (#3753)
 - Fix NSInvalidArgumentException for `NSError sentryErrorWithDomain` (#3819)
 - Again fix runtime error when including Sentry as a static lib (#3820)
+- Fix crash in hasUnfinishedChildSpansToWaitFor (#3821)
 
 ## 8.22.4
 

Sources/Sentry/SentryTracer.m

@@ -547,10 +547,6 @@ - (void)finishInternal
         self.finishCallback = nil;
     }
 
-    if (self.shouldIgnoreWaitForChildrenCallback) {
-        self.shouldIgnoreWaitForChildrenCallback = nil;
-    }
-
     // Prewarming can execute code up to viewDidLoad of a UIViewController, and keep the app in the
     // background. This can lead to auto-generated transactions lasting for minutes or even hours.
     // Therefore, we drop transactions lasting longer than SENTRY_AUTO_TRANSACTION_MAX_DURATION.

Tests/SentryTests/Transaction/SentryTracerTests.swift

@@ -178,7 +178,47 @@ class SentryTracerTests: XCTestCase {
         let span = try XCTUnwrap(spans.first, "Expected first span not to be nil")
         expect(span["timestamp"] as? TimeInterval) == tracerTimestamp.timeIntervalSince1970
 
-        expect(sut.shouldIgnoreWaitForChildrenCallback) == nil
+        expect(sut.shouldIgnoreWaitForChildrenCallback).toNot(beNil(), description: "We must not set the callback to nil because when iterating over the child spans in hasUnfinishedChildSpansToWaitFor this could lead to a crash when shouldIgnoreWaitForChildrenCallback is nil.")
+    }
+
+    /// Reproduces a crash in hasUnfinishedChildSpansToWaitFor; see https://github.com/getsentry/sentry-cocoa/issues/3781
+    /// We used to set the shouldIgnoreWaitForChildrenCallback to nil in finishInternal, which can lead
+    /// to a crash when spans keep finishing while finishInternal is executed because
+    /// shouldIgnoreWaitForChildrenCallback could be then nil in hasUnfinishedChildSpansToWaitFor.
+    func testFinish_ShouldIgnoreWaitForChildrenCallback_DoesNotCrash() throws {
+
+        for _ in 0..<5 {
+            let sut = fixture.getSut()
+
+            let dispatchQueue = DispatchQueue(label: "test", attributes: [.concurrent, .initiallyInactive])
+
+            let expectation = expectation(description: "call everything")
+            expectation.expectedFulfillmentCount = 11
+
+            sut.shouldIgnoreWaitForChildrenCallback = { _ in
+                return true
+            }
+
+            for _ in 0..<1_000 {
+                let child = sut.startChild(operation: self.fixture.transactionOperation)
+                child.finish()
+            }
+
+            dispatchQueue.async {
+                for _ in 0..<10 {
+                    let child = sut.startChild(operation: self.fixture.transactionOperation)
+                    child.finish()
+                    expectation.fulfill()
+                }
+            }
+            dispatchQueue.async {
+                sut.finish()
+                expectation.fulfill()
+            }
+
+            dispatchQueue.activate()
+            wait(for: [expectation], timeout: 1.0)
+        }
     }
 
     func testDeadlineTimer_FinishesTransactionAndChildren() {