Skip to content

Commit

Permalink
fix: Crash in Tracer for idle timeout (#2834)
Browse files Browse the repository at this point in the history
dispatch_block_create can return NULL. If it does, we finish
the transaction, and don't schedule the timeout.

Fixes GH-2832, GH-2769
  • Loading branch information
philipphofmann authored Mar 24, 2023
1 parent cce35c6 commit 85b619d
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- View hierarchy not sent for crashes (#2781)
- Crash in Tracer for idle timeout (#2834)

## 8.3.2

Expand Down
9 changes: 9 additions & 0 deletions SentryTestUtils/TestSentryDispatchQueueWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,13 @@ public class TestSentryDispatchQueueWrapper: SentryDispatchQueueWrapper {
public override func dispatchOnce(_ predicate: UnsafeMutablePointer<Int>, block: @escaping () -> Void) {
block()
}

public var createDispatchBlockReturnsNULL = false
public override func createDispatchBlock(_ block: @escaping () -> Void) -> (() -> Void)? {
if createDispatchBlockReturnsNULL {
return nil
}
return super.createDispatchBlock(block)
}

}
5 changes: 5 additions & 0 deletions Sources/Sentry/SentryDispatchQueueWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ - (void)dispatchOnce:(dispatch_once_t *)predicate block:(void (^)(void))block
dispatch_once(predicate, block);
}

- (nullable dispatch_block_t)createDispatchBlock:(void (^)(void))block
{
return dispatch_block_create(0, block);
}

@end

NS_ASSUME_NONNULL_END
13 changes: 10 additions & 3 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,21 @@ - (void)dispatchIdleTimeout
[self.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock];
}
__weak SentryTracer *weakSelf = self;
_idleTimeoutBlock = dispatch_block_create(0, ^{
_idleTimeoutBlock = [self.dispatchQueueWrapper createDispatchBlock:^{
if (weakSelf == nil) {
SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not doing anything.");
return;
}
[weakSelf finishInternal];
});
[self.dispatchQueueWrapper dispatchAfter:self.idleTimeout block:_idleTimeoutBlock];
}];
if (_idleTimeoutBlock == NULL) {
SENTRY_LOG_WARN(@"Couln'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 {
[self.dispatchQueueWrapper dispatchAfter:self.idleTimeout block:_idleTimeoutBlock];
}
}

- (BOOL)hasIdleTimeout
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryDispatchQueueWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ NS_ASSUME_NONNULL_BEGIN

- (void)dispatchOnce:(dispatch_once_t *)predicate block:(void (^)(void))block;

- (nullable dispatch_block_t)createDispatchBlock:(void (^)(void))block;

@end

NS_ASSUME_NONNULL_END
20 changes: 20 additions & 0 deletions Tests/SentryTests/Performance/SentryTracerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,26 @@ class SentryTracerTests: XCTestCase {
XCTAssertEqual(status, sut.status)
}

func testIdleTransaction_CreatingDispatchBlockFails_NoTransactionCaptured() {
fixture.dispatchQueue.createDispatchBlockReturnsNULL = true

let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue)

assertTransactionNotCaptured(sut)
}

func testIdleTransaction_CreatingDispatchBlockFailsForFirstChild_FinishesTransaction() {
let sut = fixture.getSut(idleTimeout: fixture.idleTimeout, dispatchQueueWrapper: fixture.dispatchQueue)

fixture.dispatchQueue.createDispatchBlockReturnsNULL = true

let child = sut.startChild(operation: fixture.transactionOperation)
advanceTime(bySeconds: 0.1)
child.finish()

assertOneTransactionCaptured(sut)
}

func testWaitForChildrenTransactionWithStatus_OverwriteStatusInFinish() {
let sut = fixture.getSut()
sut.status = .aborted
Expand Down

0 comments on commit 85b619d

Please sign in to comment.