Merge b1af117 into 62c15d4

.github/workflows/test.yml

@@ -248,12 +248,10 @@ jobs:
   # We don't run all unit tests with Thread Sanitizer enabled because
   # that adds a significant overhead.
   thread-sanitizer:
-    # Disabled for now, see https://github.com/getsentry/sentry-cocoa/issues/3200
-    if: false 
     name: Unit iOS - Thread Sanitizer
     runs-on: macos-13
     # When there are threading issues the tests sometimes keep hanging
-    timeout-minutes: 20
+    timeout-minutes: 60
     needs: [build-test-server]
 
     steps:

Sources/Sentry/SentryANRTracker.m

@@ -71,6 +71,8 @@ - (void)detectANRs
     NSInteger reportThreshold = 5;
     NSTimeInterval sleepInterval = self.timeoutInterval / reportThreshold;
 
+    SentryCurrentDateProvider *dateProvider = SentryDependencyContainer.sharedInstance.dateProvider;
+
     // Canceling the thread can take up to sleepInterval.
     while (YES) {
         @synchronized(threadLock) {
@@ -79,8 +81,7 @@ - (void)detectANRs
             }
         }
 
-        NSDate *blockDeadline = [[SentryDependencyContainer.sharedInstance.dateProvider date]
-            dateByAddingTimeInterval:self.timeoutInterval];
+        NSDate *blockDeadline = [[dateProvider date] dateByAddingTimeInterval:self.timeoutInterval];
 
         atomic_fetch_add_explicit(&ticksSinceUiUpdate, 1, memory_order_relaxed);
 
@@ -107,8 +108,7 @@ - (void)detectANRs
         // an ANR. If the app gets suspended this thread could sleep and wake up again. To avoid
         // false positives, we don't report ANRs if the delta is too big.
         NSTimeInterval deltaFromNowToBlockDeadline =
-            [[SentryDependencyContainer.sharedInstance.dateProvider date]
-                timeIntervalSinceDate:blockDeadline];
+            [[dateProvider date] timeIntervalSinceDate:blockDeadline];
 
         if (deltaFromNowToBlockDeadline >= self.timeoutInterval) {
             SENTRY_LOG_DEBUG(
@@ -165,8 +165,8 @@ - (void)addListener:(id<SentryANRTrackerDelegate>)listener
     @synchronized(self.listeners) {
         [self.listeners addObject:listener];
 
-        if (self.listeners.count > 0 && state == kSentryANRTrackerNotRunning) {
-            @synchronized(threadLock) {
+        @synchronized(threadLock) {
+            if (self.listeners.count > 0 && state == kSentryANRTrackerNotRunning) {
                 if (state == kSentryANRTrackerNotRunning) {
                     state = kSentryANRTrackerStarting;
                     [NSThread detachNewThreadSelector:@selector(detectANRs)

Sources/Sentry/SentryFileManager.m

@@ -252,10 +252,10 @@ - (void)removeFileAtPath:(NSString *)path
 - (NSString *)storeEnvelope:(SentryEnvelope *)envelope
 {
     NSData *envelopeData = [SentrySerialization dataWithEnvelope:envelope error:nil];
-    NSString *path =
-        [self.envelopesPath stringByAppendingPathComponent:[self uniqueAscendingJsonName]];
 
     @synchronized(self) {
+        NSString *path =
+            [self.envelopesPath stringByAppendingPathComponent:[self uniqueAscendingJsonName]];
         SENTRY_LOG_DEBUG(@"Writing envelope to path: %@", path);
 
         if (![self writeData:envelopeData toPath:path]) {

Sources/Sentry/SentryLog.m

@@ -12,11 +12,19 @@ @implementation SentryLog
 static BOOL isDebug = YES;
 static SentryLevel diagnosticLevel = kSentryLevelError;
 static SentryLogOutput *logOutput;
+static NSObject *logConfigureLock;
+
++ (void)initialize
+{
+    logConfigureLock = [[NSObject alloc] init];
+}
 
 + (void)configure:(BOOL)debug diagnosticLevel:(SentryLevel)level
 {
-    isDebug = debug;
-    diagnosticLevel = level;
+    @synchronized(logConfigureLock) {
+        isDebug = debug;
+        diagnosticLevel = level;
+    }
 }
 
 + (void)logWithMessage:(NSString *)message andLevel:(SentryLevel)level
@@ -33,7 +41,9 @@ + (void)logWithMessage:(NSString *)message andLevel:(SentryLevel)level
 
 + (BOOL)willLogAtLevel:(SentryLevel)level
 {
-    return isDebug && level != kSentryLevelNone && level >= diagnosticLevel;
+    @synchronized(logConfigureLock) {
+        return isDebug && level != kSentryLevelNone && level >= diagnosticLevel;
+    }
 }
 
 // Internal and only needed for testing.

Sources/Sentry/SentryNetworkTracker.m

@@ -334,10 +334,12 @@ - (void)urlSessionTask:(NSURLSessionTask *)sessionTask setState:(NSURLSessionTas
 
 - (void)captureFailedRequests:(NSURLSessionTask *)sessionTask
 {
-    if (!self.isCaptureFailedRequestsEnabled) {
-        SENTRY_LOG_DEBUG(
-            @"captureFailedRequestsEnabled is disabled, not capturing HTTP Client errors.");
-        return;
+    @synchronized(self) {
+        if (!self.isCaptureFailedRequestsEnabled) {
+            SENTRY_LOG_DEBUG(
+                @"captureFailedRequestsEnabled is disabled, not capturing HTTP Client errors.");
+            return;
+        }
     }
 
     // if request or response are null, we can't raise the event

Sources/Sentry/SentryProfiler.mm

@@ -61,6 +61,20 @@
 std::mutex _gProfilerLock;
 SentryProfiler *_Nullable _gCurrentProfiler;
 
+BOOL
+threadSanitizerIsPresent(void)
+{
+#    if defined(__has_feature)
+#        if __has_feature(thread_sanitizer)
+    return YES;
+#            pragma clang diagnostic push
+#            pragma clang diagnostic ignored "-Wunreachable-code"
+#        endif // __has_feature(thread_sanitizer)
+#    endif // defined(__has_feature)
+
+    return NO;
+}
+
 NSString *
 profilerTruncationReasonName(SentryProfilerTruncationReason reason)
 {
@@ -497,17 +511,10 @@ - (void)startMetricProfiler
 
 - (void)start
 {
-// Disable profiling when running with TSAN because it produces a TSAN false
-// positive, similar to the situation described here:
-// https://github.com/envoyproxy/envoy/issues/2561
-#    if defined(__has_feature)
-#        if __has_feature(thread_sanitizer)
-    SENTRY_LOG_DEBUG(@"Disabling profiling when running with TSAN");
-    return;
-#            pragma clang diagnostic push
-#            pragma clang diagnostic ignored "-Wunreachable-code"
-#        endif // __has_feature(thread_sanitizer)
-#    endif // defined(__has_feature)
+    if (threadSanitizerIsPresent()) {
+        SENTRY_LOG_DEBUG(@"Disabling profiling when running with TSAN");
+        return;
+    }
 
     if (_profiler != nullptr) {
         // This theoretically shouldn't be possible as long as we're checking for nil and running

Sources/Sentry/SentrySDK.m

@@ -137,24 +137,29 @@ + (void)setStartInvocations:(NSUInteger)value
 
 + (void)startWithOptions:(SentryOptions *)options
 {
+    [SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel];
+
     // We accept the tradeoff that the SDK might not be fully initialized directly after
     // initializing it on a background thread because scheduling the init synchronously on the main
     // thread could lead to deadlocks.
-    [SentryThreadWrapper onMainThread:^{
-        startInvocations++;
+    SENTRY_LOG_DEBUG(@"Starting SDK...");
 
-        [SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel];
+    startInvocations++;
 
-        SentryClient *newClient = [[SentryClient alloc] initWithOptions:options];
-        [newClient.fileManager moveAppStateToPreviousAppState];
-        [newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs];
+    SentryClient *newClient = [[SentryClient alloc] initWithOptions:options];
+    [newClient.fileManager moveAppStateToPreviousAppState];
+    [newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs];
 
-        SentryScope *scope = options.initialScope(
-            [[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]);
-        // The Hub needs to be initialized with a client so that closing a session
-        // can happen.
-        [SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:scope]];
-        SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString);
+    SentryScope *scope
+        = options.initialScope([[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]);
+    // The Hub needs to be initialized with a client so that closing a session
+    // can happen.
+    [SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:scope]];
+    SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString);
+
+    SENTRY_LOG_DEBUG(@"Dispatching init work required to run on main thread.");
+    [SentryThreadWrapper onMainThread:^{
+        SENTRY_LOG_DEBUG(@"SDK main thread init started...");
         [SentrySDK installIntegrations];
 
         [SentryCrashWrapper.sharedInstance startBinaryImageCache];

Sources/Sentry/SentrySpan.m

@@ -25,6 +25,7 @@
 @implementation SentrySpan {
     NSMutableDictionary<NSString *, id> *_data;
     NSMutableDictionary<NSString *, id> *_tags;
+    NSObject *_stateLock;
     BOOL _isFinished;
 }
 
@@ -34,6 +35,7 @@ - (instancetype)initWithContext:(SentrySpanContext *)context
         self.startTimestamp = [SentryDependencyContainer.sharedInstance.dateProvider date];
         _data = [[NSMutableDictionary alloc] init];
         _tags = [[NSMutableDictionary alloc] init];
+        _stateLock = [[NSObject alloc] init];
         _isFinished = NO;
 
         _status = kSentrySpanStatusUndefined;
@@ -133,7 +135,9 @@ - (void)setMeasurement:(NSString *)name value:(NSNumber *)value unit:(SentryMeas
 
 - (BOOL)isFinished
 {
-    return _isFinished;
+    @synchronized(_stateLock) {
+        return _isFinished;
+    }
 }
 
 - (void)finish
@@ -145,7 +149,9 @@ - (void)finish
 - (void)finishWithStatus:(SentrySpanStatus)status
 {
     self.status = status;
-    _isFinished = YES;
+    @synchronized(_stateLock) {
+        _isFinished = YES;
+    }
     if (self.timestamp == nil) {
         self.timestamp = [SentryDependencyContainer.sharedInstance.dateProvider date];
         SENTRY_LOG_DEBUG(@"Setting span timestamp: %@ at system time %llu", self.timestamp,

Sources/Sentry/SentryThreadWrapper.m

@@ -1,4 +1,5 @@
 #import "SentryThreadWrapper.h"
+#import "SentryLog.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -22,8 +23,10 @@ - (void)threadFinished:(NSUUID *)threadID
 + (void)onMainThread:(void (^)(void))block
 {
     if ([NSThread isMainThread]) {
+        SENTRY_LOG_DEBUG(@"Already on main thread.");
         block();
     } else {
+        SENTRY_LOG_DEBUG(@"Dispatching asynchronously to main queue.");
         dispatch_async(dispatch_get_main_queue(), block);
     }
 }

Sources/Sentry/include/SentryProfiler.h

@@ -28,6 +28,12 @@ SENTRY_EXTERN NSString *const kSentryProfilerSerializationKeyFrameRates;
 
 SENTRY_EXTERN_C_BEGIN
 
+/**
+ * Disable profiling when running with TSAN because it produces a TSAN false positive, similar to
+ * the situation described here: https://github.com/envoyproxy/envoy/issues/2561
+ */
+BOOL threadSanitizerIsPresent(void);
+
 NSString *profilerTruncationReasonName(SentryProfilerTruncationReason reason);
 
 SENTRY_EXTERN_C_END

Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c

@@ -75,9 +75,12 @@ static sentrycrashbic_cacheChangeCallback imageRemovedCallback = NULL;
 static void
 binaryImageAdded(const struct mach_header *header, intptr_t slide)
 {
+    pthread_mutex_lock(&binaryImagesMutex);
     if (tailNode == NULL) {
+        pthread_mutex_unlock(&binaryImagesMutex);
         return;
     }
+    pthread_mutex_unlock(&binaryImagesMutex);
     Dl_info info;
     if (!dladdr(header, &info) || info.dli_fname == NULL) {
         return;

Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift

@@ -174,6 +174,11 @@ private extension SentryFramesTrackerTests {
 
 #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)
     func assertProfilingData(slow: UInt? = nil, frozen: UInt? = nil, frameRates: UInt? = nil) throws {
+        if threadSanitizerIsPresent() {
+            // profiling data will not have been gathered with TSAN running
+            return
+        }
+
         func assertFrameInfo(frame: [String: NSNumber]) throws {
             XCTAssertNotNil(frame["timestamp"], "Expected a timestamp for the frame.")
             XCTAssertNotNil(frame["value"], "Expected a duration value for the frame.")

Tests/SentryTests/PrivateSentrySDKOnlyTests.swift

@@ -125,6 +125,9 @@ class PrivateSentrySDKOnlyTests: XCTestCase {
       * Smoke Tests profiling via PrivateSentrySDKOnly. Actual profiling unit tests are done elsewhere.
      */
     func testProfilingStartAndCollect() throws {
+        if threadSanitizerIsPresent() {
+            throw XCTSkip("Profiler does not run if thread sanitizer is attached.")
+        }
         let options = Options()
         options.dsn = TestConstants.dsnAsString(username: "SentryFramesTrackingIntegrationTests")
         let client = TestClient(options: options)

Tests/SentryTests/SentryCrash/SentryCrashCPU_Tests.m

@@ -40,9 +40,26 @@ @implementation SentryCrashCPU_Tests
 
 - (void)testCPUState
 {
+    NSObject *notificationObject = [[NSObject alloc] init];
     TestThread *thread = [[TestThread alloc] init];
+    thread.notificationObject = notificationObject;
+
+    XCTestExpectation *exp = [self expectationWithDescription:@"thread started"];
+    [NSNotificationCenter.defaultCenter
+        addObserverForName:@"io.sentry.test.TestThread.main"
+                    object:notificationObject
+                     queue:nil
+                usingBlock:^(NSNotification *_Nonnull __unused notification) {
+                    [NSNotificationCenter.defaultCenter
+                        removeObserver:self
+                                  name:@"io.sentry.test.TestThread.main"
+                                object:notificationObject];
+                    [exp fulfill];
+                }];
+
     [thread start];
-    [NSThread sleepForTimeInterval:0.1];
+    [self waitForExpectationsWithTimeout:1 handler:nil];
+
     kern_return_t kr;
     kr = thread_suspend(thread.thread);
     XCTAssertTrue(kr == KERN_SUCCESS, @"");

Tests/SentryTests/SentryCrash/SentryCrashCachedData_Tests.m

@@ -38,10 +38,27 @@ - (void)testGetThreadName
     sentrycrashccd_close();
 
     NSString *expectedName = @"This is a test thread";
-    TestThread *thread = [TestThread new];
+    NSObject *notificationObject = [[NSObject alloc] init];
+    TestThread *thread = [[TestThread alloc] init];
+    thread.notificationObject = notificationObject;
     thread.name = expectedName;
+
+    XCTestExpectation *exp = [self expectationWithDescription:@"thread started"];
+    [NSNotificationCenter.defaultCenter
+        addObserverForName:@"io.sentry.test.TestThread.main"
+                    object:notificationObject
+                     queue:nil
+                usingBlock:^(NSNotification *_Nonnull __unused notification) {
+                    [NSNotificationCenter.defaultCenter
+                        removeObserver:self
+                                  name:@"io.sentry.test.TestThread.main"
+                                object:notificationObject];
+                    [exp fulfill];
+                }];
+
     [thread start];
-    [NSThread sleepForTimeInterval:0.1];
+    [self waitForExpectationsWithTimeout:1 handler:nil];
+
     sentrycrashccd_init(10);
     [NSThread sleepForTimeInterval:0.1];
     [thread cancel];

Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift

@@ -70,8 +70,10 @@ class SentryStacktraceBuilderTests: XCTestCase {
         XCTAssertTrue(filteredFrames.count == 1, "The frames must be ordered from caller to callee, or oldest to youngest.")
     }
 
-    func testConcurrentStacktraces() {
-        guard #available(macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0, *) else { return }
+    func testConcurrentStacktraces() throws {
+        guard #available(macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0, *) else {
+            throw XCTSkip("Not available for earlier platform versions")
+        }
 
         SentrySDK.start { options in
             options.dsn = TestConstants.dsnAsString(username: "SentryStacktraceBuilderTests")
@@ -87,8 +89,10 @@ class SentryStacktraceBuilderTests: XCTestCase {
         wait(for: [waitForAsyncToRun], timeout: 1)
     }
 
-    func testConcurrentStacktraces_noStitching() {
-        guard #available(macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0, *) else { return }
+    func testConcurrentStacktraces_noStitching() throws {
+        guard #available(macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0, *) else {
+            throw XCTSkip("Not available for earlier platform versions")
+        }
 
         SentrySDK.start { options in
             options.dsn = TestConstants.dsnAsString(username: "SentryStacktraceBuilderTests")