SDWebImage / SDWebImage

Asynchronous image downloader with cache support as a UIImageView category
https://sdwebimage.github.io
MIT License
25.04k stars 5.97k forks source link

dispatch_barrier_sync blocks main thread when re-requesting an image. #803

Closed mtitolo closed 6 years ago

mtitolo commented 10 years ago

I'm using UIImageView+WebCache to load some images into a bunch of UICollectionViewCells. Instead of the nice scrolling I'm used to, the screen will occasionally entirely freeze up, the collection view scrolling animations will pause, and touches do nothing. Turns out, a semaphore_wait_trap caused by a dispatch_barrier_sync is blocking the main thread.

This only happens when an image request was cancelled, and then started again. It's fine for cached images. Pretty sure all of this should be done on a background thread.

I'm only calling setImageURL:withPlaceholderImage: so no fancy stuff.

Edit: This will only reproduce on device. Issue is present on iPhone 4 (7.1.1) and latest gen iPod Touch (7.1.2). Not present on iPhone 5 (7.0.6).

* thread #1: tid = 0xaba2, 0x396dcaa8 libsystem_kernel.dylib`semaphore_wait_trap + 8, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  * frame #0: 0x396dcaa8 libsystem_kernel.dylib`semaphore_wait_trap + 8
    frame #1: 0x39751bac libsystem_platform.dylib`_os_semaphore_wait + 12
    frame #2: 0x39626c3e libdispatch.dylib`_dispatch_barrier_sync_f_slow + 134
    frame #3: 0x001d1854 ama`-[SDWebImageDownloader addProgressCallback:andCompletedBlock:forURL:createCallback:](self=0x16879790, _cmd=0x00215fb0, progressBlock=0x00000000, completedBlock=0x27d9cc94, url=0x1683c050, createCallback=0x27d9c9bc) + 396 at SDWebImageDownloader.m:180
    frame #4: 0x001d05b2 ama`-[SDWebImageDownloader downloadImageWithURL:options:progress:completed:](self=0x16879790, _cmd=0x00216078, url=0x1683c050, options=0, progressBlock=0x00000000, completedBlock=0x27d9cc94) + 406 at SDWebImageDownloader.m:111
    frame #5: 0x001d759e ama`__64-[SDWebImageManager downloadWithURL:options:progress:completed:]_block_invoke59(.block_descriptor=<unavailable>, image=0x00000000, cacheType=SDImageCacheTypeDisk) + 1702 at SDWebImageManager.m:137
    frame #6: 0x001cc7fe ama`__42-[SDImageCache queryDiskCacheForKey:done:]_block_invoke_2(.block_descriptor=<unavailable>) + 42 at SDImageCache.m:297
    frame #7: 0x39614832 libdispatch.dylib`_dispatch_call_block_and_release + 10
    frame #8: 0x3961481e libdispatch.dylib`_dispatch_client_callout + 22
    frame #9: 0x39614776 libdispatch.dylib`_dispatch_main_queue_callback_4CF$VARIANT$up + 254
    frame #10: 0x2e9468a0 CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 8
    frame #11: 0x2e945174 CoreFoundation`__CFRunLoopRun + 1300
    frame #12: 0x2e8afebe CoreFoundation`CFRunLoopRunSpecific + 522
    frame #13: 0x2e8afca2 CoreFoundation`CFRunLoopRunInMode + 106
    frame #14: 0x337b5662 GraphicsServices`GSEventRunModal + 138
    frame #15: 0x311fc14c UIKit`UIApplicationMain + 1136
    frame #16: 0x0009d1a4 ama`main(argc=1, argv=0x27d9dd04) + 116 at main.m:16
bpoplauschi commented 10 years ago

Hi @mtitolo. I've tried to reproduce this scenario, but without success. How is the cancelling occur? (is it the sd_cancelCurrentImageLoad from the sd_setImageWithURL:placeholderImage:options:progress:completed: or you are cancelling explicitly? Could you set up a demo project where the issue can be easily reproducible?

mtitolo commented 10 years ago

Took a bit of poking, and some symbolic breakpoint trickery, but I managed to reproduce the issue.

Code: https://github.com/mtitolo/cats/tree/sdwebimage-scrolling Here's an Instagram key to use: 42988e0cdc54410b80aceeb4309faf19

How to reproduce:

  1. Set a breakpoint in applicationDidFinishLaunching...
  2. Get a device, iPhone 4 or equiv, and enable Network Link Conditioner on the 3G setting.
  3. Run on the device.
  4. When the breakpoint is hit, in lldb run breakpoint set -n semaphore_wait_trap
  5. Next, run script and paste in this script:
find_in_stack = ['-[SDWebImageDownloader addProgressCallback:andCompletedBlock:forURL:createCallback:]']
def continue_ignored(frame, bp_loc, dict):
    global find_in_stack
    names = set([frame.GetFunctionName() for frame in frame.GetThread()])
    all_ignored = set(find_in_stack)
    ignored_here = all_ignored.intersection(names)
    if len(ignored_here) == 0:
        frame.GetThread().GetProcess().Continue()

quit()

_There are a number of semaphore_wait_traps that we want to skip. We are only interested in ones with -addProgressCallback:... in its stack frame_

  1. Lastly, run br comm add -F continue_ignored 2 where 2 is the ID of the breakpoint printed from step 1 (it may be a different number).
  2. Continue. Scroll around a bit, and the breakpoint will be hit.

I noticed that this will also get hit in the simulator occasionally, so this looks to be the cause of jankiness in general.

HTH.

mtitolo commented 10 years ago

Related: http://openradar.appspot.com/radar?id=4998836871233536

lxcid commented 10 years ago

I face similar issue as well.

glenna commented 10 years ago

I'm experiencing this issue as well under crappy network conditions.

cleanandsober commented 9 years ago

I am also facing the same issue. Loading many image in table view with sdwebimage freezes the app. You can check the threading from this link. http://io.putul.me/oQ67RC

yanzhiwei147 commented 9 years ago

I face similar issue as well.What's the release time for resolve this issues(version 4.0.0)? It often happen on iPhone 4 or equiv device.

cleanandsober commented 9 years ago

Hi,

Sorry but I don’t understand the issue you’re experiencing. Could you go through what you need help with?


Best Regards, Team Sober Grid contact@sobergrid.commailto:contact@sobergrid.com

[cid:2B498DF3-5AF6-43D6-94B5-E240B78B11C8]

Share us on Facebookhttps://www.facebook.com/sharer/sharer.php?u=http://goo.gl/NglQWa Share us on Twitterhttps://twitter.com/home?status=http://goo.gl/NglQWa

On Aug 13, 2015, at 11:39 PM, yanzhiwei147 notifications@github.com<mailto:notifications@github.com> wrote:

I face similar issue as well.What's the release time for resolve this issues(version 4.0.0)? It often happen on iPhone 4 or equiv device.

— Reply to this email directly or view it on GitHubhttps://github.com/rs/SDWebImage/issues/803#issuecomment-130951702.

landfound commented 9 years ago

ref http://www.objc.io/issues/2-concurrency/concurrency-apis-and-pitfalls/#starvation barrierQueue block main thread

skrew commented 9 years ago

Same problem here. Having application freezing are a very bad user experience, as many users don't know how to kill / restart application. Any "good practice" to have ?

Edit: putting [SDWebImageDownloader.sharedDownloader setMaxConcurrentDownloads: 1]; fix the problem for me. It's better than nothing ! ;)

bpoplauschi commented 8 years ago

This issue should be fixed now, on the 4.x branch. 4.0. release is coming

rajapandian-dev commented 7 years ago

this issue still there in 4.0.

using https://github.com/onevcat/Kingfisher for time being

dirtyhenry commented 7 years ago

I have the same issue here in 4.0.

The main thread hangs and when I pause the execution in Xcode, this is what I get.

capture d ecran 2017-05-22 20 48 04

(sorry for the screenshot, not sure how I can output this as text)

dirtyhenry commented 7 years ago

@rs I think this issue should be reopened, what do you think?

dreampiggy commented 6 years ago

Firstly, could you please try the lastest version and see if this still represent ?

@dirtyhenry This code seems changed to add a queue dispatch to ensure thread-safe, see : https://github.com/rs/SDWebImage/blob/4.2.3/SDWebImage/SDWebImageDownloader.m#L245-L252

And I want to point it out that GCD is a thread-pool. If the GCD thread pool hit the max thread count(currently is 66 on iOS, 64 + main thread + diapatch thread), then a new queue enter will cause a dead lock. So maybe this is unavoidable if your application contains so much threads. (Note not each queue use a thread, when you call dispatch_get_global_queue, you may share a thread with another queue. Only some specify usage such dispatch_queue_create and keep a thread-local variable may actually use a standalone thread).

dreampiggy commented 6 years ago

@mayqiyue Maybe we should drop that usage of using dispatch_queue, for mutable array, a dispatch_semaphore or other lock is enough and will not cause some queue problem.

skyline75489 commented 6 years ago

@dreampiggy Were it caused by logical problem, using other locks would lead to the same deadlock issue. However I failed to find one. Any suggestions?

mayqiyue commented 6 years ago

@dreampiggy Sorry, I made a mistake. Our issue was not caused by SD.

dreampiggy commented 6 years ago

@skyline75489 Actually, why we must use a dispatch_barrier_sync ? This process just need to remove the URL from a NSMutableArary, isn't ? Use GCD as a lock is a wrong design and I don't know why current code always do like this. (Because the issue I mentioned, GCD is a thead-pool and may reach the limit and cause dead-lock, it's not a silver bullet)

I suggest to use a lock (I use dispatch_semaphore because it not so CPU-consuming like spinLock and have a balance of performance) instead. I create one PR #2184 for this.

dreampiggy commented 6 years ago

2184 Merged. Now we use a lock instead of relay on the queue to ensure thread-safe. This may solve the issue.

I guess the issue is that sometimes the completionBlock on NSOperation return the barrier queue used in downloader, so this cause a deadlock.

The exact execution context for your completion block is not guaranteed but is typically a secondary thread.