Skip to content

Commit cc7f629

Browse files
noahsmartinnoahsmartin
authored
fix: Race condition when closing SDK (#5523)
* Fix flaky unit test exit * PR feedback * PR feedback * Update changelog
1 parent 354b020 commit cc7f629

File tree

4 files changed

+65
-26
lines changed

4 files changed

+65
-26
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- Set handled to false for fatal app hangs (#5514)
88
- User feedback widget can now be displayed in SwiftUI apps (#5223)
99
- Fix crash when SentryFileManger is nil (#5535)
10+
- Fix crash when capturing events at the same time `bindClient:` is called from a different thread (#5523)
1011

1112
### Improvements
1213

‎Sentry.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,6 +1085,7 @@
10851085
FA90FAFD2E070A3B008CAAE8 /* SentryURLRequestFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA90FAFC2E070A3B008CAAE8 /* SentryURLRequestFactory.swift */; };
10861086
FAB359982E05D7E90083D5E3 /* SentryEventSwiftHelper.h in Headers */ = {isa = PBXBuildFile; fileRef = FAB359972E05D7E90083D5E3 /* SentryEventSwiftHelper.h */; };
10871087
FAB3599A2E05D8080083D5E3 /* SentryEventSwiftHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = FAB359992E05D8080083D5E3 /* SentryEventSwiftHelper.m */; };
1088+
FAC62B652E15A4100003909D /* SentrySDKThreadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FAC62B642E15A40C0003909D /* SentrySDKThreadTests.swift */; };
10881089
FAEC270E2DF3526000878871 /* SentryUserFeedback.swift in Sources */ = {isa = PBXBuildFile; fileRef = FAEC270D2DF3526000878871 /* SentryUserFeedback.swift */; };
10891090
FAEC273D2DF3933A00878871 /* NSData+Unzip.m in Sources */ = {isa = PBXBuildFile; fileRef = FAEC273C2DF3933200878871 /* NSData+Unzip.m */; };
10901091
/* End PBXBuildFile section */
@@ -2349,6 +2350,7 @@
23492350
FA90FAFC2E070A3B008CAAE8 /* SentryURLRequestFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryURLRequestFactory.swift; sourceTree = "<group>"; };
23502351
FAB359972E05D7E90083D5E3 /* SentryEventSwiftHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryEventSwiftHelper.h; path = include/SentryEventSwiftHelper.h; sourceTree = "<group>"; };
23512352
FAB359992E05D8080083D5E3 /* SentryEventSwiftHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryEventSwiftHelper.m; sourceTree = "<group>"; };
2353+
FAC62B642E15A40C0003909D /* SentrySDKThreadTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySDKThreadTests.swift; sourceTree = "<group>"; };
23522354
FAEC270D2DF3526000878871 /* SentryUserFeedback.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUserFeedback.swift; sourceTree = "<group>"; };
23532355
FAEC273C2DF3933200878871 /* NSData+Unzip.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "NSData+Unzip.m"; sourceTree = "<group>"; };
23542356
FAEC273E2DF393E000878871 /* NSData+Unzip.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "NSData+Unzip.h"; sourceTree = "<group>"; };
@@ -2897,6 +2899,7 @@
28972899
7B0A54552523178700A71716 /* SentryScopeSwiftTests.swift */,
28982900
D8918B212849FA6D00701F9A /* SentrySDKIntegrationTestsBase.swift */,
28992901
7BA8409F24A1EC6E00B718AA /* SentrySDKTests.swift */,
2902+
FAC62B642E15A40C0003909D /* SentrySDKThreadTests.swift */,
29002903
7B0002332477F52D0035FEF1 /* SentrySessionTests.swift */,
29012904
8E70B10025CB8695002B3155 /* SentrySpanIdTests.swift */,
29022905
8ED3D305264DFE700049393B /* SwiftDescriptorTests.swift */,
@@ -5684,6 +5687,7 @@
56845687
63FE722420DA66EC00CDBAE8 /* SentryCrashMonitor_NSException_Tests.m in Sources */,
56855688
7B5AB65D27E48E5200F1D1BA /* TestThreadInspector.swift in Sources */,
56865689
7BF9EF742722A85B00B5BBEF /* SentryClassRegistrator.m in Sources */,
5690+
FAC62B652E15A4100003909D /* SentrySDKThreadTests.swift in Sources */,
56875691
D82915632C85EF0C00A6CDD4 /* SentryViewPhotographerTests.swift in Sources */,
56885692
D8DBE0CA2C0E093000FAB1FD /* SentryTouchTrackerTests.swift in Sources */,
56895693
D8F67AF42BE10F9600C9197B /* SentryUIRedactBuilderTests.swift in Sources */,

‎Sources/Sentry/SentryHub.m‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
@interface SentryHub ()
3838

39-
@property (nullable, nonatomic, strong) SentryClient *client;
39+
@property (nullable, atomic, strong) SentryClient *client;
4040
@property (nullable, nonatomic, strong) SentryScope *scope;
4141
@property (nonatomic) SentryDispatchQueueWrapper *dispatchQueue;
4242
@property (nonatomic, strong) SentryCrashWrapper *crashWrapper;
@@ -90,7 +90,7 @@ - (void)startSession
9090
{
9191
SentrySession *lastSession = nil;
9292
SentryScope *scope = self.scope;
93-
SentryOptions *options = [_client options];
93+
SentryOptions *options = [self.client options];
9494
if (options == nil) {
9595
SENTRY_LOG_ERROR(@"Options of the client are nil. Not starting a session.");
9696
return;
@@ -158,17 +158,17 @@ - (void)endSessionWithTimestamp:(NSDate *)timestamp
158158

159159
- (void)storeCurrentSession:(SentrySession *)session
160160
{
161-
[[_client fileManager] storeCurrentSession:session];
161+
[[self.client fileManager] storeCurrentSession:session];
162162
}
163163

164164
- (void)deleteCurrentSession
165165
{
166-
[[_client fileManager] deleteCurrentSession];
166+
[[self.client fileManager] deleteCurrentSession];
167167
}
168168

169169
- (void)closeCachedSessionWithTimestamp:(nullable NSDate *)timestamp
170170
{
171-
SentryFileManager *fileManager = [_client fileManager];
171+
SentryFileManager *fileManager = [self.client fileManager];
172172
SentrySession *session = [fileManager readCurrentSession];
173173
if (session == nil) {
174174
SENTRY_LOG_DEBUG(@"No cached session to close.");
@@ -177,7 +177,7 @@ - (void)closeCachedSessionWithTimestamp:(nullable NSDate *)timestamp
177177
SENTRY_LOG_DEBUG(@"A cached session was found.");
178178

179179
// Make sure there's a client bound.
180-
SentryClient *client = _client;
180+
SentryClient *client = self.client;
181181
if (client == nil) {
182182
SENTRY_LOG_DEBUG(@"No client bound.");
183183
return;
@@ -204,7 +204,7 @@ - (void)closeCachedSessionWithTimestamp:(nullable NSDate *)timestamp
204204
- (void)captureSession:(nullable SentrySession *)session
205205
{
206206
if (session != nil) {
207-
SentryClient *client = _client;
207+
SentryClient *client = self.client;
208208

209209
if (client.options.diagnosticLevel == kSentryLevelDebug) {
210210
SENTRY_LOG_DEBUG(
@@ -243,7 +243,7 @@ - (void)captureFatalEvent:(SentryEvent *)event withScope:(SentryScope *)scope
243243
{
244244
event.isFatalEvent = YES;
245245

246-
SentryClient *client = _client;
246+
SentryClient *client = self.client;
247247
if (client == nil) {
248248
return;
249249
}
@@ -337,7 +337,7 @@ - (void)saveCrashTransaction:(SentryTransaction *)transaction
337337
return;
338338
}
339339

340-
SentryClient *client = _client;
340+
SentryClient *client = self.client;
341341
if (client != nil) {
342342
[client saveCrashTransaction:transaction withScope:self.scope];
343343
}
@@ -357,7 +357,7 @@ - (SentryId *)captureEvent:(SentryEvent *)event
357357
withScope:(SentryScope *)scope
358358
additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems
359359
{
360-
SentryClient *client = _client;
360+
SentryClient *client = self.client;
361361
if (client != nil) {
362362
return [client captureEvent:event
363363
withScope:scope
@@ -370,10 +370,10 @@ - (void)captureReplayEvent:(SentryReplayEvent *)replayEvent
370370
replayRecording:(SentryReplayRecording *)replayRecording
371371
video:(NSURL *)videoURL
372372
{
373-
[_client captureReplayEvent:replayEvent
374-
replayRecording:replayRecording
375-
video:videoURL
376-
withScope:self.scope];
373+
[self.client captureReplayEvent:replayEvent
374+
replayRecording:replayRecording
375+
video:videoURL
376+
withScope:self.scope];
377377
}
378378

379379
- (id<SentrySpan>)startTransactionWithName:(NSString *)name operation:(NSString *)operation
@@ -489,7 +489,7 @@ - (SentryId *)captureMessage:(NSString *)message
489489

490490
- (SentryId *)captureMessage:(NSString *)message withScope:(SentryScope *)scope
491491
{
492-
SentryClient *client = _client;
492+
SentryClient *client = self.client;
493493
if (client != nil) {
494494
return [client captureMessage:message withScope:scope];
495495
}
@@ -504,7 +504,7 @@ - (SentryId *)captureError:(NSError *)error
504504
- (SentryId *)captureError:(NSError *)error withScope:(SentryScope *)scope
505505
{
506506
SentrySession *currentSession = _session;
507-
SentryClient *client = _client;
507+
SentryClient *client = self.client;
508508
if (client != nil) {
509509
if (currentSession != nil) {
510510
return [client captureError:error
@@ -526,7 +526,7 @@ - (SentryId *)captureException:(NSException *)exception
526526
- (SentryId *)captureException:(NSException *)exception withScope:(SentryScope *)scope
527527
{
528528
SentrySession *currentSession = _session;
529-
SentryClient *client = _client;
529+
SentryClient *client = self.client;
530530
if (client != nil) {
531531
if (currentSession != nil) {
532532
return [client captureException:exception
@@ -542,15 +542,15 @@ - (SentryId *)captureException:(NSException *)exception withScope:(SentryScope *
542542

543543
- (void)captureUserFeedback:(SentryUserFeedback *)userFeedback
544544
{
545-
SentryClient *client = _client;
545+
SentryClient *client = self.client;
546546
if (client != nil) {
547547
[client captureUserFeedback:userFeedback];
548548
}
549549
}
550550

551551
- (void)captureFeedback:(SentryFeedback *)feedback
552552
{
553-
SentryClient *client = _client;
553+
SentryClient *client = self.client;
554554
if (client != nil) {
555555
[client captureFeedback:feedback withScope:self.scope];
556556
}
@@ -575,7 +575,7 @@ - (void)addBreadcrumb:(SentryBreadcrumb *)crumb
575575

576576
- (nullable SentryClient *)getClient
577577
{
578-
return _client;
578+
return self.client;
579579
}
580580

581581
- (void)bindClient:(nullable SentryClient *)client
@@ -587,7 +587,7 @@ - (SentryScope *)scope
587587
{
588588
@synchronized(self) {
589589
if (_scope == nil) {
590-
SentryClient *client = _client;
590+
SentryClient *client = self.client;
591591
if (client != nil) {
592592
_scope = [[SentryScope alloc] initWithMaxBreadcrumbs:client.options.maxBreadcrumbs];
593593
} else {
@@ -695,7 +695,7 @@ - (void)setUser:(nullable SentryUser *)user
695695
*/
696696
- (void)storeEnvelope:(SentryEnvelope *)envelope
697697
{
698-
SentryClient *client = _client;
698+
SentryClient *client = self.client;
699699
if (client == nil) {
700700
return;
701701
}
@@ -707,7 +707,7 @@ - (void)storeEnvelope:(SentryEnvelope *)envelope
707707

708708
- (void)captureEnvelope:(SentryEnvelope *)envelope
709709
{
710-
SentryClient *client = _client;
710+
SentryClient *client = self.client;
711711
if (client == nil) {
712712
return;
713713
}
@@ -732,7 +732,7 @@ - (SentryEnvelope *)updateSessionState:(SentryEnvelope *)envelope
732732
[currentSession
733733
endSessionCrashedWithTimestamp:[SentryDependencyContainer.sharedInstance
734734
.dateProvider date]];
735-
if (_client.options.diagnosticLevel == kSentryLevelDebug) {
735+
if (self.client.options.diagnosticLevel == kSentryLevelDebug) {
736736
SENTRY_LOG_DEBUG(@"Ending session with status: %@",
737737
[self createSessionDebugString:currentSession]);
738738
}
@@ -811,15 +811,15 @@ - (NSString *)createSessionDebugString:(SentrySession *)session
811811

812812
- (void)flush:(NSTimeInterval)timeout
813813
{
814-
SentryClient *client = _client;
814+
SentryClient *client = self.client;
815815
if (client != nil) {
816816
[client flush:timeout];
817817
}
818818
}
819819

820820
- (void)close
821821
{
822-
[_client close];
822+
[self.client close];
823823
SENTRY_LOG_DEBUG(@"Closed the Hub.");
824824
}
825825

‎Tests/SentryTests/SentrySDKThreadTests.swift‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
@testable import Sentry
2+
import XCTest
3+
4+
final class SentrySDKThreadTests: XCTestCase {
5+
func testRaceWhenBindingClient() {
6+
7+
let options = Options()
8+
let sut = SentryHub(client: SentryClient(options: options), andScope: nil)
9+
10+
for _ in 0..<100 {
11+
12+
let exp = expectation(description: "wait")
13+
exp.expectedFulfillmentCount = 100
14+
15+
let queue = DispatchQueue(label: "com.sentry.test_client", qos: .userInteractive, attributes: [.concurrent, .initiallyInactive])
16+
17+
for _ in 0..<100 {
18+
queue.async {
19+
sut.bindClient(SentryClient(options: options))
20+
sut.capture(message: "Test message")
21+
exp.fulfill()
22+
}
23+
}
24+
25+
queue.activate()
26+
27+
for _ in 0..<100 {
28+
sut.bindClient(nil)
29+
}
30+
31+
wait(for: [exp], timeout: 1.0)
32+
}
33+
}
34+
}

0 commit comments

Comments
 (0)