10

I have a view controller that downloads an asset in a background GCD queue. I pass my downloading function a callback block to execute once the download is finished, and it always executes this block on the main thread.

The problem occurs if my view controller is dismissed by the user before the download is finished. I suspect what's happening is, once my view controller is dismissed, the callback block is the only thing retaining a strong reference to the controller. The callback block is only retained in a background thread, so once it's released, all the objects captured in the scope of the callback block are also released, albeit in a background queue.

This is the problem: being released in a background queue is causing dealloc to be run in that same queue, not the main queue. This, in turn, calls dealloc in the background and the app crashes:

2012-01-19 12:47:36.349 500px iOS[4892:12107] bool _WebTryThreadLock(bool), 0x306c10: Tried to obtain the web lock from a thread other than the main thread or the web thread. This may be a result of calling to UIKit from a secondary thread. Crashing now...
[Switching to process 16643 thread 0x4103]
[Switching to process 16643 thread 0x4103]
(gdb) where
#0  0x307fd3c8 in _WebTryThreadLock ()
#1  0x307ff1b0 in WebThreadLock ()
#2  0x33f7865e in -[UITextView dealloc] ()
#3  0x0005d2ce in -[GCPlaceholderTextView dealloc] (self=0x309010, _cmd=0x340ce6b8) at /Users/ash/Dropbox/500px/500px-ios/500px iOS/500px iOS/GCPlaceholderTextView.m:113
#4  0x337cac42 in -[NSObject(NSObject) release] ()
#5  0x33dee5f4 in -[UIView(Hierarchy) removeFromSuperview] ()
#6  0x33e836cc in -[UIScrollView removeFromSuperview] ()
#7  0x33f762f0 in -[UITextView removeFromSuperview] ()
#8  0x33e01de2 in -[UIView dealloc] ()
#9  0x337cac42 in -[NSObject(NSObject) release] ()
#10 0x33dee5f4 in -[UIView(Hierarchy) removeFromSuperview] ()
#11 0x33e01de2 in -[UIView dealloc] ()
#12 0x33f437e4 in -[UIScrollView dealloc] ()
#13 0x337cac42 in -[NSObject(NSObject) release] ()
#14 0x33dee5f4 in -[UIView(Hierarchy) removeFromSuperview] ()
#15 0x33e836cc in -[UIScrollView removeFromSuperview] ()
#16 0x33e01de2 in -[UIView dealloc] ()
#17 0x337cac42 in -[NSObject(NSObject) release] ()
#18 0x33dee5f4 in -[UIView(Hierarchy) removeFromSuperview] ()
#19 0x33e01de2 in -[UIView dealloc] ()
#20 0x337cac42 in -[NSObject(NSObject) release] ()
#21 0x33e5a00e in -[UIViewController dealloc] ()
#22 0x00035f16 in -[PXPhotoViewController dealloc] (self=0x5158d0, _cmd=0x340ce6b8) at /Users/ash/Dropbox/500px/500px-ios/500px iOS/500px iOS/PXPhotoViewController.m:118
#23 0x337cac42 in -[NSObject(NSObject) release] ()
#24 0x337e5046 in sendRelease ()
#25 0x331fc92e in _Block_object_dispose ()
#26 0x0003c33a in __destroy_helper_block_ () at /Users/ash/Dropbox/500px/500px-ios/500px iOS/500px iOS/PXPhotoViewController.m:878
#27 0x331fc88e in _Block_release ()
#28 0x331fc91c in _Block_object_dispose ()
#29 0x000c8d32 in __destroy_helper_block_ () at /Users/ash/Dropbox/500px/500px-ios/500px iOS/500px iOS/PXPhotoFetcher.m:557
#30 0x331fc88e in _Block_release ()
#31 0x35eec8ec in _dispatch_call_block_and_release ()
#32 0x35ee2de2 in _dispatch_queue_drain ()
#33 0x35ee2f32 in _dispatch_queue_invoke ()
#34 0x35ee24f2 in _dispatch_worker_thread2 ()
#35 0x34ecb590 in _pthread_wqthread ()
#36 0x34ecbbc4 in start_wqthread ()

The code I execute on the main thread looks like this:

[[PXPhotoFetcher sharedPXPhotoFetcher] fetchPhotoDetailsWithPriority:PhotoRequestLowPriority 
    withCallback:^(PXPhotoModel *thePhotoModel) {
        // a callback function which captures self in its scope
    } forModel:model];

I'm building for 4.3, so if I use an __unsafe_unretained reference to self in the callback block, this will fix my current problem but introduce the new problem of having a dangling pointer.

What do you recommend for architecting a solution for this? Other workarounds included overriding release so that it was always invoked on the main thread, but this obviously isn't going to work in the ARC environment.

This seems like it should be a far more common problem with ARC and GCD, but I can't find anything online. Is it just because I'm targeting < iOS 5 and can't use weak references?

Community
  • 1
  • 1
Ash Furrow
  • 12,391
  • 3
  • 57
  • 92

7 Answers7

5

UPDATE: The below solution didn't, in fact, work on iOS 4. It worked on 5 for some reason, but not 4, so I came up with a better solution.

The problem is caused by the block being destroyed in the background, so I put it into a local variable and, within a background block, invoke it and then pass it to a block on the main thread asynchronously so it's released there. It's also messy, but looks like the following:

void(^block)(void) = ^{/*do all the things*/};

dispatch_async(queue, ^{

    block();

    dispatch_async(dispatch_get_main_queue(), ^{
        if ([block isKindOfClass:[NSString class]])
            NSLog(@"Whoa, bro");
    });
});

The code on the main thread is just a trick to make sure the compiler doesn't just optimize away the code altogether; I need the block object to be released lastly by the main thread. This code appears to work with the -Os compiler optimization level.

So I've come up with a solution to my problem, though it is super hacky. I've been unable to reproduce the problem since this fix, though it is a very poor architectural design I think.

To recap, the problem is I have a block in a background queue that's being destroyed. That block is an object and it owns a strong reference to a callback block which contains a strong reference to self. The background block is being released from the background queue. So what I've done is wrap the call in another dispatch call, to the main queue. So my fetch method goes from this:

dispatch_async(backgroundQueue, ^{
    /* do all the things */
});

To this:

dispatch_async(dispatch_get_main_queue(), ^{
    dispatch_async(backgroundQueue, ^{
        /* do all the things */
    });
});

The code asynchronously dispatches a block to the main queue which then dispatches a block to the background queue. Since the background block is an object and belongs to the main queue's scope, it is released on the main thread, causing the eventual dealloc'ing of the UITextView causing the crash to occur on the main queue, as well, solving my problem.

The obvious architectural solution is to use a __weak reference to self in my callback block, but I'll have to wait until I drop support for iOS 4.3.

Ash Furrow
  • 12,391
  • 3
  • 57
  • 92
  • 1
    If you prefer a weak reference you might check out http://mikeash.com/pyblog/introducing-mazeroingweakref.html – tapi Jan 19 '12 at 23:24
  • It's a little ugly, but that's a clever trick. I wish I had a better answer. – BJ Homer Jan 20 '12 at 15:22
  • Yeah I looked at Mike Ash's solution, but since I'm only supporting iOS 4.3 in the short-term, I'll stick with this hack for a little while. – Ash Furrow Jan 20 '12 at 15:40
3

Actually, GCD allows retain and release for dispatch queues for this purpose. It is actually documented at: "Memory Management for Dispatch Queues".

dispatch_retain(first_queue);
dispatch_async(a_queue, ^{
                            do_not_wait_for_me();
                            dispatch_async(first_queue, ^{ i_am_done_now(); });
                            dispatch_release(first_queue);
                         });

In your scenario I would replace first_queue with your main dispatch queue. By retaining the main dispatch queue, you will ensure that it does not get released until the callback is finished.

Another example can be found at: "Performing a Completion Block When a Task Is Done."

flukey
  • 187
  • 2
  • 8
  • That doesn't solve the problem of the uitextview being *deallocated* on the background thread. – Ash Furrow Apr 10 '12 at 14:08
  • Can you give me some more context related to the UITextView. In both your example and answer I see no reference to the UITextView except for the debug output. Also, in your answer you seem to be using a pattern that makes your block a stack-local data structure. That is a pattern that the ["Blocks Programming Topics: Patterns To Avoid"](http://developer.apple.com/library/ios/#documentation/Cocoa/Conceptual/Blocks/Articles/bxUsing.html#//apple_ref/doc/uid/TP40007502-CH5-SW6) advises against due to scope problems. – flukey Apr 13 '12 at 21:03
2

Ash,

Your problem is an early deallocation as your queues are being torn down. Hence, stop doing that. How? You're almost there. From your background queue, you will want to SYNCHRONOUSLY return the data to the controller on the main thread. That way, when everything unwinds, they deallocate in a safe order. For example:

dispatch_async(backgroundQueue, ^{

    /* download stuff */

    dispatch_sync(dispatch_get_main_queue(), ^{

        // Pass the data to the controller.
    });
});

This pattern lets you guarantee proper delivery of your data to any UI component on the main thread and then allows graceful release.

Andrew

Edit for second answer with __block variable:

__block UIViewController *vc = danglingVC;

dispatch_async(backgroundQueue, ^{

    /* download stuff */

    dispatch_async(dispatch_get_main_queue(), ^{

        // Pass the data to the controller.

        vc = nil;
    });
});

With ARC you force releases by setting the strong storage slot to nil. Note that I can now make the transfer to the main thread asynchronous. I think this is a clear and explicit solution to your problem. It doesn't depend upon subtle interactions between ARC, GCD and blocks.

Andrew

adonoho
  • 4,339
  • 1
  • 18
  • 22
  • 1
    This won't change the order in which things are deallocated, just the order in which the internal block is run. – BJ Homer Jan 20 '12 at 15:21
  • BJ, changing the order in which things are run is the point. The synchronous call keeps the async queue from draining. Hence, the retain on the controller isn't released until after the download has completed and the data delivered to the controller. This dramatically changes the order of deallocation to a safe order. Andrew – adonoho Jan 20 '12 at 15:28
  • @adonoho It's not a premature release on any of my objects, it's a matter of scope. Even if I use your `dispatch_sync` solution, the block object passed on the first line (to the `backgroundQueue`) is what's ultimately retaining my view controller and that block object will always be released on the background thread, even with a synchronous call. – Ash Furrow Jan 20 '12 at 15:44
  • 1
    @adonoho the problem isn't that data is being delivered after the web view has been deallocated. The problem is that uiwebview doesn't like being deallocated on a a background thread, even if no network activity is going on. – BJ Homer Jan 20 '12 at 16:12
  • Ash & BJ, With ARC, you control the explicit lifetime of an item by nil-ling out its storage slot. __block variables provide this capability. I have added this code block above. Andrew – adonoho Jan 21 '12 at 14:06
  • Actually, I don't have a reference to the vc, I have a reference to a GCD block object that has a strong ownership of a VC. This block object is passed in as a parameter, and I'd rather not pass in a __block parameter to a method. – Ash Furrow Jan 21 '12 at 16:17
  • Ash, I respectfully suggest that you reassess your desire to keep the VC "hidden" to your various blocks. It is forcing you to use what you describe as a "hack". Also, you mention above that you cannot use __weak because you want to support iOS v4.3. I'm sure you know this but I'll repeat it anyway. __unsafe_unretained is supported on iOS v4.0+. The only difference from __weak is that it does not auto-nil. I suspect you do not need auto-nil-ling. Andrew – adonoho Jan 21 '12 at 17:32
  • I know how `__unsane_unretained` works and it won't solve my problem since the pointer will still be dangling. Additionally, I'm not "hiding" my VC, I'm passing in a block which happens to have a strong reference to it. Changing from a GCD block callback system would involve a major architectural change and isn't worth the effort since __weak variables will solve the problem when I drop iOS 4 support. – Ash Furrow Feb 01 '12 at 17:37
  • Ash, I somehow offended you. I apologize. Good luck with your problem. Andrew – adonoho Feb 02 '12 at 03:12
  • I wrote that far too aggressively - I apologize. The issue has been fixed with `__weak` references. – Ash Furrow Apr 13 '12 at 22:19
0

An alternative would be to store a reference of the object in a scope outside dispatch async. For example:

// create a dispatch queue
dispatch_queue_t sdq = dispatch_queue_create("com.fred.exampleQueue", NULL);

// my object references container
__block NSMutableArray *ressources = [[NSMutableArray alloc] init];

dispatch_async(sdq, ^{
    __block MyLoader *ml = [[MyAsyncLoader alloc] initWithCallback:^(id result)
    {
        NSLog(@"loader result:  %@", result);
        // since I ask for ressource in my final CallBack it's still here
        // and my loader too, I can now dispose of it.
        [ressources removeObject:ml];
    }];

    // perform async loading and call my callback when it's done...
    [ml asyncLoad];
    // save my object
    [ressources addObject:ml];
});
Fred
  • 484
  • 8
  • 16
0

I came across a similar problem recently. I had the luxury of using zeroing weak references in ARC to solve this particular problem. However I did consider an alternative solution that should work with MRC.

__block UIViewController *vc = ...;
[vc retain];
dispatch_async(backgroundQueue, ^{
    // ... do some things with vc
    dispatch_async(dispatch_get_main_queue(), ^{
        // ... do more things with vc
        [vc release]; // vc might be dealloc'd here but not before
    });
});

The __block storage qualifier on vc ensures that the block does not retain the object referred to by vc. The view controller is retained until the block that gets called on the main queue releases it and possibly gets dealloc'd at that point.

0

Your analysis seems sound. So maybe the end of the block could retain the controller, and do an autorelease on the main thread (probably after a short delay, so as to avoid race conditions).

David Dunham
  • 8,139
  • 3
  • 28
  • 41
  • I'm compiling with ARC, so calls to retain and autorelease are invalid. I could fake it with a dispatch_after call capturing the block's scope, but I think I've found a better solution. – Ash Furrow Jan 19 '12 at 23:01
  • Doh. Though perhaps that would work just dispatching a block that has the reference (I guess this is what you did, but inside-out). – David Dunham Jan 19 '12 at 23:11
0

You can never guarantee that an object will be dealloced on any particular thread, as you have found out.

The best solution I have found is in your dealloc, check to see if you are on the main thread. If not, performSelector on the main thread with the rest of the dealloc and wait for completion.

Alternatively, implement that same logic with blocks: if not on he main thread, dispatch_sync to the main queue with the rest of the dealloc.

Steve Weller
  • 4,599
  • 4
  • 23
  • 30