diff --git a/CHANGELOG.md b/CHANGELOG.md index 57c33113be6..e0c10f36a38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Fix data race when calling reportFullyDisplayed from a background thread (#3926) - Ensure flushing envelopes directly after capturing them (#3915) +- Potential deadlock when starting the SDK (#3970) ### Improvements diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 4ee4cf6d513..34fa2c0786c 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -44,11 +44,13 @@ @implementation SentrySDK static SentryHub *_Nullable currentHub; +static NSObject *currentHubLock; static BOOL crashedLastRunCalled; static SentryAppStartMeasurement *sentrySDKappStartMeasurement; static NSObject *sentrySDKappStartMeasurementLock; static BOOL _detectedStartUpCrash; static SentryOptions *_Nullable startOption; +static NSObject *startOptionsLock; /** * @brief We need to keep track of the number of times @c +[startWith...] is called, because our OOM @@ -65,6 +67,8 @@ + (void)initialize { if (self == [SentrySDK class]) { sentrySDKappStartMeasurementLock = [[NSObject alloc] init]; + currentHubLock = [[NSObject alloc] init]; + startOptionsLock = [[NSObject alloc] init]; startInvocations = 0; _detectedStartUpCrash = NO; } @@ -72,7 +76,7 @@ + (void)initialize + (SentryHub *)currentHub { - @synchronized(self) { + @synchronized(currentHubLock) { if (nil == currentHub) { currentHub = [[SentryHub alloc] initWithClient:nil andScope:nil]; } @@ -82,7 +86,7 @@ + (SentryHub *)currentHub + (nullable SentryOptions *)options { - @synchronized(self) { + @synchronized(startOptionsLock) { return startOption; } } @@ -90,14 +94,14 @@ + (nullable SentryOptions *)options /** Internal, only needed for testing. */ + (void)setCurrentHub:(nullable SentryHub *)hub { - @synchronized(self) { + @synchronized(currentHubLock) { currentHub = hub; } } /** Internal, only needed for testing. */ + (void)setStartOptions:(nullable SentryOptions *)options { - @synchronized(self) { + @synchronized(startOptionsLock) { startOption = options; } } diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index ab48269348d..d9f5a6a68c1 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -887,6 +887,41 @@ class SentrySDKTests: XCTestCase { } } +/// Tests in this class aren't part of SentrySDKTests because we need would need to undo a bunch of operations +/// that are done in the setup. +class SentrySDKWithSetupTests: XCTestCase { + + func testAccessingHubAndOptions_NoDeadlock() { + SentryLog.withOutLogs { + + let concurrentQueue = DispatchQueue(label: "concurrent", attributes: .concurrent) + + let expectation = expectation(description: "no deadlock") + expectation.expectedFulfillmentCount = 20 + + SentrySDK.setStart(Options()) + + for _ in 0..<10 { + concurrentQueue.async { + SentrySDK.currentHub().capture(message: "mess") + SentrySDK.setCurrentHub(nil) + + expectation.fulfill() + } + + concurrentQueue.async { + let hub = SentryHub(client: nil, andScope: nil) + expect(hub) != nil + + expectation.fulfill() + } + } + + wait(for: [expectation], timeout: 5.0) + } + } +} + public class MainThreadTestIntegration: NSObject, SentryIntegrationProtocol { static var expectation: XCTestExpectation?