diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index 5e6a2aa03..bf7dd8503 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -7,6 +7,7 @@ #import "NSError+messageSending.h" #import "NSURLSessionDataTask+StatusCode.h" #import "OWSError.h" +#import "OWSQueues.h" #import "OWSSignalService.h" #import "SSKEnvironment.h" #import "TSAccountManager.h" @@ -25,11 +26,21 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) && error.code == TSNetworkManagerErrorFailedConnection); } +dispatch_queue_t NetworkManagerQueue() +{ + static dispatch_queue_t serialQueue; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + serialQueue = dispatch_queue_create("org.whispersystems.networkManager", DISPATCH_QUEUE_SERIAL); + }); + return serialQueue; +} + #pragma mark - @interface OWSSessionManager : NSObject -@property (nonatomic) AFHTTPSessionManager *sessionManager; +@property (nonatomic, readonly) AFHTTPSessionManager *sessionManager; @property (nonatomic, readonly) NSDictionary *defaultHeaders; @end @@ -38,14 +49,25 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) @implementation OWSSessionManager +#pragma mark - Dependencies + +- (OWSSignalService *)signalService +{ + return [OWSSignalService sharedInstance]; +} + +#pragma mark - + - (instancetype)init { + AssertOnDispatchQueue(NetworkManagerQueue()); + self = [super init]; if (!self) { return self; } - self.sessionManager = [OWSSignalService sharedInstance].signalServiceSessionManager; + _sessionManager = [self.signalService buildSignalServiceSessionManager]; self.sessionManager.completionQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); // NOTE: We could enable HTTPShouldUsePipelining here. // Make a copy of the default headers for this session manager. @@ -54,11 +76,13 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) return self; } +// TSNetworkManager.serialQueue - (void)performRequest:(TSRequest *)request canUseAuth:(BOOL)canUseAuth success:(TSNetworkManagerSuccess)success failure:(TSNetworkManagerFailure)failure { + AssertOnDispatchQueue(NetworkManagerQueue()); OWSAssertDebug(request); OWSAssertDebug(success); OWSAssertDebug(failure); @@ -116,6 +140,20 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) #pragma mark - +// You might be asking: "why use a pool at all? We're only using the session manager +// on the serial queue, so can't we just have two session managers (1 UD, 1 non-UD) +// that we use for all requests?" +// +// That assumes that the session managers are not stateful in a way where concurrent +// requests can interfere with each other. I audited the AFNetworking codebase and my +// reading is that sessions managers are safe to use in that way - that the state of +// their properties (e.g. header values) is only used when building the request and +// can be safely changed after performRequest is complete. +// +// But I decided that I didn't want to (silently) bake that assumption into the +// codebase, since the stakes are high. The session managers aren't expensive. IMO +// better to use a pool and not re-use a session manager until its request succeeds +// or fails. @interface OWSSessionManagerPool : NSObject @property (nonatomic) NSMutableArray *pool; @@ -140,6 +178,8 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) - (OWSSessionManager *)get { + AssertOnDispatchQueue(NetworkManagerQueue()); + OWSSessionManager *_Nullable sessionManager = [self.pool lastObject]; if (sessionManager) { OWSLogVerbose(@"Cache hit."); @@ -154,6 +194,8 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) - (void)returnToPool:(OWSSessionManager *)sessionManager { + AssertOnDispatchQueue(NetworkManagerQueue()); + OWSAssertDebug(sessionManager); const NSUInteger kMaxPoolSize = 3; if (self.pool.count >= kMaxPoolSize) { @@ -173,8 +215,6 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) @property (atomic, readonly) OWSSessionManagerPool *udSessionManagerPool; @property (atomic, readonly) OWSSessionManagerPool *nonUdSessionManagerPool; -@property (atomic, readonly) dispatch_queue_t serialQueue; - @end #pragma mark - @@ -188,10 +228,6 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) return TSAccountManager.sharedInstance; } -#pragma mark - - -@synthesize serialQueue = _serialQueue; - #pragma mark - Singleton + (instancetype)sharedManager @@ -210,7 +246,6 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) _udSessionManagerPool = [OWSSessionManagerPool new]; _nonUdSessionManagerPool = [OWSSessionManagerPool new]; - _serialQueue = dispatch_queue_create("org.whispersystems.networkManager", DISPATCH_QUEUE_SERIAL); OWSSingletonAssert(); @@ -235,7 +270,7 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) OWSAssertDebug(success); OWSAssertDebug(failure); - dispatch_async(self.serialQueue, ^{ + dispatch_async(NetworkManagerQueue(), ^{ [self makeRequestSync:request completionQueue:completionQueue success:success failure:failure]; }); } @@ -262,7 +297,7 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) OWSSessionManager *sessionManager = [sessionManagerPool get]; TSNetworkManagerSuccess success = ^(NSURLSessionDataTask *task, _Nullable id responseObject) { - dispatch_async(self.serialQueue, ^{ + dispatch_async(NetworkManagerQueue(), ^{ [sessionManagerPool returnToPool:sessionManager]; }); @@ -279,7 +314,7 @@ BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) }); }; TSNetworkManagerSuccess failure = ^(NSURLSessionDataTask *task, NSError *error) { - dispatch_async(self.serialQueue, ^{ + dispatch_async(NetworkManagerQueue(), ^{ [sessionManagerPool returnToPool:sessionManager]; }); diff --git a/SignalServiceKit/src/Network/OWSSignalService.h b/SignalServiceKit/src/Network/OWSSignalService.h index 16651b787..d209a40bd 100644 --- a/SignalServiceKit/src/Network/OWSSignalService.h +++ b/SignalServiceKit/src/Network/OWSSignalService.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // NS_ASSUME_NONNULL_BEGIN @@ -12,9 +12,6 @@ extern NSString *const kNSNotificationName_IsCensorshipCircumventionActiveDidCha @interface OWSSignalService : NSObject -/// For interacting with the Signal Service -@property (nonatomic, readonly) AFHTTPSessionManager *signalServiceSessionManager; - /// For uploading avatar assets. @property (nonatomic, readonly) AFHTTPSessionManager *CDNSessionManager; @@ -29,6 +26,9 @@ extern NSString *const kNSNotificationName_IsCensorshipCircumventionActiveDidCha @property (atomic) BOOL isCensorshipCircumventionManuallyActivated; @property (atomic, nullable) NSString *manualCensorshipCircumventionCountryCode; +/// For interacting with the Signal Service +- (AFHTTPSessionManager *)buildSignalServiceSessionManager; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Network/OWSSignalService.m b/SignalServiceKit/src/Network/OWSSignalService.m index fd8a2fc3d..75571d2d3 100644 --- a/SignalServiceKit/src/Network/OWSSignalService.m +++ b/SignalServiceKit/src/Network/OWSSignalService.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // #import "OWSSignalService.h" @@ -147,7 +147,7 @@ NSString *const kNSNotificationName_IsCensorshipCircumventionActiveDidChange = } } -- (AFHTTPSessionManager *)signalServiceSessionManager +- (AFHTTPSessionManager *)buildSignalServiceSessionManager { if (self.isCensorshipCircumventionActive) { OWSLogInfo(@"using reflector HTTPSessionManager via: %@", self.censorshipConfiguration.domainFrontBaseURL);