Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: simplify dispatch queue wrapper access #3477

Merged
merged 7 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- TTFD waits for next drawn frame (#3505)
- Fix TTID/TTFD for app start transactions (#3512): TTID/TTFD spans and measurements for app start transaction now include the app start duration.
- Fix a race condition in SentryTracer (#3523)
- Missing transactions when not calling `reportFullyDisplayed` (#3477)

## 8.17.2

Expand Down
22 changes: 13 additions & 9 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
@property (nonatomic) BOOL wasFinishCalled;
@property (nonatomic, nullable, strong) NSTimer *deadlineTimer;
@property (nonnull, strong) SentryTracerConfiguration *configuration;
@property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue;

#if SENTRY_TARGET_PROFILING_SUPPORTED
@property (nonatomic) BOOL isProfiling;
Expand Down Expand Up @@ -132,6 +133,7 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti

_startSystemTime = SentryDependencyContainer.sharedInstance.dateProvider.systemTime;
_configuration = configuration;
_dispatchQueue = SentryDependencyContainer.sharedInstance.dispatchQueueWrapper;

self.transactionContext = transactionContext;
_children = [[NSMutableArray alloc] init];
Expand Down Expand Up @@ -198,10 +200,10 @@ - (void)dispatchIdleTimeout
{
@synchronized(_idleTimeoutLock) {
if (_idleTimeoutBlock != NULL) {
[_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock];
[_dispatchQueue dispatchCancel:_idleTimeoutBlock];
}
__weak SentryTracer *weakSelf = self;
_idleTimeoutBlock = [_configuration.dispatchQueueWrapper createDispatchBlock:^{
_idleTimeoutBlock = [_dispatchQueue createDispatchBlock:^{
if (weakSelf == nil) {
SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not doing anything.");
return;
Expand All @@ -215,15 +217,14 @@ - (void)dispatchIdleTimeout
// If the transaction has no children, the SDK will discard it.
[self finishInternal];
} else {
[_configuration.dispatchQueueWrapper dispatchAfter:_configuration.idleTimeout
block:_idleTimeoutBlock];
[_dispatchQueue dispatchAfter:_configuration.idleTimeout block:_idleTimeoutBlock];
}
}
}

- (BOOL)hasIdleTimeout
{
return _configuration.idleTimeout > 0 && _configuration.dispatchQueueWrapper != nil;
return _configuration.idleTimeout > 0;
}

- (BOOL)isAutoGeneratedTransaction
Expand All @@ -235,15 +236,16 @@ - (void)cancelIdleTimeout
{
@synchronized(_idleTimeoutLock) {
if ([self hasIdleTimeout]) {
[_configuration.dispatchQueueWrapper dispatchCancel:_idleTimeoutBlock];
[SentryDependencyContainer.sharedInstance.dispatchQueueWrapper
dispatchCancel:_idleTimeoutBlock];
}
}
}

- (void)startDeadlineTimer
{
__weak SentryTracer *weakSelf = self;
[_configuration.dispatchQueueWrapper dispatchOnMainQueue:^{
[_dispatchQueue dispatchOnMainQueue:^{
weakSelf.deadlineTimer = [weakSelf.configuration.timerFactory
scheduledTimerWithTimeInterval:SENTRY_AUTO_TRANSACTION_DEADLINE
repeats:NO
Expand Down Expand Up @@ -285,7 +287,7 @@ - (void)cancelDeadlineTimer

// 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
[_configuration.dispatchQueueWrapper dispatchOnMainQueue:^{
[_dispatchQueue dispatchOnMainQueue:^{
if (weakSelf == nil) {
SENTRY_LOG_DEBUG(@"WeakSelf is nil. Not invalidating deadlineTimer.");
return;
Expand Down Expand Up @@ -454,7 +456,9 @@ - (void)canBeFinished
}

if (!self.wasFinishCalled || hasUnfinishedChildSpansToWaitFor) {
SENTRY_LOG_DEBUG(@"Span with id %@ has children but isn't waiting for them right now.",
SENTRY_LOG_DEBUG(
@"Span with id %@ has children but hasn't finished yet so isn't waiting "
@"for them right now.",
self.spanId.sentrySpanIdString);
return;
}
Expand Down
6 changes: 1 addition & 5 deletions Sources/Sentry/SentryUIEventTrackerTransactionMode.m
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,16 @@
@interface
SentryUIEventTrackerTransactionMode ()

@property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueueWrapper;
@property (nonatomic, assign) NSTimeInterval idleTimeout;
@property (nullable, nonatomic, strong) NSMutableArray<SentryTracer *> *activeTransactions;

@end

@implementation SentryUIEventTrackerTransactionMode

- (instancetype)initWithDispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper
idleTimeout:(NSTimeInterval)idleTimeout
- (instancetype)initWithIdleTimeout:(NSTimeInterval)idleTimeout
{
if (self = [super init]) {
self.dispatchQueueWrapper = dispatchQueueWrapper;
self.idleTimeout = idleTimeout;
self.activeTransactions = [NSMutableArray new];
}
Expand Down Expand Up @@ -91,7 +88,6 @@ - (void)handleUIEvent:(NSString *)action
SentryTracerConfiguration *config) {
config.idleTimeout = self.idleTimeout;
config.waitForChildren = YES;
config.dispatchQueueWrapper = self.dispatchQueueWrapper;
}]];

SENTRY_LOG_DEBUG(@"Automatically started a new transaction with name: "
Expand Down
7 changes: 2 additions & 5 deletions Sources/Sentry/SentryUIEventTrackingIntegration.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#if SENTRY_HAS_UIKIT

# import <SentryDependencyContainer.h>
# import <SentryLog.h>
# import <SentryNSDataSwizzling.h>
# import <SentryOptions+Private.h>
Expand All @@ -25,10 +24,8 @@ - (BOOL)installWithOptions:(SentryOptions *)options
return NO;
}

SentryDependencyContainer *dependencies = [SentryDependencyContainer sharedInstance];
SentryUIEventTrackerTransactionMode *mode = [[SentryUIEventTrackerTransactionMode alloc]
initWithDispatchQueueWrapper:dependencies.dispatchQueueWrapper
idleTimeout:options.idleTimeout];
SentryUIEventTrackerTransactionMode *mode =
[[SentryUIEventTrackerTransactionMode alloc] initWithIdleTimeout:options.idleTimeout];

self.uiEventTracker = [[SentryUIEventTracker alloc] initWithMode:mode];

Expand Down
5 changes: 0 additions & 5 deletions Sources/Sentry/include/SentryTracerConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic) BOOL waitForChildren;

/**
* A dispatch queue wrapper to intermediate between the tracer and dispatch calls.
*/
@property (nonatomic, strong, nullable) SentryDispatchQueueWrapper *dispatchQueueWrapper;

#if SENTRY_TARGET_PROFILING_SUPPORTED
/**
* Whether to sample a profile corresponding to this transaction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@

#if SENTRY_HAS_UIKIT

@class SentryDispatchQueueWrapper;

NS_ASSUME_NONNULL_BEGIN

@interface SentryUIEventTrackerTransactionMode : NSObject <SentryUIEventTrackerMode>
SENTRY_NO_INIT

- (instancetype)initWithDispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper
idleTimeout:(NSTimeInterval)idleTimeout;
- (instancetype)initWithIdleTimeout:(NSTimeInterval)idleTimeout;

@end

Expand Down
2 changes: 1 addition & 1 deletion Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class SentryProfilerSwiftTests: XCTestCase {
}
SentryDependencyContainer.sharedInstance().dispatchFactory = dispatchFactory
SentryDependencyContainer.sharedInstance().timerFactory = timeoutTimerFactory
SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = dispatchQueueWrapper

systemWrapper.overrides.cpuUsage = NSNumber(value: mockCPUUsage)
systemWrapper.overrides.memoryFootprintBytes = mockMemoryFootprint
Expand All @@ -76,7 +77,6 @@ class SentryProfilerSwiftTests: XCTestCase {
if let idleTimeout = idleTimeout {
$0.idleTimeout = idleTimeout
}
$0.dispatchQueueWrapper = self.dispatchQueueWrapper
$0.waitForChildren = true
$0.timerFactory = self.timeoutTimerFactory
}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class SentryTimeToDisplayTrackerTest: XCTestCase {
return SentryTracer(transactionContext: TransactionContext(operation: "ui.load"), hub: hub, configuration: SentryTracerConfiguration(block: {
$0.waitForChildren = true
$0.timerFactory = self.timerFactory
$0.dispatchQueueWrapper = TestSentryDispatchQueueWrapper()
}))
}
}
Expand All @@ -41,6 +40,7 @@ class SentryTimeToDisplayTrackerTest: XCTestCase {
override func setUp() {
super.setUp()
SentryDependencyContainer.sharedInstance().dateProvider = fixture.dateProvider
SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = TestSentryDispatchQueueWrapper()
}

override func tearDown() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ class SentryUIEventTrackerTests: XCTestCase {
init () {
dispatchQueue.blockBeforeMainBlock = { false }
SentryDependencyContainer.sharedInstance().swizzleWrapper = swizzleWrapper
uiEventTrackerMode = SentryUIEventTrackerTransactionMode(dispatchQueueWrapper: dispatchQueue, idleTimeout: 3.0)
SentryDependencyContainer.sharedInstance().dispatchQueueWrapper = dispatchQueue
uiEventTrackerMode = SentryUIEventTrackerTransactionMode(idleTimeout: 3.0)
}

func getSut() -> SentryUIEventTracker {
Expand Down
Loading
Loading