diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f0f954910d..bcb56b36faa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Fix rare memory access issue for auto tracing (#4894). For more details, see issue [#4887](https://github.com/getsentry/sentry-cocoa/issues/4887). - Move assignment of file IO span origin outside of block (#4888) +- Deadline timeout crash in SentryTracer (#4911) ## 8.45.0 diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m index f5f8df6a4c7..0585a3cddeb 100644 --- a/Sources/Sentry/SentryTracer.m +++ b/Sources/Sentry/SentryTracer.m @@ -9,7 +9,6 @@ #import "SentryInternalDefines.h" #import "SentryLog.h" #import "SentryNSDictionarySanitize.h" -#import "SentryNSTimerFactory.h" #import "SentryNoOpSpan.h" #import "SentryOptions+Private.h" #import "SentryProfilingConditionals.h" @@ -72,7 +71,6 @@ @interface SentryTracer () * @c -[finish] doesn't necessarily lead to finishing the tracer, because it could still wait for * child spans to finish if @c waitForChildren is @c YES . */ @property (nonatomic) BOOL wasFinishCalled; -@property (nonatomic, nullable, strong) NSTimer *deadlineTimer; @property (nonnull, strong) SentryTracerConfiguration *configuration; @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue; @property (nonatomic, strong) SentryDebugImageProvider *debugImageProvider; @@ -91,10 +89,11 @@ @implementation SentryTracer { SentryAppStartMeasurement *appStartMeasurement; #endif // SENTRY_HAS_UIKIT NSMutableDictionary *_measurements; + NSObject *_dispatchTimeoutLock; dispatch_block_t _idleTimeoutBlock; + dispatch_block_t _deadlineTimeoutBlock; NSMutableArray> *_children; BOOL _startTimeChanged; - NSObject *_idleTimeoutLock; #if SENTRY_HAS_UIKIT NSUInteger initTotalFrames; @@ -147,10 +146,6 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti _measurements = [[NSMutableDictionary alloc] init]; self.finishStatus = kSentrySpanStatusUndefined; - if (_configuration.timerFactory == nil) { - _configuration.timerFactory = [[SentryNSTimerFactory alloc] init]; - } - #if SENTRY_HAS_UIKIT [hub configureScope:^(SentryScope *scope) { if (scope.currentScreen != nil) { @@ -165,13 +160,13 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti #endif // SENTRY_HAS_UIKIT - _idleTimeoutLock = [[NSObject alloc] init]; + _dispatchTimeoutLock = [[NSObject alloc] init]; if ([self hasIdleTimeout]) { - [self dispatchIdleTimeout]; + [self startIdleTimeout]; } if ([self isAutoGeneratedTransaction]) { - [self startDeadlineTimer]; + [self startDeadlineTimeout]; } #if SENTRY_HAS_UIKIT @@ -213,6 +208,7 @@ - (void)dealloc sentry_discardProfilerForTracer(self.internalID); } #endif // SENTRY_TARGET_PROFILING_SUPPORTED + [self cancelDeadlineTimeout]; } - (nullable SentryTracer *)tracer @@ -220,73 +216,63 @@ - (nullable SentryTracer *)tracer return self; } -- (void)dispatchIdleTimeout -{ - @synchronized(_idleTimeoutLock) { - if (_idleTimeoutBlock != NULL) { - [_dispatchQueue dispatchCancel:_idleTimeoutBlock]; - } - __weak SentryTracer *weakSelf = self; - _idleTimeoutBlock = [_dispatchQueue 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 { - [_dispatchQueue dispatchAfter:_configuration.idleTimeout block:_idleTimeoutBlock]; - } - } -} +#pragma mark - Timeouts - (BOOL)hasIdleTimeout { return _configuration.idleTimeout > 0; } -- (BOOL)isAutoGeneratedTransaction +- (void)startIdleTimeout { - return _configuration.waitForChildren || [self hasIdleTimeout]; + __weak SentryTracer *weakSelf = self; + dispatch_block_t newBlock = [_dispatchQueue createDispatchBlock:^{ + if (weakSelf == nil) { + SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not doing anything."); + return; + } + [weakSelf finishInternal]; + }]; + + @synchronized(_dispatchTimeoutLock) { + [self dispatchTimeout:_idleTimeoutBlock + newBlock:newBlock + interval:_configuration.idleTimeout]; + _idleTimeoutBlock = newBlock; + } } - (void)cancelIdleTimeout { - @synchronized(_idleTimeoutLock) { + @synchronized(_dispatchTimeoutLock) { if ([self hasIdleTimeout]) { - [SentryDependencyContainer.sharedInstance.dispatchQueueWrapper - dispatchCancel:_idleTimeoutBlock]; + [_dispatchQueue dispatchCancel:_idleTimeoutBlock]; } } } -- (void)startDeadlineTimer +- (void)startDeadlineTimeout { __weak SentryTracer *weakSelf = self; - [_dispatchQueue dispatchAsyncOnMainQueue:^{ - weakSelf.deadlineTimer = [weakSelf.configuration.timerFactory - scheduledTimerWithTimeInterval:SENTRY_AUTO_TRANSACTION_DEADLINE - repeats:NO - block:^(NSTimer *_Nonnull timer) { - if (weakSelf == nil) { - SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not calling " - @"deadlineTimerFired."); - return; - } - [weakSelf deadlineTimerFired]; - }]; + dispatch_block_t newBlock = [_dispatchQueue createDispatchBlock:^{ + if (weakSelf == nil) { + SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not doing anything."); + return; + } + [weakSelf deadlineTimeoutExceeded]; }]; + + @synchronized(_dispatchTimeoutLock) { + [self dispatchTimeout:_deadlineTimeoutBlock + newBlock:newBlock + interval:SENTRY_AUTO_TRANSACTION_DEADLINE]; + _deadlineTimeoutBlock = newBlock; + } } -- (void)deadlineTimerFired +- (void)deadlineTimeoutExceeded { - SENTRY_LOG_DEBUG(@"Sentry tracer deadline fired"); + SENTRY_LOG_DEBUG(@"Sentry tracer deadline exceeded"); @synchronized(self) { // This try to minimize a race condition with a proper call to `finishInternal`. if (self.isFinished) { @@ -305,25 +291,38 @@ - (void)deadlineTimerFired [self finishInternal]; } -- (void)cancelDeadlineTimer +- (void)cancelDeadlineTimeout { - if (self.deadlineTimer == nil) { - return; + @synchronized(_dispatchTimeoutLock) { + if (_deadlineTimeoutBlock != NULL) { + [_dispatchQueue dispatchCancel:_deadlineTimeoutBlock]; + _deadlineTimeoutBlock = NULL; + } } +} - // If the main thread is busy the tracer could be deallocated in between. - __weak SentryTracer *weakSelf = self; +- (void)dispatchTimeout:(dispatch_block_t)currentBlock + newBlock:(dispatch_block_t)newBlock + interval:(NSTimeInterval)timeInterval +{ + if (currentBlock != NULL) { + [_dispatchQueue dispatchCancel:currentBlock]; + } - // The timer must be invalidated from the thread on which the timer was installed, see - // https://developer.apple.com/documentation/foundation/nstimer/1415405-invalidate#1770468 - [_dispatchQueue dispatchAsyncOnMainQueue:^{ - if (weakSelf == nil) { - SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not invalidating deadlineTimer."); - return; - } - [weakSelf.deadlineTimer invalidate]; - weakSelf.deadlineTimer = nil; - }]; + if (newBlock == NULL) { + SENTRY_LOG_WARN(@"Couldn't create dispatch after block. Finishing transaction."); + // If the transaction has no children, the SDK will discard it. + [self finishInternal]; + } else { + [_dispatchQueue dispatchAfter:timeInterval block:newBlock]; + } +} + +#pragma mark - Tracer + +- (BOOL)isAutoGeneratedTransaction +{ + return _configuration.waitForChildren || [self hasIdleTimeout]; } - (id)getActiveSpan @@ -499,7 +498,7 @@ - (void)canBeFinished SENTRY_LOG_DEBUG( @"Span with id %@ isn't waiting on children and needs idle timeout dispatched.", self.spanId.sentrySpanIdString); - [self dispatchIdleTimeout]; + [self startIdleTimeout]; return; } @@ -575,7 +574,7 @@ - (void)finishInternal - (BOOL)finishTracer:(SentrySpanStatus)unfinishedSpansFinishStatus shouldCleanUp:(BOOL)shouldCleanUp { if (shouldCleanUp) { - [self cancelDeadlineTimer]; + [self cancelDeadlineTimeout]; } if (self.isFinished) { diff --git a/Sources/Sentry/SentryUIEventTrackerTransactionMode.m b/Sources/Sentry/SentryUIEventTrackerTransactionMode.m index 17da5fd80e7..38b085268b2 100644 --- a/Sources/Sentry/SentryUIEventTrackerTransactionMode.m +++ b/Sources/Sentry/SentryUIEventTrackerTransactionMode.m @@ -49,7 +49,7 @@ - (void)handleUIEvent:(NSString *)action if (sameAction) { SENTRY_LOG_DEBUG(@"Dispatching idle timeout for transaction with span id %@", currentActiveTransaction.spanId.sentrySpanIdString); - [currentActiveTransaction dispatchIdleTimeout]; + [currentActiveTransaction startIdleTimeout]; return; } diff --git a/Sources/Sentry/include/SentryTracer.h b/Sources/Sentry/include/SentryTracer.h index e9eb9e4fb90..74e2f375051 100644 --- a/Sources/Sentry/include/SentryTracer.h +++ b/Sources/Sentry/include/SentryTracer.h @@ -90,7 +90,7 @@ static const NSTimeInterval SENTRY_AUTO_TRANSACTION_MAX_DURATION = 500.0; */ + (nullable SentryTracer *)getTracer:(id)span; -- (void)dispatchIdleTimeout; +- (void)startIdleTimeout; /** * This method is designed to be used when the app crashes. It finishes the transaction and stores diff --git a/Sources/Sentry/include/SentryTracerConfiguration.h b/Sources/Sentry/include/SentryTracerConfiguration.h index 76bc217337c..cb5eee2d037 100644 --- a/Sources/Sentry/include/SentryTracerConfiguration.h +++ b/Sources/Sentry/include/SentryTracerConfiguration.h @@ -9,7 +9,6 @@ NS_ASSUME_NONNULL_BEGIN @class SentryDispatchQueueWrapper; -@class SentryNSTimerFactory; @class SentrySamplerDecision; @interface SentryTracerConfiguration : NSObject @@ -49,11 +48,6 @@ NS_ASSUME_NONNULL_BEGIN */ @property (nonatomic) NSTimeInterval idleTimeout; -/** - * A writer around NSTimer, to make it testable - */ -@property (nonatomic, strong, nullable) SentryNSTimerFactory *timerFactory; - + (SentryTracerConfiguration *)configurationWithBlock: (void (^)(SentryTracerConfiguration *configuration))block; diff --git a/Tests/SentryProfilerTests/SentryProfileTestFixture.swift b/Tests/SentryProfilerTests/SentryProfileTestFixture.swift index 836e0db93e8..34094fe22d7 100644 --- a/Tests/SentryProfilerTests/SentryProfileTestFixture.swift +++ b/Tests/SentryProfilerTests/SentryProfileTestFixture.swift @@ -108,7 +108,6 @@ class SentryProfileTestFixture { $0.idleTimeout = idleTimeout } $0.waitForChildren = true - $0.timerFactory = self.timeoutTimerFactory })) } diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift index 5b29e93acce..b73ace2288f 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift @@ -9,20 +9,19 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { private class Fixture { let dateProvider: TestCurrentDateProvider = TestCurrentDateProvider() - lazy var timerFactory = TestSentryNSTimerFactory(currentDateProvider: dateProvider) - + let dispatchQueue = TestSentryDispatchQueueWrapper() var displayLinkWrapper = TestDisplayLinkWrapper() var framesTracker: SentryFramesTracker init() { - framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, dispatchQueueWrapper: TestSentryDispatchQueueWrapper(), + framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, dispatchQueueWrapper: dispatchQueue, notificationCenter: TestNSNotificationCenterWrapper(), keepDelayedFramesDuration: 0) SentryDependencyContainer.sharedInstance().framesTracker = framesTracker framesTracker.start() } func getSut(name: String, waitForFullDisplay: Bool) -> SentryTimeToDisplayTracker { - return SentryTimeToDisplayTracker(name: name, waitForFullDisplay: waitForFullDisplay, dispatchQueueWrapper: SentryDispatchQueueWrapper()) + return SentryTimeToDisplayTracker(name: name, waitForFullDisplay: waitForFullDisplay, dispatchQueueWrapper: dispatchQueue) } func getTracer() throws -> SentryTracer { @@ -30,7 +29,6 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { let hub = TestHub(client: SentryClient(options: options, fileManager: try TestFileManager(options: options), deleteOldEnvelopeItems: false), andScope: nil) return SentryTracer(transactionContext: TransactionContext(operation: "ui.load"), hub: hub, configuration: SentryTracerConfiguration(block: { $0.waitForChildren = true - $0.timerFactory = self.timerFactory })) } } @@ -40,7 +38,7 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { override func setUp() { super.setUp() SentryDependencyContainer.sharedInstance().dateProvider = fixture.dateProvider - SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = TestSentryDispatchQueueWrapper() + SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = fixture.dispatchQueue } override func tearDown() { @@ -279,8 +277,8 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 11)) - // Timeout for tracer times out - try fixture.timerFactory.fire() + // Deadline timeout for tracer times out + fixture.dispatchQueue.invokeLastDispatchAfter() fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 12)) sut.reportFullyDisplayed() @@ -329,8 +327,8 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 11)) - // Timeout for tracer times out - try fixture.timerFactory.fire() + // Deadline timeout for tracer times out + fixture.dispatchQueue.invokeLastDispatchAfter() fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 12)) sut.reportFullyDisplayed() diff --git a/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift b/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift index 62a5c60b4ad..8cb848c2252 100644 --- a/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift +++ b/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift @@ -288,7 +288,7 @@ class SentryUIEventTrackerTests: XCTestCase { private func assertResetsTimeout(_ firstTransaction: SentryTracer, _ secondTransaction: SentryTracer) { XCTAssertTrue(firstTransaction === secondTransaction) XCTAssertEqual(1, fixture.dispatchQueue.dispatchCancelInvocations.count) - XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(3, fixture.dispatchQueue.dispatchAfterInvocations.count, "Expected 3 dispatchAfter invocations. One for the initial timeout, one for the reset and one for the deadline timeout.") } private func assertFinishesTransaction(_ transaction: SentryTracer, _ operation: String) { diff --git a/Tests/SentryTests/Transaction/SentryTracerTests.swift b/Tests/SentryTests/Transaction/SentryTracerTests.swift index 7c6fe8ab29d..538f24c50c6 100644 --- a/Tests/SentryTests/Transaction/SentryTracerTests.swift +++ b/Tests/SentryTests/Transaction/SentryTracerTests.swift @@ -28,7 +28,6 @@ class SentryTracerTests: XCTestCase { let scope: Scope let dispatchQueue = TestSentryDispatchQueueWrapper() let debugImageProvider = TestDebugImageProvider() - lazy var timerFactory = TestSentryNSTimerFactory(currentDateProvider: currentDateProvider) let transactionName = "Some Transaction" let transactionOperation = "ui.load" @@ -100,7 +99,6 @@ class SentryTracerTests: XCTestCase { customSamplingContext: [:], configuration: SentryTracerConfiguration(block: { $0.waitForChildren = waitForChildren - $0.timerFactory = self.timerFactory $0.idleTimeout = idleTimeout $0.finishMustBeCalled = finishMustBeCalled })) @@ -242,7 +240,7 @@ class SentryTracerTests: XCTestCase { } } - func testDeadlineTimer_FinishesTransactionAndChildren() throws { + func testDeadlineTimerout_FinishesTransactionAndChildren() throws { let sut = fixture.getSut() let child1 = sut.startChild(operation: fixture.transactionOperation) @@ -251,7 +249,7 @@ class SentryTracerTests: XCTestCase { child3.finish() - try fixture.timerFactory.fire() + fixture.dispatchQueue.invokeLastDispatchAfter() assertOneTransactionCaptured(sut) @@ -261,95 +259,45 @@ class SentryTracerTests: XCTestCase { XCTAssertEqual(child3.status, .ok) } - func testDeadlineTimer_StartedAndCancelledOnMainThread() throws { - let sut = fixture.getSut() - let child1 = sut.startChild(operation: fixture.transactionOperation) + func testCancelDeadlineTimeout_TracerDeallocated() throws { - try fixture.timerFactory.fire() - - XCTAssertEqual(sut.status, .deadlineExceeded) - XCTAssertEqual(child1.status, .deadlineExceeded) - XCTAssertEqual(2, fixture.dispatchQueue.blockOnMainInvocations.count, "The NSTimer must be started and cancelled on the main thread.") - } - - func testCancelDeadlineTimer_TracerDeallocated() throws { -#if !os(tvOS) && !os(watchOS) - if sentry_threadSanitizerIsPresent() { - throw XCTSkip("doesn't currently work with TSAN enabled. the tracer instance remains retained by something in the TSAN dylib, and we cannot debug the memory graph with TSAN attached to see what is retaining it. it's likely out of our control.") - } -#endif // !os(tvOS) && !os(watchOS) - - var invocations = 0 - fixture.dispatchQueue.blockBeforeMainBlock = { - // The second invocation the block for invalidating the timer - // which we want to call manually below. - if invocations == 1 { - return false - } - - invocations += 1 - return true - } - - var timer: Timer? weak var weakSut: SentryTracer? // Added internal function so the tracer gets deallocated after executing this function. func startTracer() throws { let sut = fixture.getSut() - timer = try XCTUnwrap(Dynamic(sut).deadlineTimer.asObject as? Timer) weakSut = sut // The TestHub keeps a reference to the tracer in capturedEventsWithScopes. // We set it to nil to avoid that. sut.hub = nil - sut.finish() } try startTracer() XCTAssertNil(weakSut, "sut was not deallocated") - try fixture.timerFactory.fire() - - let invalidateTimerBlock = fixture.dispatchQueue.blockOnMainInvocations.last - if invalidateTimerBlock != nil { - invalidateTimerBlock!() - } - - // Ensure the timer was not invalidated - XCTAssertTrue(timer?.isValid ?? false) + XCTAssertEqual(1, fixture.dispatchQueue.dispatchCancelInvocations.count, "Expected one cancel invocation for the deadline timeout.") } - func testDeadlineTimer_WhenCancelling_IsInvalidated() throws { - let sut = fixture.getSut() - let timer: Timer? = Dynamic(sut).deadlineTimer - _ = sut.startChild(operation: fixture.transactionOperation) - - try fixture.timerFactory.fire() - - XCTAssertNil(Dynamic(sut).deadlineTimer.asObject, "DeadlineTimer should be nil.") - XCTAssertFalse(timer?.isValid ?? true) - } - - func testDeadlineTimer_FiresAfterTracerDeallocated() throws { + func testDeadlineTimeout_FiresAfterTracerDeallocated() throws { // Added internal function so the tracer gets deallocated after executing this function. func startTracer() { _ = fixture.getSut() } startTracer() - try fixture.timerFactory.fire() + fixture.dispatchQueue.invokeLastDispatchAfter() } - func testDeadlineTimerForManualTransaction_NoWorkQueuedOnMainQueue() { + func testDeadlineTimoutForManualTransaction_NoDeadlineTimeoutQueued() { let sut = fixture.getSut(waitForChildren: false, idleTimeout: 0.0) - let invocationsBeforeFinish = fixture.dispatchQueue.blockOnMainInvocations.count + let invocationsBeforeFinish = fixture.dispatchQueue.dispatchAfterInvocations.count sut.finish() - let invocationsAfterFinish = fixture.dispatchQueue.blockOnMainInvocations.count + let invocationsAfterFinish = fixture.dispatchQueue.dispatchAfterInvocations.count XCTAssertEqual(invocationsBeforeFinish, invocationsAfterFinish) } @@ -369,7 +317,7 @@ class SentryTracerTests: XCTestCase { XCTAssertEqual(1, debugImageProvider.getDebugImagesFromCacheForFramesInvocations.count, "Tracer must retrieve debug images from cache.") } - func testDeadlineTimer_ForAutoTransaction_FinishesChildSpans() throws { + func testDeadlineTimeout_ForAutoTransaction_FinishesChildSpans() throws { let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child1 = sut.startChild(operation: fixture.transactionOperation) let child2 = sut.startChild(operation: fixture.transactionOperation) @@ -377,8 +325,7 @@ class SentryTracerTests: XCTestCase { child3.finish() - XCTAssertTrue(fixture.timerFactory.isTimerInitialized()) - try fixture.timerFactory.fire() + fixture.dispatchQueue.invokeLastDispatchAfter() XCTAssertEqual(sut.status, .deadlineExceeded) XCTAssertEqual(child1.status, .deadlineExceeded) @@ -386,7 +333,7 @@ class SentryTracerTests: XCTestCase { XCTAssertEqual(child3.status, .ok) } - func testDeadlineTimer_ForManualTransactions_DoesNotFinishChildSpans() throws { + func testDeadlineTimeout_ForManualTransactions_DoesNotFinishChildSpans() throws { let sut = fixture.getSut(waitForChildren: false) let child1 = sut.startChild(operation: fixture.transactionOperation) let child2 = sut.startChild(operation: fixture.transactionOperation) @@ -394,7 +341,7 @@ class SentryTracerTests: XCTestCase { child3.finish() - XCTAssertFalse(fixture.timerFactory.isTimerInitialized()) + XCTAssertEqual(0, fixture.dispatchQueue.dispatchAfterInvocations.count) XCTAssertEqual(sut.status, .undefined) XCTAssertEqual(child1.status, .undefined) @@ -402,11 +349,11 @@ class SentryTracerTests: XCTestCase { XCTAssertEqual(child3.status, .ok) } - func testDeadlineTimer_Finish_Cancels_Timer() { + func testDeadlineTimeout_Finish_CancelsDeadlineTimeout() { let sut = fixture.getSut() sut.finish() - - XCTAssertFalse(try XCTUnwrap(fixture.timerFactory.overrides).timer.isValid) + + XCTAssertEqual(1, fixture.dispatchQueue.dispatchCancelInvocations.count, "Excpected one cancel invocation for the deadline timeout.") } func testDeadlineTimer_MultipleSpansFinishedInParallel() { @@ -423,7 +370,7 @@ class SentryTracerTests: XCTestCase { func testFinish_CheckDefaultStatus() throws { let sut = fixture.getSut() sut.finish() - try fixture.timerFactory.fire() + fixture.dispatchQueue.invokeLastDispatchAfter() XCTAssertEqual(sut.status, .ok) } @@ -572,8 +519,8 @@ class SentryTracerTests: XCTestCase { sut.startChild(operation: fixture.transactionOperation) - XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) - XCTAssertEqual(1, fixture.dispatchQueue.dispatchCancelInvocations.count) + XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count, "Expected two dispatchAfter invocations one for the idle timeout and one for the deadline timer.") + XCTAssertEqual(1, fixture.dispatchQueue.dispatchCancelInvocations.count, "Expected one cancel invocation for the idle timeout.") } func testIdleTimeoutWithRealDispatchQueue_SpanAdded_IdleTimeoutCancelled() { @@ -597,12 +544,12 @@ class SentryTracerTests: XCTestCase { let child1 = sut.startChild(operation: fixture.transactionOperation) let child2 = sut.startChild(operation: fixture.transactionOperation) - XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) child1.finish() - XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) child2.finish() - XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(3, fixture.dispatchQueue.dispatchAfterInvocations.count) fixture.dispatchQueue.invokeLastDispatchAfter() @@ -611,20 +558,20 @@ class SentryTracerTests: XCTestCase { func testIdleTimeout_ChildSpanFinished_IdleStarted() { let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) - XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) let child = sut.startChild(operation: fixture.transactionOperation) XCTAssertEqual(1, fixture.dispatchQueue.dispatchCancelInvocations.count) child.finish() - XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(3, fixture.dispatchQueue.dispatchAfterInvocations.count) // The grandchild is a NoOp span let grandChild = child.startChild(operation: fixture.transactionOperation) XCTAssertEqual(3, fixture.dispatchQueue.dispatchCancelInvocations.count) grandChild.finish() - XCTAssertEqual(3, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(4, fixture.dispatchQueue.dispatchAfterInvocations.count) XCTAssertEqual(4, fixture.dispatchQueue.dispatchCancelInvocations.count) fixture.dispatchQueue.invokeLastDispatchAfter() @@ -727,17 +674,17 @@ class SentryTracerTests: XCTestCase { let sut = fixture.getSut(idleTimeout: fixture.idleTimeout) let child = sut.startChild(operation: fixture.transactionOperation) - XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) sut.finish() XCTAssertEqual(1, fixture.dispatchQueue.dispatchCancelInvocations.count) - XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) XCTAssertFalse(sut.isFinished) advanceTime(bySeconds: 1) child.finish() - XCTAssertEqual(1, fixture.dispatchQueue.dispatchAfterInvocations.count) + XCTAssertEqual(2, fixture.dispatchQueue.dispatchAfterInvocations.count) XCTAssertTrue(sut.isFinished) } @@ -1305,7 +1252,7 @@ class SentryTracerTests: XCTestCase { func testFinishShouldBeCalled_Timeout_NotCaptured() throws { let sut = fixture.getSut(finishMustBeCalled: true) - try fixture.timerFactory.fire() + fixture.dispatchQueue.invokeLastDispatchAfter() assertTransactionNotCaptured(sut) } @@ -1350,14 +1297,13 @@ class SentryTracerTests: XCTestCase { XCTAssertEqual(1, fixture.client.saveCrashTransactionInvocations.count) } - func testFinishForCrash_DoesNotCancelDeadlineTimer() throws { + func testFinishForCrash_DoesNotCancelDeadlineTimeout() throws { let sut = fixture.getSut() _ = sut.startChild(operation: fixture.transactionOperation) - let timer = try XCTUnwrap(Dynamic(sut).deadlineTimer.asObject as? Timer) sut.finishForCrash() - XCTAssertTrue(timer.isValid) + XCTAssertEqual(0, fixture.dispatchQueue.dispatchCancelInvocations.count, "Expected no cancel invocation for the deadline timeout.") } func testFinishForCrash_DoesNotCallFinishCallback() throws {