From f3f0c400f64de9fd0094056db59ce500b5002292 Mon Sep 17 00:00:00 2001
From: Philipp Hofmann <philipp.hofmann@sentry.io>
Date: Mon, 13 May 2024 13:18:49 +0200
Subject: [PATCH] fix: Potential deadlock when starting the SDK

Fix a deadlock when two threads access SentrySDK.options and
SentrySDK.currentHub, which used the same object in synchronized.
This problem is fixed now by using two independent locks for
SentrySDK.options and SentrySDK.currentHub.

Fixes GH-3956, GH-3899
---
 CHANGELOG.md                           |  1 +
 Sources/Sentry/SentrySDK.m             | 12 ++++++---
 Tests/SentryTests/SentrySDKTests.swift | 35 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 57c33113be..e0c10f36a3 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 4ee4cf6d51..34fa2c0786 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 ab48269348..d9f5a6a68c 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?