1

Which of the following code section is correct?

Definition of correct for "me":

  • It should not have a retain cycle and so should not leak.
  • It must guarantee the running of method2 and method3. So MyObject variable in block must never ever be nil.(which may occur in __weak definitions...) (I do not want to check if it is nil to prevent crash. I always want it to be non-nil)

Update: (Additional Info) Instruments tool did show strange leaks until I replace __block with __weak. However, after that I remembered that __weak references may disappear anytime. I have to be sure that it doesn't disappear and leak too. I don't have a timer. This someMethod is called on main thread when it observes a specific NSNotification.

@implementation MyObject...

-(void)someMethod{
    AnotherObject *abc=[[AnotherObject alloc]init];
    __weak MyObject *weakSelf=self;
    abc.onSuccess=^{
         __strong MyObject * strongSelf = weakSelf;
        [strongSelf method2];
        [strongSelf method3];
    }

}

OR

@implementation MyObject...

-(void)someMethod{
    AnotherObject *abc=[[AnotherObject alloc]init];
    __block MyObject *blockSelf=self;
    abc.onSuccess=^{
        [blockSelf method2];
        [blockSelf method3];

        blockSelf=nil;
    }

}

Update 2: Actual Code which always leaks if I don't use __weak:

    __block RButton *_self=self;
    _aimageView.onSuccess=^(void){
        [_self.headerLabel setText:[_self.book title]];
        _self = nil;
    };
frankish
  • 6,738
  • 9
  • 49
  • 100

4 Answers4

1

A block can mention self without causing any leak. So the first thing to do is specify the circumstances. Why do you think your block leaks? Have you checked? There's no point worrying if there is no reason to worry.

If the block's mentioning self does cause a leak, then the steps you can take depend upon the reason why that is happening. But you haven't shown us that. For example, a dispatch timer block that mentions self might cause a retain cycle, but when you are done with the timer you can break the cycle, so then there's no leak.

Thus it is better to understand a particular retain cycle than to program every block defensively when you don't have to.

EDIT (in response to your comment) This code from my book demonstrates various ways of handling memory management in an NSNotification observer situation: click here

matt
  • 515,959
  • 87
  • 875
  • 1,141
  • Hello matt, thank you for your answer. Instruments tool did show strange leaks until I replace __block with __weak. However, after that I remembered that __weak references may disappear anytime. I have to be sure that it doesn't disappear and leak too. I don't have a timer. This `someMethod` is called on main thread when it observes a specific NSNotification. – frankish Jan 25 '14 at 20:02
  • Okay, good info. But you should say that in your question, because there are other ways to break the retain cycle from an NSNotification observer than doing the weak-strong dance. (Though I actually prefer the weak-strong dance in this situation.) – matt Jan 25 '14 at 20:05
  • 1
    This code [from my book](http://shop.oreilly.com/product/0636920032465.do) demonstrates various ways of handling memory management in an NSNotification observer situation: https://github.com/mattneub/Programming-iOS-Book-Examples/blob/master/bk1ch12p350NotificationLeaker/ch12p325NotificationLeaker/FlipsideViewController.m – matt Jan 25 '14 at 20:06
  • Thank you! Great link. However, this is not the situation that I am having. Notification is broadcasted globally and observer should always observe it until the application ends. In your "strong dance" situation, you check for if(_self). I shouldn't check it, it should always already has to be non-nil "in my case". I updated the question with the original code (added to the bottom of the question) – frankish Jan 25 '14 at 20:41
1

I have looked at your actual code in "Update 2", but it's missing a lot of information. For example, is _aimageView an instance variable? local variable? or what? And what causes this onSuccess to be called?

If _aimageView is a local variable then I don't see evidence of a retain cycle.

My first thought is I don't understand why you want to guarantee that the code inside the body of the block runs in this example. It looks like the code inside is just updating some UI elements. Well, if the UI is no longer displaying (as when the current object has no references to it other than perhaps by this block), what's the point of updating the UI elements?

It's important to know what causes onSuccess to be called because different types of callbacks require different memory management architectures. For example, if the block is fired in response to a touch or some kind of event like that, then chances are that the object pointed to by self (which is probably some kind of view or view controller) must be still alive in order that the event occurs. If that's the case, then __weak will do what you want.

Basically, by the naming of these variables, it is reasonable to conclude that _aimageView is probably an image view that is conceptually "owned" by the current object, and the onSuccess is a completion block that is "owned" by the image view. Unless some other object has a strong reference to it, the "owned" object's lifetime is limited to its "owning" object's lifetime. Thus the block will not outlive the image view, which will not outlive the current object.

The only way that what you are afraid of (when the block runs, the object pointed to by self is deallocated) can happen, is if some other object stores a strong reference to _aimageView, or to the block. For the block, it's very unlikely that a "success block" of one object will be stored with other objects. For the image view, it's similarly unlikely if the image view "belongs" to the current object. (It's conceivable that it may be stored by the autorelease pool or something; but the autorelease pool will not call stuff on it except release, so that's not a problem.)

The only exception I can think of is if the image view is kept by a pending network operation or something, which when it is done calls the onSucess block. But if that were the case, I would say it's bad design to have a single object serve as both a view and a network operation. Rather, in that case, one should have a dedicated network operation object, which is a local variable, and set a completion block on it (e.g. store image in image view, set labels, etc.), start the operation, and not need to store the operation. Then the block can refer to self strongly, but there is no retain cycle.

To summarize, the block (and the image view that owns it) should fall into one of two categories, based on who keeps it alive (i.e. who retains it, who maintains a strong reference to it):

  • If self keeps it alive: e.g. A view controller keeps its views and subviews alive. In this case, it is safe to use __weak because the block does not exist outside of the life of self.

  • If someone else keeps it alive: e.g. an alert view -- you just create it and show it; the system maintains it on screen afterwards. In this case, self should not (and does not need to) have a reference to it, and it's okay to reference self strongly in the block; it won't cause a retain cycle.

Try to avoid situations where both self and someone else keep it alive.

newacct
  • 119,665
  • 29
  • 163
  • 224
  • Thank you for this detailed answer. "Self" is a custom UIButton which has an ImageView inserted on it's view. You are right; _aimageView is @property(nonatomic,strong) AutomatedImageView class which is a manually created subclass of UIImageView: It automatically downloads and shows the image as you have guessed. I use onSuccess to perform additional operations. While using Instruments tool's "Retain Cycle" part, the graphics show this: self(A custom UIButton) retains imageview and imageview retains self.(caused by __block) – frankish Jan 29 '14 at 17:38
  • So I don't know why it happens. Because I remove the `self`(button) from it's `superview` and set the button (`self`) to `nil`. The button is deallocated as I log it in `dealloc` method. I also tried setting `onSuccess` to `nil` and `_aimageView` to `nil` manually inside `dealloc` of `self`(button) but it didn't work. As far as I understand, `__weak` is not going to be nulled if self is still alive. I thought it may get nulled randomly even it's owner is still alive. That solves my problem. (btw that would be wonderful to understand if `__block` always creates retain cycle in this situation). – frankish Jan 29 '14 at 17:41
  • @frankish: If the block has a weak reference to the button, and if no other references to the button remain, the button will be deallocated. I still think this makes the most sense here, since all the block is doing is updating UI elements, which is pointless if the button is no longer used by anyone else. Also, Instruments does not have a "Retain Cycle" instrument, so I don't know what you're talking about. – newacct Feb 01 '14 at 10:10
  • Yes, as long as it never makes my __weak reference null while the button is on UI, it is OK for me. I incorrectly typed the title for the "Retain Cycle" section in Instruments. The correct one: I see cycles in Instruments when I select "Leaks" and select "Cycles & Roots" from the tabs located at the center of the tool (which shows "Leaks" as default) (Just right-side of the Snaphots header) – frankish Feb 04 '14 at 17:15
0

As you require to guarantee that method2 and method3 are executed then you have a requirement that the object referenced by self stays alive. To do this you follow the standard pattern: keep a strong reference for as long as you need it. Your second code fragment does that.

Reference cycles are not inherently bad, it is unmanaged ones that are. Here you make a cycle and then break it, so there is no issue.

(Note as you can send a message to nil the first code fragment will not crash if strongSelf is nil - it just won't do anything.)

CRD
  • 52,522
  • 5
  • 70
  • 86
  • Thank you for your answer. So, setting blockSelf to nil like the second code fragment, is ok? You mean that line while saying "and then break it, so there is no issue"? – frankish Jan 25 '14 at 20:00
  • @frankish - Of course, you can always set a variable to `nil` (provided you not losing a reference your program needs of course!). Setting references to `nil` is a standard way to break a cycle. – CRD Jan 25 '14 at 20:07
  • I updated the question and added the real code to the bottom, which always leaks if I use __block. I tried various things but it always leaked until I replaced __block with __weak. – frankish Jan 25 '14 at 20:39
  • @frankish - I can't say what is happening in your case, you need to explore further. If you break the cycle you don't leak due to the cycle. In this code the cycle is broken by the block assigning `nil` to a link in the cycle, which requires the block to be called. If you use a weak reference the block doesn't need to be called as no cycle in created in there first place. Maybe that is your difference? – CRD Jan 25 '14 at 22:15
0
-(void)someMethod{
    AnotherObject *abc=[[AnotherObject alloc]init];
    abc.onSuccess=^{
        [self method2];
        [self method3];
    }
}

This will make. It won't cause a retain cycle.

Mindy
  • 429
  • 4
  • 6