From dd68f2f2d48b84b6c8bbc3f43d0bc209ea5b1c71 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sun, 28 Jan 2018 23:44:53 +0800 Subject: [PATCH 1/2] Ensure all the session delegate completionHandler called. Fix the leak when response error code below iOS 10 --- SDWebImage/SDWebImageDownloader.m | 42 ++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/SDWebImage/SDWebImageDownloader.m b/SDWebImage/SDWebImageDownloader.m index 88d22bf5..61eef5ed 100644 --- a/SDWebImage/SDWebImageDownloader.m +++ b/SDWebImage/SDWebImageDownloader.m @@ -318,16 +318,22 @@ didReceiveResponse:(NSURLResponse *)response // Identify the operation that runs this task and pass it the delegate method SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:dataTask]; - - [dataOperation URLSession:session dataTask:dataTask didReceiveResponse:response completionHandler:completionHandler]; + if ([dataOperation respondsToSelector:@selector(URLSession:dataTask:didReceiveResponse:completionHandler:)]) { + [dataOperation URLSession:session dataTask:dataTask didReceiveResponse:response completionHandler:completionHandler]; + } else { + if (completionHandler) { + completionHandler(NSURLSessionResponseAllow); + } + } } - (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveData:(NSData *)data { // Identify the operation that runs this task and pass it the delegate method SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:dataTask]; - - [dataOperation URLSession:session dataTask:dataTask didReceiveData:data]; + if ([dataOperation respondsToSelector:@selector(URLSession:dataTask:didReceiveData:)]) { + [dataOperation URLSession:session dataTask:dataTask didReceiveData:data]; + } } - (void)URLSession:(NSURLSession *)session @@ -337,8 +343,13 @@ didReceiveResponse:(NSURLResponse *)response // Identify the operation that runs this task and pass it the delegate method SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:dataTask]; - - [dataOperation URLSession:session dataTask:dataTask willCacheResponse:proposedResponse completionHandler:completionHandler]; + if ([dataOperation respondsToSelector:@selector(URLSession:dataTask:willCacheResponse:completionHandler:)]) { + [dataOperation URLSession:session dataTask:dataTask willCacheResponse:proposedResponse completionHandler:completionHandler]; + } else { + if (completionHandler) { + completionHandler(proposedResponse); + } + } } #pragma mark NSURLSessionTaskDelegate @@ -347,19 +358,21 @@ didReceiveResponse:(NSURLResponse *)response // Identify the operation that runs this task and pass it the delegate method SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:task]; - - [dataOperation URLSession:session task:task didCompleteWithError:error]; + if ([dataOperation respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]) { + [dataOperation URLSession:session task:task didCompleteWithError:error]; + } } - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPerformHTTPRedirection:(NSHTTPURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest * _Nullable))completionHandler { // Identify the operation that runs this task and pass it the delegate method SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:task]; - if ([dataOperation respondsToSelector:@selector(URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:)]) { [dataOperation URLSession:session task:task willPerformHTTPRedirection:response newRequest:request completionHandler:completionHandler]; } else { - completionHandler(request); + if (completionHandler) { + completionHandler(request); + } } } @@ -367,8 +380,13 @@ didReceiveResponse:(NSURLResponse *)response // Identify the operation that runs this task and pass it the delegate method SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:task]; - - [dataOperation URLSession:session task:task didReceiveChallenge:challenge completionHandler:completionHandler]; + if ([dataOperation respondsToSelector:@selector(URLSession:task:didReceiveChallenge:completionHandler:)]) { + [dataOperation URLSession:session task:task didReceiveChallenge:challenge completionHandler:completionHandler]; + } else { + if (completionHandler) { + completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil); + } + } } @end From 9080afdbacab897465d682c11e33e0357f5c5923 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Mon, 29 Jan 2018 01:16:41 +0800 Subject: [PATCH 2/2] Use the correct way to specify cancel if the response status code is invalid. --- SDWebImage/SDWebImageDownloaderOperation.m | 50 ++++++++-------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/SDWebImage/SDWebImageDownloaderOperation.m b/SDWebImage/SDWebImageDownloaderOperation.m index 553b0e95..ad8d5976 100644 --- a/SDWebImage/SDWebImageDownloaderOperation.m +++ b/SDWebImage/SDWebImageDownloaderOperation.m @@ -182,7 +182,7 @@ typedef NSMutableDictionary SDCallbacksDictionary; [[NSNotificationCenter defaultCenter] postNotificationName:SDWebImageDownloadStartNotification object:weakSelf]; }); } else { - [self callCompletionBlocksWithError:[NSError errorWithDomain:NSURLErrorDomain code:0 userInfo:@{NSLocalizedDescriptionKey : @"Connection can't be initialized"}]]; + [self callCompletionBlocksWithError:[NSError errorWithDomain:NSURLErrorDomain code:0 userInfo:@{NSLocalizedDescriptionKey : @"Task can't be initialized"}]]; } #if SD_UIKIT @@ -215,7 +215,7 @@ typedef NSMutableDictionary SDCallbacksDictionary; [[NSNotificationCenter defaultCenter] postNotificationName:SDWebImageDownloadStopNotification object:weakSelf]; }); - // As we cancelled the connection, its callback won't be called and thus won't + // As we cancelled the task, its callback won't be called and thus won't // maintain the isFinished and isExecuting flags. if (self.isExecuting) self.executing = NO; if (!self.isFinished) self.finished = YES; @@ -278,48 +278,36 @@ typedef NSMutableDictionary SDCallbacksDictionary; dataTask:(NSURLSessionDataTask *)dataTask didReceiveResponse:(NSURLResponse *)response completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { + NSURLSessionResponseDisposition disposition = NSURLSessionResponseAllow; + NSInteger expected = (NSInteger)response.expectedContentLength; + expected = expected > 0 ? expected : 0; + self.expectedSize = expected; + self.response = response; - //'304 Not Modified' is an exceptional one + //'304 Not Modified' is an exceptional one. It should be treated as cancelled. if (![response respondsToSelector:@selector(statusCode)] || (((NSHTTPURLResponse *)response).statusCode < 400 && ((NSHTTPURLResponse *)response).statusCode != 304)) { - NSInteger expected = (NSInteger)response.expectedContentLength; - expected = expected > 0 ? expected : 0; - self.expectedSize = expected; for (SDWebImageDownloaderProgressBlock progressBlock in [self callbacksForKey:kProgressCallbackKey]) { progressBlock(0, expected, self.request.URL); } - - self.imageData = [[NSMutableData alloc] initWithCapacity:expected]; - self.response = response; - __weak typeof(self) weakSelf = self; - dispatch_async(dispatch_get_main_queue(), ^{ - [[NSNotificationCenter defaultCenter] postNotificationName:SDWebImageDownloadReceiveResponseNotification object:weakSelf]; - }); } else { - NSUInteger code = ((NSHTTPURLResponse *)response).statusCode; - - //This is the case when server returns '304 Not Modified'. It means that remote image is not changed. - //In case of 304 we need just cancel the operation and return cached image from the cache. - if (code == 304) { - [self cancelInternal]; - } else { - [self.dataTask cancel]; - } - __weak typeof(self) weakSelf = self; - dispatch_async(dispatch_get_main_queue(), ^{ - [[NSNotificationCenter defaultCenter] postNotificationName:SDWebImageDownloadStopNotification object:weakSelf]; - }); - - [self callCompletionBlocksWithError:[NSError errorWithDomain:NSURLErrorDomain code:((NSHTTPURLResponse *)response).statusCode userInfo:nil]]; - - [self done]; + // Status code invalid and marked as cancelled. Do not call `[self.dataTask cancel]` which may mass up URLSession life cycle + disposition = NSURLSessionResponseCancel; } + __weak typeof(self) weakSelf = self; + dispatch_async(dispatch_get_main_queue(), ^{ + [[NSNotificationCenter defaultCenter] postNotificationName:SDWebImageDownloadReceiveResponseNotification object:weakSelf]; + }); + if (completionHandler) { - completionHandler(NSURLSessionResponseAllow); + completionHandler(disposition); } } - (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveData:(NSData *)data { + if (!self.imageData) { + self.imageData = [[NSMutableData alloc] initWithCapacity:self.expectedSize]; + } [self.imageData appendData:data]; if ((self.options & SDWebImageDownloaderProgressiveDownload) && self.expectedSize > 0) {