Skip to content

Commit

Permalink
Fix some unsafe thread behavior
Browse files Browse the repository at this point in the history 
Reviewed By: javache

Differential Revision: D3789293

fbshipit-source-id: 80118c7f8faf487fe35d4d83a91f023219f6bf80
  • Loading branch information
@mhorowitz
mhorowitz authored and Facebook Github Bot 6 committed Sep 2, 2016
1 parent 6abacc8 commit a8cf12a
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 56 deletions.
12 changes: 6 additions & 6 deletions Libraries/Network/RCTNetwork.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
objects = {

/* Begin PBXBuildFile section */
134E969A1BCEB7F800AFFDA1 /* RCTDataRequestHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 134E96991BCEB7F800AFFDA1 /* RCTDataRequestHandler.m */; settings = {ASSET_TAGS = (); }; };
134E969A1BCEB7F800AFFDA1 /* RCTDataRequestHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 134E96991BCEB7F800AFFDA1 /* RCTDataRequestHandler.m */; };
1372B7371AB03E7B00659ED6 /* RCTNetInfo.m in Sources */ = {isa = PBXBuildFile; fileRef = 1372B7361AB03E7B00659ED6 /* RCTNetInfo.m */; };
13D6D66A1B5FCF8200883BE9 /* RCTNetworkTask.m in Sources */ = {isa = PBXBuildFile; fileRef = 13D6D6691B5FCF8200883BE9 /* RCTNetworkTask.m */; };
13EF800E1BCBE015003F47DD /* RCTFileRequestHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF800D1BCBE015003F47DD /* RCTFileRequestHandler.m */; settings = {ASSET_TAGS = (); }; };
13EF800E1BCBE015003F47DD /* RCTFileRequestHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF800D1BCBE015003F47DD /* RCTFileRequestHandler.m */; };
352DA0BA1B17855800AA15A8 /* RCTHTTPRequestHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 352DA0B81B17855800AA15A8 /* RCTHTTPRequestHandler.m */; };
58B512081A9E6CE300147676 /* RCTNetworking.m in Sources */ = {isa = PBXBuildFile; fileRef = 58B512071A9E6CE300147676 /* RCTNetworking.m */; };
58B512081A9E6CE300147676 /* RCTNetworking.mm in Sources */ = {isa = PBXBuildFile; fileRef = 58B512071A9E6CE300147676 /* RCTNetworking.mm */; };
/* End PBXBuildFile section */

/* Begin PBXCopyFilesBuildPhase section */
Expand Down Expand Up @@ -40,7 +40,7 @@
352DA0B81B17855800AA15A8 /* RCTHTTPRequestHandler.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTHTTPRequestHandler.m; sourceTree = "<group>"; };
58B511DB1A9E6C8500147676 /* libRCTNetwork.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libRCTNetwork.a; sourceTree = BUILT_PRODUCTS_DIR; };
58B512061A9E6CE300147676 /* RCTNetworking.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTNetworking.h; sourceTree = "<group>"; };
58B512071A9E6CE300147676 /* RCTNetworking.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTNetworking.m; sourceTree = "<group>"; };
58B512071A9E6CE300147676 /* RCTNetworking.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTNetworking.mm; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -68,7 +68,7 @@
1372B7351AB03E7B00659ED6 /* RCTNetInfo.h */,
1372B7361AB03E7B00659ED6 /* RCTNetInfo.m */,
58B512061A9E6CE300147676 /* RCTNetworking.h */,
58B512071A9E6CE300147676 /* RCTNetworking.m */,
58B512071A9E6CE300147676 /* RCTNetworking.mm */,
58B511DC1A9E6C8500147676 /* Products */,
);
indentWidth = 2;
Expand Down Expand Up @@ -143,7 +143,7 @@
13EF800E1BCBE015003F47DD /* RCTFileRequestHandler.m in Sources */,
134E969A1BCEB7F800AFFDA1 /* RCTDataRequestHandler.m in Sources */,
1372B7371AB03E7B00659ED6 /* RCTNetInfo.m in Sources */,
58B512081A9E6CE300147676 /* RCTNetworking.m in Sources */,
58B512081A9E6CE300147676 /* RCTNetworking.mm in Sources */,
352DA0BA1B17855800AA15A8 /* RCTHTTPRequestHandler.m in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
35 changes: 21 additions & 14 deletions Libraries/Network/RCTNetworking.m → Libraries/Network/RCTNetworking.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#import "RCTNetworking.h"

#include <mutex>

#import "RCTAssert.h"
#import "RCTConvert.h"
#import "RCTNetworkTask.h"
Expand Down Expand Up @@ -48,7 +50,7 @@ @implementation RCTHTTPFormDataHelper
const size_t boundaryLength = 70;
const char *boundaryChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_./";

char *bytes = malloc(boundaryLength);
char *bytes = (char*)malloc(boundaryLength);
size_t charCount = strlen(boundaryChars);
for (int i = 0; i < boundaryLength; i++) {
bytes[i] = boundaryChars[arc4random_uniform((u_int32_t)charCount)];
Expand Down Expand Up @@ -126,6 +128,7 @@ - (RCTURLRequestCancellationBlock)handleResult:(NSDictionary<NSString *, id> *)r
@implementation RCTNetworking
{
NSMutableDictionary<NSNumber *, RCTNetworkTask *> *_tasksByRequestID;
std::mutex _handlersLock;
NSArray<id<RCTURLRequestHandler>> *_handlers;
}

Expand All @@ -149,19 +152,23 @@ @implementation RCTNetworking
return nil;
}

if (!_handlers) {
// Get handlers, sorted in reverse priority order (highest priority first)
_handlers = [[self.bridge modulesConformingToProtocol:@protocol(RCTURLRequestHandler)] sortedArrayUsingComparator:^NSComparisonResult(id<RCTURLRequestHandler> a, id<RCTURLRequestHandler> b) {
float priorityA = [a respondsToSelector:@selector(handlerPriority)] ? [a handlerPriority] : 0;
float priorityB = [b respondsToSelector:@selector(handlerPriority)] ? [b handlerPriority] : 0;
if (priorityA > priorityB) {
return NSOrderedAscending;
} else if (priorityA < priorityB) {
return NSOrderedDescending;
} else {
return NSOrderedSame;
}
}];
{
std::lock_guard<std::mutex> lock(_handlersLock);

if (!_handlers) {
// Get handlers, sorted in reverse priority order (highest priority first)
_handlers = [[self.bridge modulesConformingToProtocol:@protocol(RCTURLRequestHandler)] sortedArrayUsingComparator:^NSComparisonResult(id<RCTURLRequestHandler> a, id<RCTURLRequestHandler> b) {
float priorityA = [a respondsToSelector:@selector(handlerPriority)] ? [a handlerPriority] : 0;
float priorityB = [b respondsToSelector:@selector(handlerPriority)] ? [b handlerPriority] : 0;
if (priorityA > priorityB) {
return NSOrderedAscending;
} else if (priorityA < priorityB) {
return NSOrderedDescending;
} else {
return NSOrderedSame;
}
}];
}
}

if (RCT_DEBUG) {
Expand Down
63 changes: 31 additions & 32 deletions React/Base/RCTModuleData.m → React/Base/RCTModuleData.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

#import <objc/runtime.h>

#include <mutex>

#import "RCTBridge.h"
#import "RCTBridge+Private.h"
#import "RCTModuleMethod.h"
Expand All @@ -23,7 +25,7 @@ @implementation RCTModuleData
NSDictionary<NSString *, id> *_constantsToExport;
NSString *_queueName;
__weak RCTBridge *_bridge;
NSLock *_instanceLock;
std::mutex _instanceLock;
BOOL _setupComplete;
}

Expand All @@ -36,8 +38,6 @@ - (void)setUp
_implementsBatchDidComplete = [_moduleClass instancesRespondToSelector:@selector(batchDidComplete)];
_implementsPartialBatchDidFlush = [_moduleClass instancesRespondToSelector:@selector(partialBatchDidFlush)];

_instanceLock = [NSLock new];

static IMP objectInitMethod;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
Expand Down Expand Up @@ -84,43 +84,42 @@ - (instancetype)initWithModuleInstance:(id<RCTBridgeModule>)instance
- (void)setUpInstanceAndBridge
{
RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"[RCTModuleData setUpInstanceAndBridge] [_instanceLock lock]", @{ @"moduleClass": _moduleClass });
[_instanceLock lock];
if (!_setupComplete && _bridge.valid) {
if (!_instance) {
if (RCT_DEBUG && _requiresMainQueueSetup) {
RCTAssertMainQueue();
}
RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"[RCTModuleData setUpInstanceAndBridge] [_moduleClass new]", @{ @"moduleClass": _moduleClass });
_instance = [_moduleClass new];
RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @"", nil);
{
std::unique_lock<std::mutex> lock(_instanceLock);

if (!_setupComplete && _bridge.valid) {
if (!_instance) {
// Module init returned nil, probably because automatic instantatiation
// of the module is not supported, and it is supposed to be passed in to
// the bridge constructor. Mark setup complete to avoid doing more work.
_setupComplete = YES;
RCTLogWarn(@"The module %@ is returning nil from its constructor. You "
"may need to instantiate it yourself and pass it into the "
"bridge.", _moduleClass);
if (RCT_DEBUG && _requiresMainQueueSetup) {
RCTAssertMainQueue();
}
RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"[RCTModuleData setUpInstanceAndBridge] [_moduleClass new]", @{ @"moduleClass": _moduleClass });
_instance = [_moduleClass new];
RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @"", nil);
if (!_instance) {
// Module init returned nil, probably because automatic instantatiation
// of the module is not supported, and it is supposed to be passed in to
// the bridge constructor. Mark setup complete to avoid doing more work.
_setupComplete = YES;
RCTLogWarn(@"The module %@ is returning nil from its constructor. You "
"may need to instantiate it yourself and pass it into the "
"bridge.", _moduleClass);
}
}

if (_instance && RCTProfileIsProfiling()) {
RCTProfileHookInstance(_instance);
}
}

if (_instance && RCTProfileIsProfiling()) {
RCTProfileHookInstance(_instance);
// Bridge must be set before methodQueue is set up, as methodQueue
// initialization requires it (View Managers get their queue by calling
// self.bridge.uiManager.methodQueue)
[self setBridgeForInstance];
}

// Bridge must be set before methodQueue is set up, as methodQueue
// initialization requires it (View Managers get their queue by calling
// self.bridge.uiManager.methodQueue)
[self setBridgeForInstance];
[self setUpMethodQueue];
}
[_instanceLock unlock];
RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @"", nil);

// This is called outside of the lock in order to prevent deadlock issues
// because the logic in `setUpMethodQueue` can cause `moduleData.instance`
// to be accessed re-entrantly.
[self setUpMethodQueue];

// This is called outside of the lock in order to prevent deadlock issues
// because the logic in `finishSetupForInstance` can cause
// `moduleData.instance` to be accessed re-entrantly.
Expand Down
8 changes: 4 additions & 4 deletions React/React.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
1450FF8A1BCFF28A00208362 /* RCTProfileTrampoline-x86_64.S in Sources */ = {isa = PBXBuildFile; fileRef = 1450FF851BCFF28A00208362 /* RCTProfileTrampoline-x86_64.S */; };
14A43DF31C20B1C900794BC8 /* RCTJSCProfiler.m in Sources */ = {isa = PBXBuildFile; fileRef = 14A43DF21C20B1C900794BC8 /* RCTJSCProfiler.m */; };
14C2CA711B3AC63800E6CBB2 /* RCTModuleMethod.m in Sources */ = {isa = PBXBuildFile; fileRef = 14C2CA701B3AC63800E6CBB2 /* RCTModuleMethod.m */; };
14C2CA741B3AC64300E6CBB2 /* RCTModuleData.m in Sources */ = {isa = PBXBuildFile; fileRef = 14C2CA731B3AC64300E6CBB2 /* RCTModuleData.m */; };
14C2CA741B3AC64300E6CBB2 /* RCTModuleData.mm in Sources */ = {isa = PBXBuildFile; fileRef = 14C2CA731B3AC64300E6CBB2 /* RCTModuleData.mm */; };
14C2CA761B3AC64F00E6CBB2 /* RCTFrameUpdate.m in Sources */ = {isa = PBXBuildFile; fileRef = 14C2CA751B3AC64F00E6CBB2 /* RCTFrameUpdate.m */; };
14C2CA781B3ACB0400E6CBB2 /* RCTBatchedBridge.m in Sources */ = {isa = PBXBuildFile; fileRef = 14C2CA771B3ACB0400E6CBB2 /* RCTBatchedBridge.m */; };
14F3620D1AABD06A001CE568 /* RCTSwitch.m in Sources */ = {isa = PBXBuildFile; fileRef = 14F362081AABD06A001CE568 /* RCTSwitch.m */; };
Expand Down Expand Up @@ -256,7 +256,7 @@
14C2CA6F1B3AC63800E6CBB2 /* RCTModuleMethod.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTModuleMethod.h; sourceTree = "<group>"; };
14C2CA701B3AC63800E6CBB2 /* RCTModuleMethod.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTModuleMethod.m; sourceTree = "<group>"; };
14C2CA721B3AC64300E6CBB2 /* RCTModuleData.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTModuleData.h; sourceTree = "<group>"; };
14C2CA731B3AC64300E6CBB2 /* RCTModuleData.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTModuleData.m; sourceTree = "<group>"; };
14C2CA731B3AC64300E6CBB2 /* RCTModuleData.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTModuleData.mm; sourceTree = "<group>"; };
14C2CA751B3AC64F00E6CBB2 /* RCTFrameUpdate.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTFrameUpdate.m; sourceTree = "<group>"; };
14C2CA771B3ACB0400E6CBB2 /* RCTBatchedBridge.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTBatchedBridge.m; sourceTree = "<group>"; };
14F362071AABD06A001CE568 /* RCTSwitch.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTSwitch.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -606,7 +606,7 @@
83CBBA4D1A601E3B00E9B192 /* RCTLog.h */,
83CBBA4E1A601E3B00E9B192 /* RCTLog.m */,
14C2CA721B3AC64300E6CBB2 /* RCTModuleData.h */,
14C2CA731B3AC64300E6CBB2 /* RCTModuleData.m */,
14C2CA731B3AC64300E6CBB2 /* RCTModuleData.mm */,
14C2CA6F1B3AC63800E6CBB2 /* RCTModuleMethod.h */,
14C2CA701B3AC63800E6CBB2 /* RCTModuleMethod.m */,
13A6E20F1C19ABC700845B82 /* RCTNullability.h */,
Expand Down Expand Up @@ -773,7 +773,7 @@
14F484561AABFCE100FDF6B9 /* RCTSliderManager.m in Sources */,
13D033631C1837FE0021DC29 /* RCTClipboard.m in Sources */,
85C199EE1CD2407900DAD810 /* RCTJSCWrapper.mm in Sources */,
14C2CA741B3AC64300E6CBB2 /* RCTModuleData.m in Sources */,
14C2CA741B3AC64300E6CBB2 /* RCTModuleData.mm in Sources */,
142014191B32094000CC17BA /* RCTPerformanceLogger.m in Sources */,
83CBBA981A6020BB00E9B192 /* RCTTouchHandler.m in Sources */,
3EDCA8A51D3591E700450C31 /* RCTErrorInfo.m in Sources */,
Expand Down

0 comments on commit a8cf12a

Please sign in to comment.