copy global data structures in a synchronized context

Sources/Sentry/SentryProfileTimeseries.mm

@@ -7,8 +7,6 @@
 #    import "SentryLog.h"
 #    import "SentryTransaction.h"
 
-std::mutex _gSamplesArrayLock;
-
 /**
  * Print a debug log to help diagnose slicing errors.
  * @param start @c YES if this is an attempt to find the start of the sliced data based on the
@@ -44,51 +42,45 @@
 NSArray<SentrySample *> *_Nullable slicedProfileSamples(
     NSArray<SentrySample *> *samples, SentryTransaction *transaction)
 {
-    NSArray<SentrySample *> *samplesCopy;
-    {
-        std::lock_guard<std::mutex> l(_gSamplesArrayLock);
-        samplesCopy = [samples copy];
-    }
-
-    if (samplesCopy.count == 0) {
+    if (samples.count == 0) {
         return nil;
     }
 
     const auto transactionStart = transaction.startSystemTime;
     const auto firstIndex =
-        [samplesCopy indexOfObjectWithOptions:NSEnumerationConcurrent
-                                  passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
-                                      BOOL *_Nonnull stop) {
-                                      *stop = sample.absoluteTimestamp >= transactionStart;
-                                      return *stop;
-                                  }];
+        [samples indexOfObjectWithOptions:NSEnumerationConcurrent
+                              passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
+                                  BOOL *_Nonnull stop) {
+                                  *stop = sample.absoluteTimestamp >= transactionStart;
+                                  return *stop;
+                              }];
 
     if (firstIndex == NSNotFound) {
-        logSlicingFailureWithArray(samplesCopy, transaction, /*start*/ YES);
+        logSlicingFailureWithArray(samples, transaction, /*start*/ YES);
         return nil;
     } else {
         SENTRY_LOG_DEBUG(@"Found first slice sample at index %lu", firstIndex);
     }
 
     const auto transactionEnd = transaction.endSystemTime;
     const auto lastIndex =
-        [samplesCopy indexOfObjectWithOptions:NSEnumerationConcurrent | NSEnumerationReverse
-                                  passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
-                                      BOOL *_Nonnull stop) {
-                                      *stop = sample.absoluteTimestamp <= transactionEnd;
-                                      return *stop;
-                                  }];
+        [samples indexOfObjectWithOptions:NSEnumerationConcurrent | NSEnumerationReverse
+                              passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
+                                  BOOL *_Nonnull stop) {
+                                  *stop = sample.absoluteTimestamp <= transactionEnd;
+                                  return *stop;
+                              }];
 
     if (lastIndex == NSNotFound) {
-        logSlicingFailureWithArray(samplesCopy, transaction, /*start*/ NO);
+        logSlicingFailureWithArray(samples, transaction, /*start*/ NO);
         return nil;
     } else {
         SENTRY_LOG_DEBUG(@"Found last slice sample at index %lu", lastIndex);
     }
 
     const auto range = NSMakeRange(firstIndex, (lastIndex - firstIndex) + 1);
     const auto indices = [NSIndexSet indexSetWithIndexesInRange:range];
-    return [samplesCopy objectsAtIndexes:indices];
+    return [samples objectsAtIndexes:indices];
 }
 
 @implementation SentrySample

Sources/Sentry/SentryProfiler.mm

@@ -79,11 +79,13 @@
     return [symbolNSStr substringWithRange:[match rangeAtIndex:1]];
 }
 
+std::mutex _gDataStructureLock;
+
 void
 processBacktrace(const Backtrace &backtrace,
     NSMutableDictionary<NSString *, NSMutableDictionary *> *threadMetadata,
     NSMutableDictionary<NSString *, NSDictionary *> *queueMetadata,
-    NSMutableArray<SentrySample *> *samples, NSMutableArray<NSMutableArray<NSNumber *> *> *stacks,
+    NSMutableArray<SentrySample *> *samples, NSMutableArray<NSArray<NSNumber *> *> *stacks,
     NSMutableArray<NSDictionary<NSString *, id> *> *frames,
     NSMutableDictionary<NSString *, NSNumber *> *frameIndexLookup,
     NSMutableDictionary<NSString *, NSNumber *> *stackIndexLookup)
@@ -155,7 +157,7 @@
     }
 
     {
-        std::lock_guard<std::mutex> l(_gSamplesArrayLock);
+        std::lock_guard<std::mutex> l(_gDataStructureLock);
         [samples addObject:sample];
     }
 }
@@ -362,29 +364,41 @@ + (SentryEnvelopeItem *)createProfilingEnvelopeItemForTransaction:(SentryTransac
 
     const auto payload = [NSMutableDictionary<NSString *, id> dictionary];
 
-    NSArray<SentrySample *> *samples = _gCurrentProfiler->_profileData[@"profile"][@"samples"];
+    NSArray<SentrySample *> *samplesCopy;
+    NSArray<NSArray<NSNumber *> *> *stacksCopy;
+    NSArray<NSDictionary<NSString *, id> *> *framesCopy;
+    NSDictionary<NSString *, NSDictionary *> *threadMetadataCopy;
+    NSDictionary<NSString *, NSDictionary *> *queueMetadataCopy;
+    {
+        std::lock_guard<std::mutex> d(_gDataStructureLock);
+        samplesCopy = [_gCurrentProfiler->_profileData[@"profile"][@"samples"] copy];
+        stacksCopy = [_gCurrentProfiler->_profileData[@"profile"][@"stacks"] copy];
+        framesCopy = [_gCurrentProfiler->_profileData[@"profile"][@"frames"] copy];
+        threadMetadataCopy = [_gCurrentProfiler->_profileData[@"profile"][@"thread_metadata"] copy];
+        queueMetadataCopy = [_gCurrentProfiler->_profileData[@"profile"][@"queue_metadata"] copy];
+    }
 
     // We need at least two samples to be able to draw a stack frame for any given function: one
     // sample for the start of the frame and another for the end. Otherwise we would only have a
     // stack frame with 0 duration, which wouldn't make sense.
-    if ([samples count] < 2) {
+    if ([samplesCopy count] < 2) {
         SENTRY_LOG_DEBUG(@"Not enough samples in profile");
         return nil;
     }
 
     // slice the profile data to only include the samples/metrics within the transaction
-    const auto slicedSamples = slicedProfileSamples(samples, transaction);
+    const auto slicedSamples = slicedProfileSamples(samplesCopy, transaction);
     if (slicedSamples.count < 2) {
         SENTRY_LOG_DEBUG(@"Not enough samples in profile during the transaction");
         return nil;
     }
 
     payload[@"profile"] = @{
         @"samples" : serializedSamplesWithRelativeTimestamps(slicedSamples, transaction),
-        @"stacks" : _gCurrentProfiler->_profileData[@"profile"][@"stacks"],
-        @"frames" : _gCurrentProfiler->_profileData[@"profile"][@"frames"],
-        @"thread_metadata" : _gCurrentProfiler->_profileData[@"profile"][@"thread_metadata"],
-        @"queue_metadata" : _gCurrentProfiler->_profileData[@"profile"][@"queue_metadata"],
+        @"stacks" : stacksCopy,
+        @"frames" : framesCopy,
+        @"thread_metadata" : threadMetadataCopy,
+        @"queue_metadata" : queueMetadataCopy,
     };
 
     // add the serialized info for the associated transaction
@@ -614,7 +628,7 @@ - (void)start
      * ]
      */
     const auto samples = [NSMutableArray<SentrySample *> array];
-    const auto stacks = [NSMutableArray<NSMutableArray<NSNumber *> *> array];
+    const auto stacks = [NSMutableArray<NSArray<NSNumber *> *> array];
     const auto frames = [NSMutableArray<NSDictionary<NSString *, id> *> array];
     const auto frameIndexLookup = [NSMutableDictionary<NSString *, NSNumber *> dictionary];
     const auto stackIndexLookup = [NSMutableDictionary<NSString *, NSNumber *> dictionary];

Sources/Sentry/include/SentryProfileTimeseries.h

@@ -8,13 +8,6 @@
 
 @class SentryTransaction;
 
-/**
- * Synchronizes reads and writes to the samples array; otherwise there will be a data race between
- * when the sampling profiler tries to insert a new sample, and when we iterate over the sample
- * array with fast enumeration to extract only those samples needed for a given transaction.
- */
-SENTRY_EXTERN std::mutex _gSamplesArrayLock;
-
 NS_ASSUME_NONNULL_BEGIN
 
 /** A storage class to hold the data associated with a single profiler sample. */

Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift

@@ -209,7 +209,7 @@ class SentryProfilerSwiftTests: XCTestCase {
 
     override class func setUp() {
         super.setUp()
-        SentryLog.configure(true, diagnosticLevel: .debug)
+//        SentryLog.configure(true, diagnosticLevel: .debug)
     }
 
     override func setUp() {
@@ -236,7 +236,7 @@ class SentryProfilerSwiftTests: XCTestCase {
     }
 
     func testConcurrentProfilingTransactions() throws {
-        let numberOfTransactions = 10
+        let numberOfTransactions = 100
         var spans = [Span]()
 
         func createConcurrentSpansWithMetrics() throws {