Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the thread block #399

Closed
erlangzhang opened this issue May 26, 2013 · 11 comments
Closed

the thread block #399

erlangzhang opened this issue May 26, 2013 · 11 comments

Comments

@erlangzhang
Copy link

I run the example demo, drag to Down an drag up, the UI thread is blocking,I find it is blocking at the class SDWebImageDownloader,
dispatch_sync(self.barrierQueue OR
dispatch_barrier_sync(self.barrierQueue
I hope it will help, thx

@fanyj
Copy link

fanyj commented Aug 8, 2013

I am facing the exactly same problem with the lastest release (v3.3) on ios5.0 or ios5.1. It's wired that this problem can't be reproducted on ios6.0 or higher. I stop the app on simulater and find out that there is about 50+ threads pasued on
SDWebImageDownloaderOperation start

        // Make sure to run the runloop in our background thread so it can process downloaded data
        CFRunLoopRun();

and
SDWebImageDownloader callbacksForURL:(NSURL *)url

    dispatch_sync(self.barrierQueue, ^
    {
        callbacksForURL = self.URLCallbacks[url];
    });

May be there is a deadlock.
image

@fanyj
Copy link

fanyj commented Aug 8, 2013

Nevermind,this problem has been disscussed here #425 , and fix can be found here #444 .

@erlangzhang
Copy link
Author

thans for your reply, it's very usefull.

@erlangzhang
Copy link
Author

but i found in the newest code, it will retain cycle again, in
SDWebImageDownloader the code

 id operation = blockOperation;

blockOperation = nil; // break retain cycle

return operation;

has no use,it can't break retain cycle

@erlangzhang
Copy link
Author

the newest code #397 is ios5, the thread will go to deadthread

@rs
Copy link
Member

rs commented Aug 8, 2013

Does it work better if you replace

     id operation = blockOperation;

by:

    __weak SDWebImageDownloaderOperation *operation = blockOperation;

@erlangzhang
Copy link
Author

i think it is not the blockOperation cause retain cycle;i down the newest code this time ,and there is my fix code ,hope this will help.

in file SDWebImageManager.m -----> downloadWithURL function

first: replace all "operation" to "weakOperation"
secnond: __block id subOperation;

        __weak typeof (subOperation) weakSubOperation = subOperation;

        subOperation = [self.imageDownloader downloadImageWithURL:url options:downloaderOptions progress:progressBlock completed:^(UIImage *downloadedImage, NSData *data, NSError *error, BOOL finished);

replace all "subOperation" to "weakSubOperation"

so all Code is

  • (id)downloadWithURL:(NSURL *)url options:(SDWebImageOptions)options progress:(SDWebImageDownloaderProgressBlock)progressBlock completed:(SDWebImageCompletedWithFinishedBlock)completedBlock

{

// Very common mistake is to send the URL using NSString object instead of NSURL. For some strange reason, XCode won't

// throw any warning for this type mismatch. Here we failsafe this error by allowing URLs to be passed as NSString.

if ([url isKindOfClass:NSString.class])

{

    url = [NSURL URLWithString:(NSString *)url];

}




// Prevents app crashing on argument type error like sending NSNull instead of NSURL

if (![url isKindOfClass:NSURL.class])

{

    url = nil;

}




__block SDWebImageCombinedOperation *operation = SDWebImageCombinedOperation.new;

__weak SDWebImageCombinedOperation *weakOperation = operation;



BOOL isFailedUrl = NO;

@synchronized(self.failedURLs)

{

    isFailedUrl = [self.failedURLs containsObject:url];

}




if (!url || !completedBlock || (!(options & SDWebImageRetryFailed) && isFailedUrl))

{

    if (completedBlock)

    {

        dispatch_main_sync_safe(^

        {

            NSError *error = [NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorFileDoesNotExist userInfo:nil];

            completedBlock(nil, error, SDImageCacheTypeNone, YES);

        });

    }

    return operation;

}




@synchronized(self.runningOperations)

{

    [self.runningOperations addObject:operation];

}

NSString *key = [self cacheKeyForURL:url];




[self.imageCache queryDiskCacheForKey:key done:^(UIImage *image, SDImageCacheType cacheType)

{

    if (weakOperation.isCancelled) return;




    if ((!image || options & SDWebImageRefreshCached) && (![self.delegate respondsToSelector:@selector(imageManager:shouldDownloadImageForURL:)] || [self.delegate imageManager:self shouldDownloadImageForURL:url]))

    {

        if (image && options & SDWebImageRefreshCached)

        {

            dispatch_main_sync_safe(^

            {

                // If image was found in the cache bug SDWebImageRefreshCached is provided, notify about the cached image

                // AND try to re-download it in order to let a chance to NSURLCache to refresh it from server.

                completedBlock(image, nil, cacheType, YES);

            });

        }




        // download if no image or requested to refresh anyway, and download allowed by delegate

        SDWebImageDownloaderOptions downloaderOptions = 0;

        if (options & SDWebImageLowPriority) downloaderOptions |= SDWebImageDownloaderLowPriority;

        if (options & SDWebImageProgressiveDownload) downloaderOptions |= SDWebImageDownloaderProgressiveDownload;

        if (options & SDWebImageRefreshCached) downloaderOptions |= SDWebImageDownloaderUseNSURLCache;

        if (image && options & SDWebImageRefreshCached)

        {

            // force progressive off if image already cached but forced refreshing

            downloaderOptions &= ~SDWebImageDownloaderProgressiveDownload;

            // ignore image read from NSURLCache if image if cached but force refreshing

            downloaderOptions |= SDWebImageDownloaderIgnoreCachedResponse;

        }

        __block id<SDWebImageOperation> subOperation;

        __weak typeof (subOperation) weakSubOperation = subOperation;

        subOperation = [self.imageDownloader downloadImageWithURL:url options:downloaderOptions progress:progressBlock completed:^(UIImage *downloadedImage, NSData *data, NSError *error, BOOL finished)

        {                

            if (weakOperation.cancelled)

            {

                dispatch_main_sync_safe(^

                {

                    completedBlock(nil, nil, SDImageCacheTypeNone, finished);

                });

            }

            else if (error)

            {

                dispatch_main_sync_safe(^

                {

                    completedBlock(nil, error, SDImageCacheTypeNone, finished);

                });




                if (error.code != NSURLErrorNotConnectedToInternet)

                {

                    @synchronized(self.failedURLs)

                    {

                        [self.failedURLs addObject:url];

                    }

                }

            }

            else

            {

                BOOL cacheOnDisk = !(options & SDWebImageCacheMemoryOnly);




                if (options & SDWebImageRefreshCached && image && !downloadedImage)

                {

                    // Image refresh hit the NSURLCache cache, do not call the completion block

                }

                // NOTE: We don't call transformDownloadedImage delegate method on animated images as most transformation code would mangle it

                else if (downloadedImage && !downloadedImage.images && [self.delegate respondsToSelector:@selector(imageManager:transformDownloadedImage:withURL:)])

                {

                    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^

                    {

                        UIImage *transformedImage = [self.delegate imageManager:self transformDownloadedImage:downloadedImage withURL:url];




                        dispatch_main_sync_safe(^

                        {

                            completedBlock(transformedImage, nil, SDImageCacheTypeNone, finished);

                        });




                        if (transformedImage && finished)

                        {

                            NSData *dataToStore = [transformedImage isEqual:downloadedImage] ? data : nil;

                            [self.imageCache storeImage:transformedImage imageData:dataToStore forKey:key toDisk:cacheOnDisk];

                        }

                    });

                }

                else

                {

                    dispatch_main_sync_safe(^

                    {

                        completedBlock(downloadedImage, nil, SDImageCacheTypeNone, finished);

                    });




                    if (downloadedImage && finished)

                    {

                        [self.imageCache storeImage:downloadedImage imageData:data forKey:key toDisk:cacheOnDisk];

                    }

                }

            }




            if (finished)

            {

                @synchronized(self.runningOperations)

                {

                    [self.runningOperations removeObject:weakOperation];

                }

            }

        }];

        operation.cancelBlock = ^{[weakSubOperation cancel];};

    }

    else if (image)

    {

        dispatch_main_sync_safe(^

        {

            completedBlock(image, nil, cacheType, YES);

        });

        @synchronized(self.runningOperations)

        {

            [self.runningOperations removeObject:weakOperation];

        }

    }

    else

    {

        // Image not in cache and download disallowed by delegate

        dispatch_main_sync_safe(^

        {

            completedBlock(nil, nil, SDImageCacheTypeNone, YES);

        });

        @synchronized(self.runningOperations)

        {

            [self.runningOperations removeObject:weakOperation];

        }

    }

}];




return operation;

}

but, may be there are some problem, may be the cancle download images operation can't cancel immediately.

rs pushed a commit that referenced this issue Aug 8, 2013
@rs
Copy link
Member

rs commented Aug 8, 2013

Does this helps ?
I can't find a way to reproduce the issue. I'm shooting in the dark.

@erlangzhang
Copy link
Author

are you running the code in iPhone 5.0 simulator,if you drag to down of the list ,and fast drag to top ,repeate this action, it will happen

RoCry pushed a commit to RoCry/SDWebImage that referenced this issue Aug 14, 2013
@amitpdev
Copy link

+1 for this issue -
I'm having the same problem with a table of ~200 records.
iOS 5.1 on iPhone simulator and iOS 5.01 on iPhone 4S:
UI is blocked for several seconds, then goes back to be responsive.
No crashes as far as I tested.

@rs
Copy link
Member

rs commented Aug 22, 2013

Please see #466

@rs rs closed this as completed Aug 22, 2013
devedup pushed a commit to FilmFlexMovies/SDWebImage that referenced this issue Sep 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants