Progress updates should now only happen in the main thread, fixing iphone issues
Showing
4 changed files
with
76 additions
and
23 deletions
| @@ -112,9 +112,6 @@ | @@ -112,9 +112,6 @@ | ||
| 112 | //This lock will block the request until the delegate supplies authentication info | 112 | //This lock will block the request until the delegate supplies authentication info |
| 113 | NSConditionLock *authenticationLock; | 113 | NSConditionLock *authenticationLock; |
| 114 | 114 | ||
| 115 | - //This lock prevents the operation from being cancelled while it is trying to update the progress, and vice versa | ||
| 116 | - NSLock *progressLock; | ||
| 117 | - | ||
| 118 | //Called on the delegate when the request completes successfully | 115 | //Called on the delegate when the request completes successfully |
| 119 | SEL didFinishSelector; | 116 | SEL didFinishSelector; |
| 120 | 117 |
| @@ -31,6 +31,9 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -31,6 +31,9 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 31 | [((ASIHTTPRequest*)clientCallBackInfo) handleNetworkEvent: type]; | 31 | [((ASIHTTPRequest*)clientCallBackInfo) handleNetworkEvent: type]; |
| 32 | } | 32 | } |
| 33 | 33 | ||
| 34 | +//This lock prevents the operation from being cancelled while it is trying to update the progress, and vice versa | ||
| 35 | + | ||
| 36 | +static NSLock *progressLock; | ||
| 34 | 37 | ||
| 35 | @implementation ASIHTTPRequest | 38 | @implementation ASIHTTPRequest |
| 36 | 39 | ||
| @@ -38,6 +41,11 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -38,6 +41,11 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 38 | 41 | ||
| 39 | #pragma mark init / dealloc | 42 | #pragma mark init / dealloc |
| 40 | 43 | ||
| 44 | +- (void)initialize | ||
| 45 | +{ | ||
| 46 | + progressLock = [[NSLock alloc] init]; | ||
| 47 | +} | ||
| 48 | + | ||
| 41 | - (id)initWithURL:(NSURL *)newURL | 49 | - (id)initWithURL:(NSURL *)newURL |
| 42 | { | 50 | { |
| 43 | self = [super init]; | 51 | self = [super init]; |
| @@ -85,7 +93,6 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -85,7 +93,6 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 85 | [domain release]; | 93 | [domain release]; |
| 86 | [authenticationRealm release]; | 94 | [authenticationRealm release]; |
| 87 | [url release]; | 95 | [url release]; |
| 88 | - [progressLock release]; | ||
| 89 | [authenticationLock release]; | 96 | [authenticationLock release]; |
| 90 | [lastActivityTime release]; | 97 | [lastActivityTime release]; |
| 91 | [responseCookies release]; | 98 | [responseCookies release]; |
| @@ -214,9 +221,6 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -214,9 +221,6 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 214 | - (void)loadRequest | 221 | - (void)loadRequest |
| 215 | { | 222 | { |
| 216 | CFRunLoopAddCommonMode(CFRunLoopGetCurrent(),ASIHTTPRequestRunMode); | 223 | CFRunLoopAddCommonMode(CFRunLoopGetCurrent(),ASIHTTPRequestRunMode); |
| 217 | - | ||
| 218 | - [progressLock release]; | ||
| 219 | - progressLock = [[NSLock alloc] init]; | ||
| 220 | 224 | ||
| 221 | [authenticationLock release]; | 225 | [authenticationLock release]; |
| 222 | authenticationLock = [[NSConditionLock alloc] initWithCondition:1]; | 226 | authenticationLock = [[NSConditionLock alloc] initWithCondition:1]; |
| @@ -270,14 +274,18 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -270,14 +274,18 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 270 | } | 274 | } |
| 271 | 275 | ||
| 272 | //Record when the request started, so we can timeout if nothing happens | 276 | //Record when the request started, so we can timeout if nothing happens |
| 273 | - [self setLastActivityTime:[[NSDate new] autorelease]]; | 277 | + [self setLastActivityTime:[NSDate date]]; |
| 274 | 278 | ||
| 275 | // Wait for the request to finish | 279 | // Wait for the request to finish |
| 276 | while (!complete) { | 280 | while (!complete) { |
| 277 | 281 | ||
| 282 | + //This may take a while, so we'll release the pool each cycle to stop a giant backlog building up | ||
| 283 | + [pool release]; | ||
| 284 | + pool = [[NSAutoreleasePool alloc] init]; | ||
| 285 | + | ||
| 278 | //See if we need to timeout | 286 | //See if we need to timeout |
| 279 | if (lastActivityTime && timeOutSeconds > 0) { | 287 | if (lastActivityTime && timeOutSeconds > 0) { |
| 280 | - if ([[[NSDate new] autorelease] timeIntervalSinceDate:lastActivityTime] > timeOutSeconds) { | 288 | + if ([[NSDate date] timeIntervalSinceDate:lastActivityTime] > timeOutSeconds) { |
| 281 | [self failWithProblem:@"Request timed out"]; | 289 | [self failWithProblem:@"Request timed out"]; |
| 282 | [self cancelLoad]; | 290 | [self cancelLoad]; |
| 283 | complete = YES; | 291 | complete = YES; |
| @@ -292,7 +300,7 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -292,7 +300,7 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 292 | complete = YES; | 300 | complete = YES; |
| 293 | break; | 301 | break; |
| 294 | } | 302 | } |
| 295 | - [self updateProgressIndicators]; | 303 | + [self performSelectorOnMainThread:@selector(updateProgressIndicators) withObject:nil waitUntilDone:YES]; |
| 296 | 304 | ||
| 297 | //This thread should wait for 1/4 second for the stream to do something. We'll stop early if it does. | 305 | //This thread should wait for 1/4 second for the stream to do something. We'll stop early if it does. |
| 298 | CFRunLoopRunInMode(ASIHTTPRequestRunMode,0.25,YES); | 306 | CFRunLoopRunInMode(ASIHTTPRequestRunMode,0.25,YES); |
| @@ -332,20 +340,16 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -332,20 +340,16 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 332 | 340 | ||
| 333 | - (void)updateProgressIndicators | 341 | - (void)updateProgressIndicators |
| 334 | { | 342 | { |
| 335 | - [progressLock lock]; | 343 | + |
| 336 | - if ([self isCancelled]) { | ||
| 337 | - [progressLock unlock]; | ||
| 338 | - return; | ||
| 339 | - } | ||
| 340 | [self updateUploadProgress]; | 344 | [self updateUploadProgress]; |
| 341 | [self updateDownloadProgress]; | 345 | [self updateDownloadProgress]; |
| 342 | - [progressLock unlock]; | ||
| 343 | 346 | ||
| 344 | } | 347 | } |
| 345 | 348 | ||
| 346 | 349 | ||
| 347 | + (void)setProgress:(double)progress forProgressIndicator:(id)indicator | 350 | + (void)setProgress:(double)progress forProgressIndicator:(id)indicator |
| 348 | { | 351 | { |
| 352 | + | ||
| 349 | SEL selector; | 353 | SEL selector; |
| 350 | 354 | ||
| 351 | //Cocoa Touch: UIProgressView | 355 | //Cocoa Touch: UIProgressView |
| @@ -393,6 +397,12 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -393,6 +397,12 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 393 | 397 | ||
| 394 | -(void)removeUploadProgressSoFar | 398 | -(void)removeUploadProgressSoFar |
| 395 | { | 399 | { |
| 400 | + [progressLock lock]; | ||
| 401 | + if ([self isCancelled]) { | ||
| 402 | + [progressLock unlock]; | ||
| 403 | + return; | ||
| 404 | + } | ||
| 405 | + | ||
| 396 | //We're using a progress queue or compatible controller to handle progress | 406 | //We're using a progress queue or compatible controller to handle progress |
| 397 | if ([uploadProgressDelegate respondsToSelector:@selector(incrementUploadProgressBy:)]) { | 407 | if ([uploadProgressDelegate respondsToSelector:@selector(incrementUploadProgressBy:)]) { |
| 398 | int value = 0-lastBytesSent; | 408 | int value = 0-lastBytesSent; |
| @@ -407,12 +417,19 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -407,12 +417,19 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 407 | //We aren't using a queue, we should just set progress of the indicator to 0 | 417 | //We aren't using a queue, we should just set progress of the indicator to 0 |
| 408 | } else { | 418 | } else { |
| 409 | [ASIHTTPRequest setProgress:0 forProgressIndicator:uploadProgressDelegate]; | 419 | [ASIHTTPRequest setProgress:0 forProgressIndicator:uploadProgressDelegate]; |
| 410 | - } | 420 | + } |
| 421 | + [progressLock unlock]; | ||
| 411 | } | 422 | } |
| 412 | 423 | ||
| 413 | 424 | ||
| 414 | - (void)resetUploadProgress:(NSNumber *)max | 425 | - (void)resetUploadProgress:(NSNumber *)max |
| 415 | { | 426 | { |
| 427 | + [progressLock lock]; | ||
| 428 | + if ([self isCancelled]) { | ||
| 429 | + [progressLock unlock]; | ||
| 430 | + return; | ||
| 431 | + } | ||
| 432 | + | ||
| 416 | //We're using a progress queue or compatible controller to handle progress | 433 | //We're using a progress queue or compatible controller to handle progress |
| 417 | if ([uploadProgressDelegate respondsToSelector:@selector(incrementUploadSizeBy:)]) { | 434 | if ([uploadProgressDelegate respondsToSelector:@selector(incrementUploadSizeBy:)]) { |
| 418 | int value = [max intValue]; | 435 | int value = [max intValue]; |
| @@ -426,11 +443,18 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -426,11 +443,18 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 426 | } else { | 443 | } else { |
| 427 | [ASIHTTPRequest setProgress:0 forProgressIndicator:uploadProgressDelegate]; | 444 | [ASIHTTPRequest setProgress:0 forProgressIndicator:uploadProgressDelegate]; |
| 428 | } | 445 | } |
| 446 | + [progressLock unlock]; | ||
| 429 | } | 447 | } |
| 430 | 448 | ||
| 431 | - (void)updateUploadProgress | 449 | - (void)updateUploadProgress |
| 432 | { | 450 | { |
| 433 | - [self setLastActivityTime:[[NSDate new] autorelease]]; | 451 | + [progressLock lock]; |
| 452 | + if ([self isCancelled]) { | ||
| 453 | + [progressLock unlock]; | ||
| 454 | + return; | ||
| 455 | + } | ||
| 456 | + | ||
| 457 | + [self setLastActivityTime:[NSDate date]]; | ||
| 434 | 458 | ||
| 435 | unsigned int byteCount = [[(NSNumber *)CFReadStreamCopyProperty (readStream, kCFStreamPropertyHTTPRequestBytesWrittenCount) autorelease] unsignedIntValue]; | 459 | unsigned int byteCount = [[(NSNumber *)CFReadStreamCopyProperty (readStream, kCFStreamPropertyHTTPRequestBytesWrittenCount) autorelease] unsignedIntValue]; |
| 436 | if (uploadProgressDelegate) { | 460 | if (uploadProgressDelegate) { |
| @@ -439,7 +463,8 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -439,7 +463,8 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 439 | if ([uploadProgressDelegate respondsToSelector:@selector(incrementUploadProgressBy:)]) { | 463 | if ([uploadProgressDelegate respondsToSelector:@selector(incrementUploadProgressBy:)]) { |
| 440 | int value = byteCount-lastBytesSent; | 464 | int value = byteCount-lastBytesSent; |
| 441 | SEL selector = @selector(incrementUploadProgressBy:); | 465 | SEL selector = @selector(incrementUploadProgressBy:); |
| 442 | - NSMethodSignature *signature = [[uploadProgressDelegate class] instanceMethodSignatureForSelector:selector]; | 466 | + NSMethodSignature *signature = nil; |
| 467 | + signature = [[uploadProgressDelegate class] instanceMethodSignatureForSelector:selector]; | ||
| 443 | NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; | 468 | NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:signature]; |
| 444 | [invocation setTarget:uploadProgressDelegate]; | 469 | [invocation setTarget:uploadProgressDelegate]; |
| 445 | [invocation setSelector:selector]; | 470 | [invocation setSelector:selector]; |
| @@ -453,11 +478,19 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -453,11 +478,19 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 453 | 478 | ||
| 454 | } | 479 | } |
| 455 | lastBytesSent = byteCount; | 480 | lastBytesSent = byteCount; |
| 481 | + [progressLock unlock]; | ||
| 456 | } | 482 | } |
| 457 | 483 | ||
| 458 | 484 | ||
| 459 | - (void)resetDownloadProgress:(NSNumber *)max | 485 | - (void)resetDownloadProgress:(NSNumber *)max |
| 460 | { | 486 | { |
| 487 | + [progressLock lock]; | ||
| 488 | + if ([self isCancelled]) { | ||
| 489 | + [progressLock unlock]; | ||
| 490 | + return; | ||
| 491 | + } | ||
| 492 | + | ||
| 493 | + | ||
| 461 | //We're using a progress queue or compatible controller to handle progress | 494 | //We're using a progress queue or compatible controller to handle progress |
| 462 | if ([downloadProgressDelegate respondsToSelector:@selector(incrementDownloadSizeBy:)]) { | 495 | if ([downloadProgressDelegate respondsToSelector:@selector(incrementDownloadSizeBy:)]) { |
| 463 | int value = [max intValue]; | 496 | int value = [max intValue]; |
| @@ -471,17 +504,27 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -471,17 +504,27 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 471 | } else { | 504 | } else { |
| 472 | [ASIHTTPRequest setProgress:0 forProgressIndicator:downloadProgressDelegate]; | 505 | [ASIHTTPRequest setProgress:0 forProgressIndicator:downloadProgressDelegate]; |
| 473 | } | 506 | } |
| 507 | + [progressLock unlock]; | ||
| 474 | } | 508 | } |
| 475 | 509 | ||
| 476 | - (void)updateDownloadProgress | 510 | - (void)updateDownloadProgress |
| 477 | { | 511 | { |
| 478 | - [self setLastActivityTime:[[NSDate new] autorelease]]; | 512 | + [progressLock lock]; |
| 513 | + if ([self isCancelled]) { | ||
| 514 | + [progressLock unlock]; | ||
| 515 | + return; | ||
| 516 | + } | ||
| 517 | + | ||
| 518 | + [self setLastActivityTime:[NSDate date]]; | ||
| 479 | 519 | ||
| 480 | //We won't update downlaod progress until we've examined the headers, since we might need to authenticate | 520 | //We won't update downlaod progress until we've examined the headers, since we might need to authenticate |
| 481 | if (downloadProgressDelegate && responseHeaders) { | 521 | if (downloadProgressDelegate && responseHeaders) { |
| 482 | 522 | ||
| 483 | //We're using a progress queue or compatible controller to handle progress | 523 | //We're using a progress queue or compatible controller to handle progress |
| 484 | if ([downloadProgressDelegate respondsToSelector:@selector(incrementDownloadProgressBy:)]) { | 524 | if ([downloadProgressDelegate respondsToSelector:@selector(incrementDownloadProgressBy:)]) { |
| 525 | + | ||
| 526 | + NSAutoreleasePool *thePool = [[NSAutoreleasePool alloc] init]; | ||
| 527 | + | ||
| 485 | int value = totalBytesRead-lastBytesRead; | 528 | int value = totalBytesRead-lastBytesRead; |
| 486 | SEL selector = @selector(incrementDownloadProgressBy:); | 529 | SEL selector = @selector(incrementDownloadProgressBy:); |
| 487 | NSMethodSignature *signature = [[downloadProgressDelegate class] instanceMethodSignatureForSelector:selector]; | 530 | NSMethodSignature *signature = [[downloadProgressDelegate class] instanceMethodSignatureForSelector:selector]; |
| @@ -491,13 +534,16 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | @@ -491,13 +534,16 @@ static void ReadStreamClientCallBack(CFReadStreamRef readStream, CFStreamEventTy | ||
| 491 | [invocation setArgument:&value atIndex:2]; | 534 | [invocation setArgument:&value atIndex:2]; |
| 492 | [invocation invoke]; | 535 | [invocation invoke]; |
| 493 | 536 | ||
| 537 | + [thePool release]; | ||
| 538 | + | ||
| 494 | //We aren't using a queue, we should just set progress of the indicator to 0 | 539 | //We aren't using a queue, we should just set progress of the indicator to 0 |
| 495 | } else if (contentLength > 0) { | 540 | } else if (contentLength > 0) { |
| 496 | [ASIHTTPRequest setProgress:(double)(totalBytesRead/contentLength) forProgressIndicator:downloadProgressDelegate]; | 541 | [ASIHTTPRequest setProgress:(double)(totalBytesRead/contentLength) forProgressIndicator:downloadProgressDelegate]; |
| 497 | } | 542 | } |
| 498 | 543 | ||
| 499 | lastBytesRead = totalBytesRead; | 544 | lastBytesRead = totalBytesRead; |
| 500 | - } | 545 | + } |
| 546 | + [progressLock unlock]; | ||
| 501 | } | 547 | } |
| 502 | 548 | ||
| 503 | #pragma mark handling request complete / failure | 549 | #pragma mark handling request complete / failure |
| @@ -80,8 +80,16 @@ | @@ -80,8 +80,16 @@ | ||
| 80 | { | 80 | { |
| 81 | if ([operation isKindOfClass:[ASIHTTPRequest class]]) { | 81 | if ([operation isKindOfClass:[ASIHTTPRequest class]]) { |
| 82 | requestsCount++; | 82 | requestsCount++; |
| 83 | - [(ASIHTTPRequest *)operation setUploadProgressDelegate:self]; | 83 | + if (uploadProgressDelegate) { |
| 84 | - [(ASIHTTPRequest *)operation setDownloadProgressDelegate:self]; | 84 | + [(ASIHTTPRequest *)operation setUploadProgressDelegate:self]; |
| 85 | + } else { | ||
| 86 | + [(ASIHTTPRequest *)operation setUploadProgressDelegate:NULL]; | ||
| 87 | + } | ||
| 88 | + if (downloadProgressDelegate) { | ||
| 89 | + [(ASIHTTPRequest *)operation setDownloadProgressDelegate:self]; | ||
| 90 | + } else { | ||
| 91 | + [(ASIHTTPRequest *)operation setDownloadProgressDelegate:NULL]; | ||
| 92 | + } | ||
| 85 | [(ASIHTTPRequest *)operation setDelegate:self]; | 93 | [(ASIHTTPRequest *)operation setDelegate:self]; |
| 86 | [(ASIHTTPRequest *)operation setDidFailSelector:@selector(requestDidFail:)]; | 94 | [(ASIHTTPRequest *)operation setDidFailSelector:@selector(requestDidFail:)]; |
| 87 | [(ASIHTTPRequest *)operation setDidFinishSelector:@selector(requestDidFinish:)]; | 95 | [(ASIHTTPRequest *)operation setDidFinishSelector:@selector(requestDidFinish:)]; |
| 1 | To do: | 1 | To do: |
| 2 | +Add note about not locking main thread when using progress | ||
| 2 | Replace this with a link to the docs? | 3 | Replace this with a link to the docs? |
| 4 | +Add to release notes - lots of leaks fixed, and it it releases temporary stuff during a request rather than waiting until the end | ||
| 3 | 5 | ||
| 4 | 6 | ||
| 5 | [NEW!] Documentation is available at: http://allseeing-i.com/asi-http-request | 7 | [NEW!] Documentation is available at: http://allseeing-i.com/asi-http-request |
-
Please register or login to post a comment