First iteration of thread-safety on AFURLConnectionOperation, using a combination of NSRecursiveLock and common sense

This commit is contained in:
Mattt Thompson 2011-12-08 12:23:58 -06:00
parent e37c641656
commit 680fcd5071
2 changed files with 103 additions and 75 deletions

View file

@ -75,6 +75,10 @@ extern NSString * const AFNetworkingOperationDidFinishNotification;
*/ */
@interface AFURLConnectionOperation : NSOperation { @interface AFURLConnectionOperation : NSOperation {
@private @private
unsigned short _state;
BOOL _cancelled;
NSRecursiveLock *_lock;
NSSet *_runLoopModes; NSSet *_runLoopModes;
NSURLConnection *_connection; NSURLConnection *_connection;

View file

@ -22,14 +22,18 @@
#import "AFURLConnectionOperation.h" #import "AFURLConnectionOperation.h"
static NSUInteger const kAFHTTPMinimumInitialDataCapacity = 1024;
static NSUInteger const kAFHTTPMaximumInitialDataCapacity = 1024 * 1024 * 8;
typedef enum { typedef enum {
AFHTTPOperationReadyState = 1, AFHTTPOperationReadyState = 1,
AFHTTPOperationExecutingState = 2, AFHTTPOperationExecutingState = 2,
AFHTTPOperationFinishedState = 3, AFHTTPOperationFinishedState = 3,
} AFOperationState; } _AFOperationState;
typedef unsigned short AFOperationState;
static NSUInteger const kAFHTTPMinimumInitialDataCapacity = 1024;
static NSUInteger const kAFHTTPMaximumInitialDataCapacity = 1024 * 1024 * 8;
static NSString * const kAFNetworkingLockName = @"com.alamofire.networking.operation.lock";
NSString * const AFNetworkingErrorDomain = @"com.alamofire.networking.error"; NSString * const AFNetworkingErrorDomain = @"com.alamofire.networking.error";
@ -51,9 +55,35 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
} }
} }
static inline BOOL AFStateTransitionIsValid(AFOperationState fromState, AFOperationState toState, BOOL isCancelled) {
switch (fromState) {
case AFHTTPOperationReadyState:
switch (toState) {
case AFHTTPOperationExecutingState:
return YES;
case AFHTTPOperationFinishedState:
return isCancelled;
default:
return NO;
}
case AFHTTPOperationExecutingState:
switch (toState) {
case AFHTTPOperationFinishedState:
return YES;
default:
return NO;
}
case AFHTTPOperationFinishedState:
return NO;
default:
return YES;
}
}
@interface AFURLConnectionOperation () @interface AFURLConnectionOperation ()
@property (readwrite, nonatomic, assign) AFOperationState state; @property (readwrite, nonatomic, assign) AFOperationState state;
@property (readwrite, nonatomic, assign, getter = isCancelled) BOOL cancelled; @property (readwrite, nonatomic, assign, getter = isCancelled) BOOL cancelled;
@property (readwrite, nonatomic, retain) NSRecursiveLock *lock;
@property (readwrite, nonatomic, assign) NSURLConnection *connection; @property (readwrite, nonatomic, assign) NSURLConnection *connection;
@property (readwrite, nonatomic, retain) NSURLRequest *request; @property (readwrite, nonatomic, retain) NSURLRequest *request;
@property (readwrite, nonatomic, retain) NSURLResponse *response; @property (readwrite, nonatomic, retain) NSURLResponse *response;
@ -65,7 +95,6 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
@property (readwrite, nonatomic, copy) AFURLConnectionOperationProgressBlock uploadProgress; @property (readwrite, nonatomic, copy) AFURLConnectionOperationProgressBlock uploadProgress;
@property (readwrite, nonatomic, copy) AFURLConnectionOperationProgressBlock downloadProgress; @property (readwrite, nonatomic, copy) AFURLConnectionOperationProgressBlock downloadProgress;
- (BOOL)shouldTransitionToState:(AFOperationState)state;
- (void)operationDidStart; - (void)operationDidStart;
- (void)finish; - (void)finish;
@end @end
@ -86,6 +115,7 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
@synthesize outputStream = _outputStream; @synthesize outputStream = _outputStream;
@synthesize uploadProgress = _uploadProgress; @synthesize uploadProgress = _uploadProgress;
@synthesize downloadProgress = _downloadProgress; @synthesize downloadProgress = _downloadProgress;
@synthesize lock = _lock;
+ (void)networkRequestThreadEntryPoint:(id)__unused object { + (void)networkRequestThreadEntryPoint:(id)__unused object {
do { do {
@ -113,6 +143,9 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
return nil; return nil;
} }
self.lock = [[[NSRecursiveLock alloc] init] autorelease];
self.lock.name = kAFNetworkingLockName;
self.runLoopModes = [NSSet setWithObject:NSRunLoopCommonModes]; self.runLoopModes = [NSSet setWithObject:NSRunLoopCommonModes];
self.request = urlRequest; self.request = urlRequest;
@ -123,6 +156,8 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
} }
- (void)dealloc { - (void)dealloc {
[_lock release];
[_runLoopModes release]; [_runLoopModes release];
[_request release]; [_request release];
@ -132,7 +167,12 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
[_responseData release]; [_responseData release];
[_responseString release]; [_responseString release];
[_dataAccumulator release]; [_dataAccumulator release];
[_outputStream release]; _outputStream = nil;
if (_outputStream) {
[_outputStream close];
[_outputStream release];
_outputStream = nil;
}
[_uploadProgress release]; [_uploadProgress release];
[_downloadProgress release]; [_downloadProgress release];
@ -141,6 +181,7 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
} }
- (void)setCompletionBlock:(void (^)(void))block { - (void)setCompletionBlock:(void (^)(void))block {
[self.lock lock];
if (!block) { if (!block) {
[super setCompletionBlock:nil]; [super setCompletionBlock:nil];
} else { } else {
@ -150,6 +191,7 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
[_blockSelf setCompletionBlock:nil]; [_blockSelf setCompletionBlock:nil];
}]; }];
} }
[self.lock unlock];
} }
- (NSInputStream *)inputStream { - (NSInputStream *)inputStream {
@ -171,65 +213,41 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
} }
- (void)setState:(AFOperationState)state { - (void)setState:(AFOperationState)state {
if (![self shouldTransitionToState:state]) { [self.lock lock];
return; if (AFStateTransitionIsValid(self.state, state, [self isCancelled])) {
} NSString *oldStateKey = AFKeyPathFromOperationState(self.state);
NSString *newStateKey = AFKeyPathFromOperationState(state);
NSString *oldStateKey = AFKeyPathFromOperationState(self.state);
NSString *newStateKey = AFKeyPathFromOperationState(state); [self willChangeValueForKey:newStateKey];
[self willChangeValueForKey:oldStateKey];
[self willChangeValueForKey:newStateKey]; _state = state;
[self willChangeValueForKey:oldStateKey]; [self didChangeValueForKey:oldStateKey];
_state = state; [self didChangeValueForKey:newStateKey];
[self didChangeValueForKey:oldStateKey];
[self didChangeValueForKey:newStateKey]; switch (state) {
case AFHTTPOperationExecutingState:
switch (state) { [[NSNotificationCenter defaultCenter] postNotificationName:AFNetworkingOperationDidStartNotification object:self];
case AFHTTPOperationExecutingState: break;
[[NSNotificationCenter defaultCenter] postNotificationName:AFNetworkingOperationDidStartNotification object:self]; case AFHTTPOperationFinishedState:
break; [[NSNotificationCenter defaultCenter] postNotificationName:AFNetworkingOperationDidFinishNotification object:self];
case AFHTTPOperationFinishedState: break;
[[NSNotificationCenter defaultCenter] postNotificationName:AFNetworkingOperationDidFinishNotification object:self]; default:
break; break;
default: }
break;
}
}
- (BOOL)shouldTransitionToState:(AFOperationState)state {
switch (self.state) {
case AFHTTPOperationReadyState:
switch (state) {
case AFHTTPOperationExecutingState:
return YES;
default:
return NO;
}
case AFHTTPOperationExecutingState:
switch (state) {
case AFHTTPOperationFinishedState:
return YES;
default:
return NO;
}
case AFHTTPOperationFinishedState:
return NO;
default:
return YES;
} }
[self.lock unlock];
} }
- (void)setCancelled:(BOOL)cancelled { - (void)setCancelled:(BOOL)cancelled {
[self.lock lock];
[self willChangeValueForKey:@"isCancelled"]; [self willChangeValueForKey:@"isCancelled"];
_cancelled = cancelled; _cancelled = cancelled;
[self didChangeValueForKey:@"isCancelled"]; [self didChangeValueForKey:@"isCancelled"];
[self.lock unlock];
if ([self isCancelled]) {
self.state = AFHTTPOperationFinishedState;
}
} }
- (NSString *)responseString { - (NSString *)responseString {
[self.lock lock];
if (!_responseString && self.response && self.responseData) { if (!_responseString && self.response && self.responseData) {
NSStringEncoding textEncoding = NSUTF8StringEncoding; NSStringEncoding textEncoding = NSUTF8StringEncoding;
if (self.response.textEncodingName) { if (self.response.textEncodingName) {
@ -238,6 +256,7 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
self.responseString = [[[NSString alloc] initWithData:self.responseData encoding:textEncoding] autorelease]; self.responseString = [[[NSString alloc] initWithData:self.responseData encoding:textEncoding] autorelease];
} }
[self.lock unlock];
return _responseString; return _responseString;
} }
@ -271,20 +290,21 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
} }
- (void)operationDidStart { - (void)operationDidStart {
[self.lock lock];
if ([self isCancelled]) { if ([self isCancelled]) {
[self finish]; [self finish];
return; } else {
self.connection = [[[NSURLConnection alloc] initWithRequest:self.request delegate:self startImmediately:NO] autorelease];
NSRunLoop *runLoop = [NSRunLoop currentRunLoop];
for (NSString *runLoopMode in self.runLoopModes) {
[self.connection scheduleInRunLoop:runLoop forMode:runLoopMode];
[self.outputStream scheduleInRunLoop:runLoop forMode:runLoopMode];
}
[self.connection start];
} }
[self.lock unlock];
self.connection = [[[NSURLConnection alloc] initWithRequest:self.request delegate:self startImmediately:NO] autorelease];
NSRunLoop *runLoop = [NSRunLoop currentRunLoop];
for (NSString *runLoopMode in self.runLoopModes) {
[self.connection scheduleInRunLoop:runLoop forMode:runLoopMode];
[self.outputStream scheduleInRunLoop:runLoop forMode:runLoopMode];
}
[self.connection start];
} }
- (void)finish { - (void)finish {
@ -292,15 +312,19 @@ static inline NSString * AFKeyPathFromOperationState(AFOperationState state) {
} }
- (void)cancel { - (void)cancel {
if ([self isFinished]) { [self.lock lock];
return; if (![self isFinished] && ![self isCancelled]) {
[super cancel];
self.cancelled = YES;
[self.connection cancel];
// We must send this delegate protcol message ourselves since the above [self.connection cancel] causes the connection to never send another message to its delegate.
NSDictionary *userInfo = [NSDictionary dictionaryWithObject:[self.request URL] forKey:NSURLErrorFailingURLErrorKey];
[self performSelector:@selector(connection:didFailWithError:) withObject:self.connection withObject:[NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorCancelled userInfo:userInfo]];
} }
[self.lock unlock];
[super cancel];
self.cancelled = YES;
[self.connection cancel];
} }
#pragma mark - NSURLConnectionDelegate #pragma mark - NSURLConnectionDelegate
@ -361,7 +385,7 @@ didReceiveResponse:(NSURLResponse *)response
- (void)connection:(NSURLConnection *)__unused connection - (void)connection:(NSURLConnection *)__unused connection
didFailWithError:(NSError *)error didFailWithError:(NSError *)error
{ {
self.error = error; self.error = error;
if (self.outputStream) { if (self.outputStream) {