Merge branch 'charlesmchen/messageSenderDeadlocks'

SignalServiceKit/src/Messages/OWSMessageSender.m

@@ -1303,8 +1303,14 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
         __block dispatch_semaphore_t sema = dispatch_semaphore_create(0);
         __block PreKeyBundle *_Nullable bundle;
         __block NSException *_Nullable exception;
-        [self.networkManager makeRequest:[[TSRecipientPrekeyRequest alloc] initWithRecipient:identifier
+        // It's not ideal that we're using a semaphore inside a read/write transaction.
-                                                                                    deviceId:[deviceNumber stringValue]]
+        // To avoid deadlock, we need to ensure that our success/failure completions
+        // are called _off_ the main thread.  Otherwise we'll deadlock if the main
+        // thread is blocked on opening a transaction.
+        TSRequest *request =
+            [[TSRecipientPrekeyRequest alloc] initWithRecipient:identifier deviceId:[deviceNumber stringValue]];
+        [self.networkManager makeRequest:request
+            completionQueue:dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0)
             success:^(NSURLSessionDataTask *task, id responseObject) {
                 bundle = [PreKeyBundle preKeyBundleFromDictionary:responseObject forDeviceNumber:deviceNumber];
                 dispatch_semaphore_signal(sema);

SignalServiceKit/src/Network/API/TSNetworkManager.h

@@ -1,5 +1,5 @@
 //
-//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
 //
 
 /**
@@ -27,6 +27,9 @@ extern NSString *const TSNetworkManagerDomain;
 
 BOOL IsNSErrorNetworkFailure(NSError *_Nullable error);
 
+typedef void (^TSNetworkManagerSuccess)(NSURLSessionDataTask *task, id responseObject);
+typedef void (^TSNetworkManagerFailure)(NSURLSessionDataTask *task, NSError *error);
+
 @interface TSNetworkManager : NSObject
 
 - (instancetype)init NS_UNAVAILABLE;
@@ -34,8 +37,16 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error);
 + (instancetype)sharedManager;
 
 - (void)makeRequest:(TSRequest *)request
-            success:(void (^)(NSURLSessionDataTask *task, id responseObject))success
+            success:(TSNetworkManagerSuccess)success
-            failure:(void (^)(NSURLSessionDataTask *task, NSError *error))failure NS_SWIFT_NAME(makeRequest(_:success:failure:));
+            failure:(TSNetworkManagerFailure)failure NS_SWIFT_NAME(makeRequest(_:success:failure:));
+
+- (void)makeRequest:(TSRequest *)request
+    completionQueue:(dispatch_queue_t)completionQueue
+            success:(TSNetworkManagerSuccess)success
+            failure:(TSNetworkManagerFailure)failure NS_SWIFT_NAME(makeRequest(_:shouldCompleteOnMainQueue
+:success
+:failure
+:));
 
 @end
 

SignalServiceKit/src/Network/API/TSNetworkManager.m

@@ -55,9 +55,21 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error);
 #pragma mark Manager Methods
 
 - (void)makeRequest:(TSRequest *)request
-            success:(void (^)(NSURLSessionDataTask *task, id responseObject))success
+            success:(TSNetworkManagerSuccess)success
-            failure:(void (^)(NSURLSessionDataTask *task, NSError *error))failureBlock
+            failure:(TSNetworkManagerFailure)failure
 {
+    return [self makeRequest:request completionQueue:dispatch_get_main_queue() success:success failure:failure];
+}
+
+- (void)makeRequest:(TSRequest *)request
+    completionQueue:(dispatch_queue_t)completionQueue
+            success:(TSNetworkManagerSuccess)success
+            failure:(TSNetworkManagerFailure)failureBlock
+{
+    OWSAssert(request);
+    OWSAssert(success);
+    OWSAssert(failureBlock);
+
     DDLogInfo(@"%@ Making request: %@", self.logTag, request);
     if (!CurrentAppContext().isMainApp) {
         if (![request isKindOfClass:[TSRecipientPrekeyRequest class]]
@@ -73,6 +85,9 @@ typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error);
         [TSNetworkManager errorPrettifyingForFailureBlock:failureBlock];
 
     AFHTTPSessionManager *sessionManager = [OWSSignalService sharedInstance].signalServiceSessionManager;
+    // [OWSSignalService signalServiceSessionManager] always returns a new instance of
+    // session manager, so its safe to reconfigure it here.
+    sessionManager.completionQueue = completionQueue;
 
     if ([request isKindOfClass:[TSVerifyCodeRequest class]]) {
         // We plant the Authorization parameter ourselves, no need to double add.

SignalServiceKit/src/Network/OWSSignalService.m

@@ -205,7 +205,6 @@ NSString *const kNSNotificationName_IsCensorshipCircumventionActiveDidChange =
 
     sessionManager.requestSerializer = [AFJSONRequestSerializer serializer];
     [sessionManager.requestSerializer setValue:self.censorshipConfiguration.signalServiceReflectorHost forHTTPHeaderField:@"Host"];
-
     sessionManager.responseSerializer = [AFJSONResponseSerializer serializer];
 
     return sessionManager;