Committed by
Ben Copsey
Change mechanism for calling delegates to be more thread safe
The previous mechanism can end up calling a delegate that has been removed by a setDelegate:nil, and hence may already be deallocated. Now we run a callback on the mainthread, which only dereferences the delegate/queue and selectors to be called when it runs on the main thread. The authenticationdelegate code is still using the old method. This should be looked at separately; it is slightly more complex as it needs to know if a delegate exists to be called.
Showing
3 changed files
with
13 additions
and
13 deletions
| @@ -508,10 +508,10 @@ extern unsigned long const ASIWWANBandwidthThrottleAmount; | @@ -508,10 +508,10 @@ extern unsigned long const ASIWWANBandwidthThrottleAmount; | ||
| 508 | - (void)incrementUploadSizeBy:(long long)length; | 508 | - (void)incrementUploadSizeBy:(long long)length; |
| 509 | 509 | ||
| 510 | // Helper method for interacting with progress indicators to abstract the details of different APIS (NSProgressIndicator and UIProgressView) | 510 | // Helper method for interacting with progress indicators to abstract the details of different APIS (NSProgressIndicator and UIProgressView) |
| 511 | -+ (void)updateProgressIndicator:(id)indicator withProgress:(unsigned long long)progress ofTotal:(unsigned long long)total; | 511 | ++ (void)updateProgressIndicator:(id *)indicator withProgress:(unsigned long long)progress ofTotal:(unsigned long long)total; |
| 512 | 512 | ||
| 513 | // Helper method used for performing invocations on the main thread (used for progress) | 513 | // Helper method used for performing invocations on the main thread (used for progress) |
| 514 | -+ (void)performSelector:(SEL)selector onTarget:(id)target withObject:(id)object amount:(void *)amount; | 514 | ++ (void)performSelector:(SEL)selector onTarget:(id *)target withObject:(id)object amount:(void *)amount; |
| 515 | 515 | ||
| 516 | #pragma mark handling request complete / failure | 516 | #pragma mark handling request complete / failure |
| 517 | 517 |
This diff is collapsed. Click to expand it.
| @@ -11,7 +11,7 @@ | @@ -11,7 +11,7 @@ | ||
| 11 | 11 | ||
| 12 | // Private stuff | 12 | // Private stuff |
| 13 | @interface ASINetworkQueue () | 13 | @interface ASINetworkQueue () |
| 14 | - - (void)resetProgressDelegate:(id)progressDelegate; | 14 | + - (void)resetProgressDelegate:(id *)progressDelegate; |
| 15 | @property (assign) int requestsCount; | 15 | @property (assign) int requestsCount; |
| 16 | @end | 16 | @end |
| 17 | 17 | ||
| @@ -79,33 +79,33 @@ | @@ -79,33 +79,33 @@ | ||
| 79 | - (void)setUploadProgressDelegate:(id)newDelegate | 79 | - (void)setUploadProgressDelegate:(id)newDelegate |
| 80 | { | 80 | { |
| 81 | uploadProgressDelegate = newDelegate; | 81 | uploadProgressDelegate = newDelegate; |
| 82 | - [self resetProgressDelegate:newDelegate]; | 82 | + [self resetProgressDelegate:&uploadProgressDelegate]; |
| 83 | 83 | ||
| 84 | } | 84 | } |
| 85 | 85 | ||
| 86 | - (void)setDownloadProgressDelegate:(id)newDelegate | 86 | - (void)setDownloadProgressDelegate:(id)newDelegate |
| 87 | { | 87 | { |
| 88 | downloadProgressDelegate = newDelegate; | 88 | downloadProgressDelegate = newDelegate; |
| 89 | - [self resetProgressDelegate:newDelegate]; | 89 | + [self resetProgressDelegate:&downloadProgressDelegate]; |
| 90 | } | 90 | } |
| 91 | 91 | ||
| 92 | -- (void)resetProgressDelegate:(id)progressDelegate | 92 | +- (void)resetProgressDelegate:(id *)progressDelegate |
| 93 | { | 93 | { |
| 94 | #if !TARGET_OS_IPHONE | 94 | #if !TARGET_OS_IPHONE |
| 95 | // If the uploadProgressDelegate is an NSProgressIndicator, we set its MaxValue to 1.0 so we can treat it similarly to UIProgressViews | 95 | // If the uploadProgressDelegate is an NSProgressIndicator, we set its MaxValue to 1.0 so we can treat it similarly to UIProgressViews |
| 96 | SEL selector = @selector(setMaxValue:); | 96 | SEL selector = @selector(setMaxValue:); |
| 97 | - if ([progressDelegate respondsToSelector:selector]) { | 97 | + if ([*progressDelegate respondsToSelector:selector]) { |
| 98 | double max = 1.0; | 98 | double max = 1.0; |
| 99 | [ASIHTTPRequest performSelector:selector onTarget:progressDelegate withObject:nil amount:&max]; | 99 | [ASIHTTPRequest performSelector:selector onTarget:progressDelegate withObject:nil amount:&max]; |
| 100 | } | 100 | } |
| 101 | selector = @selector(setDoubleValue:); | 101 | selector = @selector(setDoubleValue:); |
| 102 | - if ([progressDelegate respondsToSelector:selector]) { | 102 | + if ([*progressDelegate respondsToSelector:selector]) { |
| 103 | double value = 0.0; | 103 | double value = 0.0; |
| 104 | [ASIHTTPRequest performSelector:selector onTarget:progressDelegate withObject:nil amount:&value]; | 104 | [ASIHTTPRequest performSelector:selector onTarget:progressDelegate withObject:nil amount:&value]; |
| 105 | } | 105 | } |
| 106 | #else | 106 | #else |
| 107 | SEL selector = @selector(setProgress:); | 107 | SEL selector = @selector(setProgress:); |
| 108 | - if ([progressDelegate respondsToSelector:selector]) { | 108 | + if ([*progressDelegate respondsToSelector:selector]) { |
| 109 | float value = 0.0f; | 109 | float value = 0.0f; |
| 110 | [ASIHTTPRequest performSelector:selector onTarget:progressDelegate withObject:nil amount:&value]; | 110 | [ASIHTTPRequest performSelector:selector onTarget:progressDelegate withObject:nil amount:&value]; |
| 111 | } | 111 | } |
| @@ -153,7 +153,7 @@ | @@ -153,7 +153,7 @@ | ||
| 153 | [self addHEADOperation:HEADRequest]; | 153 | [self addHEADOperation:HEADRequest]; |
| 154 | [request addDependency:HEADRequest]; | 154 | [request addDependency:HEADRequest]; |
| 155 | if ([request shouldResetDownloadProgress]) { | 155 | if ([request shouldResetDownloadProgress]) { |
| 156 | - [self resetProgressDelegate:[request downloadProgressDelegate]]; | 156 | + [self resetProgressDelegate:&downloadProgressDelegate]; |
| 157 | [request setShouldResetDownloadProgress:NO]; | 157 | [request setShouldResetDownloadProgress:NO]; |
| 158 | } | 158 | } |
| 159 | } | 159 | } |
| @@ -168,7 +168,7 @@ | @@ -168,7 +168,7 @@ | ||
| 168 | } | 168 | } |
| 169 | // Tell the request not to increment the upload size when it starts, as we've already added its length | 169 | // Tell the request not to increment the upload size when it starts, as we've already added its length |
| 170 | if ([request shouldResetUploadProgress]) { | 170 | if ([request shouldResetUploadProgress]) { |
| 171 | - [self resetProgressDelegate:[request uploadProgressDelegate]]; | 171 | + [self resetProgressDelegate:&uploadProgressDelegate]; |
| 172 | [request setShouldResetUploadProgress:NO]; | 172 | [request setShouldResetUploadProgress:NO]; |
| 173 | } | 173 | } |
| 174 | 174 | ||
| @@ -229,7 +229,7 @@ | @@ -229,7 +229,7 @@ | ||
| 229 | { | 229 | { |
| 230 | [self setBytesDownloadedSoFar:[self bytesDownloadedSoFar]+bytes]; | 230 | [self setBytesDownloadedSoFar:[self bytesDownloadedSoFar]+bytes]; |
| 231 | if ([self downloadProgressDelegate]) { | 231 | if ([self downloadProgressDelegate]) { |
| 232 | - [ASIHTTPRequest updateProgressIndicator:[self downloadProgressDelegate] withProgress:[self bytesDownloadedSoFar] ofTotal:[self totalBytesToDownload]]; | 232 | + [ASIHTTPRequest updateProgressIndicator:&downloadProgressDelegate withProgress:[self bytesDownloadedSoFar] ofTotal:[self totalBytesToDownload]]; |
| 233 | } | 233 | } |
| 234 | } | 234 | } |
| 235 | 235 | ||
| @@ -237,7 +237,7 @@ | @@ -237,7 +237,7 @@ | ||
| 237 | { | 237 | { |
| 238 | [self setBytesUploadedSoFar:[self bytesUploadedSoFar]+bytes]; | 238 | [self setBytesUploadedSoFar:[self bytesUploadedSoFar]+bytes]; |
| 239 | if ([self uploadProgressDelegate]) { | 239 | if ([self uploadProgressDelegate]) { |
| 240 | - [ASIHTTPRequest updateProgressIndicator:[self uploadProgressDelegate] withProgress:[self bytesUploadedSoFar] ofTotal:[self totalBytesToUpload]]; | 240 | + [ASIHTTPRequest updateProgressIndicator:&uploadProgressDelegate withProgress:[self bytesUploadedSoFar] ofTotal:[self totalBytesToUpload]]; |
| 241 | } | 241 | } |
| 242 | } | 242 | } |
| 243 | 243 |
-
Please register or login to post a comment