Fix flowIDMap being accessed from multiple threads simultaneously

Summary:`flowIDMap` lives on the bridge to map from the IDs used for the flow events in JS and the ones generated by `RCTProfile` in the native side. It was being accessed from multiple threads (the various modules' queues in the bridge and the JS thread), so we lock before touching it. Reviewed By: javache Differential Revision: D3102745 fb-gh-sync-id: 93d012d124e8b5d1a390c10a98ef5e3a068ccf63 fbshipit-source-id: 93d012d124e8b5d1a390c10a98ef5e3a068ccf63
facebook · Mar 30, 2016 · 2be42ab · 2be42ab
1 parent 3614e78
commit 2be42ab
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 7 deletions.
diff --git a/React/Base/RCTBatchedBridge.m b/React/Base/RCTBatchedBridge.m
@@ -61,6 +61,7 @@ @implementation RCTBatchedBridge
 
 @synthesize flowID = _flowID;
 @synthesize flowIDMap = _flowIDMap;
+@synthesize flowIDMapLock = _flowIDMapLock;
 @synthesize loading = _loading;
 @synthesize valid = _valid;
 
@@ -885,10 +886,12 @@ - (void)handleBuffer:(NSArray *)buffer
       @autoreleasepool {
         for (NSNumber *indexObj in calls) {
           NSUInteger index = indexObj.unsignedIntegerValue;
-          if (callID != -1 && _flowIDMap != NULL) {
+          if (RCT_DEV && callID != -1 && _flowIDMap != NULL && RCTProfileIsProfiling()) {
+            [self.flowIDMapLock lock];
             int64_t newFlowID = (int64_t)CFDictionaryGetValue(_flowIDMap, (const void *)(_flowID + index));
             _RCTProfileEndFlowEvent(@(newFlowID));
             CFDictionaryRemoveValue(_flowIDMap, (const void *)(_flowID + index));
+            [self.flowIDMapLock unlock];
           }
           [self _handleRequestNumber:index
                             moduleID:[moduleIDs[index] integerValue]

diff --git a/React/Base/RCTBridge+Private.h b/React/Base/RCTBridge+Private.h
@@ -16,6 +16,7 @@
 // Used for the profiler flow events between JS and native
 @property (nonatomic, assign) int64_t flowID;
 @property (nonatomic, assign) CFMutableDictionaryRef flowIDMap;
+@property (nonatomic, strong) NSLock *flowIDMapLock;
 
 + (instancetype)currentBridge;
 + (void)setCurrentBridge:(RCTBridge *)bridge;

diff --git a/React/Executors/RCTJSCExecutor.m b/React/Executors/RCTJSCExecutor.m
@@ -302,17 +302,26 @@ - (void)setUp
 
   __weak RCTBridge *weakBridge = _bridge;
 #ifndef __clang_analyzer__
-  weakBridge.flowIDMap = CFDictionaryCreateMutable(NULL, 0, NULL, NULL);
+  _bridge.flowIDMap = CFDictionaryCreateMutable(NULL, 0, NULL, NULL);
 #endif
+  _bridge.flowIDMapLock = [NSLock new];
   [self addSynchronousHookWithName:@"nativeTraceBeginAsyncFlow" usingBlock:^(__unused uint64_t tag, __unused NSString *name, int64_t cookie) {
-    int64_t newCookie = [_RCTProfileBeginFlowEvent() longLongValue];
-    CFDictionarySetValue(weakBridge.flowIDMap, (const void *)cookie, (const void *)newCookie);
+    if (RCTProfileIsProfiling()) {
+      [weakBridge.flowIDMapLock lock];
+      int64_t newCookie = [_RCTProfileBeginFlowEvent() longLongValue];
+      CFDictionarySetValue(weakBridge.flowIDMap, (const void *)cookie, (const void *)newCookie);
+      [weakBridge.flowIDMapLock unlock];
+    }
   }];
 
   [self addSynchronousHookWithName:@"nativeTraceEndAsyncFlow" usingBlock:^(__unused uint64_t tag, __unused NSString *name, int64_t cookie) {
-    int64_t newCookie = (int64_t)CFDictionaryGetValue(weakBridge.flowIDMap, (const void *)cookie);
-    _RCTProfileEndFlowEvent(@(newCookie));
-    CFDictionaryRemoveValue(weakBridge.flowIDMap, (const void *)cookie);
+    if (RCTProfileIsProfiling()) {
+      [weakBridge.flowIDMapLock lock];
+      int64_t newCookie = (int64_t)CFDictionaryGetValue(weakBridge.flowIDMap, (const void *)cookie);
+      _RCTProfileEndFlowEvent(@(newCookie));
+      CFDictionaryRemoveValue(weakBridge.flowIDMap, (const void *)cookie);
+      [weakBridge.flowIDMapLock unlock];
+    }
   }];
 
   [self executeBlockOnJavaScriptQueue:^{