3

Right now I have a portion of code like this:

__strong MyRequest *this = self;

 MyHTTPRequestOperation *operation = [[MyHTTPRequestOperation alloc]initWithRequest:urlRequest];
 [operation setCompletionBlockWithSuccess:^(AFHTTPRequestOperation *request, id responseObject) {
     [this requestFinished:request];
 }
 failure:^(AFHTTPRequestOperation *request, NSError *error) {
     [this requestFailed:request withError:error];
 }];

I am mainly doing this because some other classes are inheriting from the class where this code is located and implementing their own requestFinished and requestFailed.

If I change the self reference to __weak I start getting some EXC_BAD_ACCESS errors. With a __strong reference everything works fine but I'm afraid about creating a retain cycle. Note that i am using ARC.

Is this code creating a retain cycle that will cause problems? Any simple solution to this? Any different approach that I can follow to let inheriting classes implement their own methods to handle responses?

Matthieu Riegler
  • 31,918
  • 20
  • 95
  • 134
jdrm
  • 621
  • 2
  • 8
  • 21
  • 1
    Where was the crash? What do the finished/failed methods contain? As written, that exact code will not crash with a __weak. – bbum Oct 15 '12 at 18:16
  • i don't understand the need for the `this` variable. why can't you just call `[self requestFinished]` without using that var? if you have subclasses of the `MyRequest` class and override the `requestFinished` then that subclass's custom `requestFinished` will be called anyways – jere Oct 15 '12 at 18:24
  • @jere: You're not supposed to have a strong reference to `self` from within a block (see [this question](http://stackoverflow.com/questions/4352561/retain-cycle-on-self-with-blocks)). – Tim Oct 15 '12 at 18:39
  • @Tim: That's not true at all. It's okay as long as `self` doesn't have a strong reference to the block – newacct Oct 16 '12 at 01:40
  • newacct: very good point, and my previous comment (in that light) is rather misleading. @jere: the danger of having a strong reference to `self` inside a block is the chance you'll create a retain cycle, which can't later be garbage collected; this happens, as newacct pointed out, when `self` also has a strong reference to the block. Since the two reference each other, they each effectively prevent the other from being deallocated. (I think the question I linked explained this, but my earlier summary was pretty unclear - sorry!) – Tim Oct 16 '12 at 01:58
  • so, if i got it right it would be practically the same thing that happens when you have a delegate property set as `retain`, right? – jere Oct 16 '12 at 13:18
  • what about __block ? http://stackoverflow.com/questions/7080927/what-does-the-block-keyword-mean – Epaga Feb 12 '13 at 14:55
  • I don't see this mentioned anywhere else, but `this` and `self` are the same thing. Just because you've assigned it to two different variables doesn't mean it's a different object. If `self` retains your block and your block retains `this`, it's still a circular reference as `this` and `self` are the same object. – Steven Fisher Feb 23 '13 at 21:48

1 Answers1

11

Yes, it creates a retain cycle. Does it cause problems? Maybe.

If the API supports it, you can reset the handlers, which will break the retain cycle manually:

[operation setCompletionBlockWithSuccess:nil failure:nil];

Or you can use a weak reference. However, you say you tried a weak reference, and it crashed. A weak reference is guaranteed to be either nil at the start of a message, or remain valid until the message has been processed. In other words, consider...

__weak MyRequest *weakSelf = self;
dispatch_async(someQ, ^{
    [weakSelf doSomething];
});

If weakSelf is nil when the asynchronous block executes, then "nothing" happens. If it is not nil, then it is guaranteed to be retained at least until doSomething has finished. In effect, it is similar to this:

__weak MyRequest *weakSelf = self;
dispatch_async(someQ, ^{
    { id obj = weakSelf; [weakSelf doSomething]; obj = nil; }
});

Note, however, that if you did this:

__weak MyRequest *weakSelf = self;
dispatch_async(someQ, ^{
    [weakSelf doSomething];
    [weakSelf doSomethingElse];
});

that the object could become nil in-between doSomething and doSomethingElse.

Furthermore, if you access an instance variable via a weak reference, you are just asking for a SEGV:

__weak MyRequest *weakSelf = self;
dispatch_async(someQ, ^{
    foo = weakSelf->someIVar; // This can go BOOM!
});

Thus, if your handler is accessing the weak reference for a single message, then you should be fine. Anything else should do the "Weak-Strong-Dance."

__weak MyRequest *weakSelf = self;
dispatch_async(someQ, ^{
    MyRequest *strongSelf = weakSelf;
    if (!strongSelf) return;
    [strongSelf doSomething];
    [strongSelf doSomethingElse];
    foo = strongSelf->someIVar;
});

If you think you are following the guidelines, then maybe a more complete source code example, with the crash details would help...

Jody Hagins
  • 27,943
  • 6
  • 58
  • 87
  • 5
    where's the cycle? the block retains `self`, `operation` retains the block. But I don't see `self` retaining `operation` anywhere – newacct Oct 16 '12 at 01:37
  • @newacct Right. The cycle is not explicit in the original code alone, but the question only contains a snippet - I usually just ignore questions like this because they lack information and most times I am left to make an assumption - which is almost always a bad idea. Maybe I should have ignored this one as well. Anyway, since it fails with weak, but works with strong, I assume the operation is being held strongly by self - which creates the cycle. My post was to show how to break one, assuming it was there... not prove that one exists in this case. – Jody Hagins Oct 16 '12 at 15:49
  • Thanks for the full explanation and in the future I'll try to include more information abut the surrroundings of the problem. I'll put some additional comments about my specific problem soon. – jdrm Oct 19 '12 at 17:26
  • 3
    Using dispatch_async as the example is misleading as self does not hold a strong reference to it so, there is no threat of a retain cycle. Or am I not understanding correctly? – Remover Oct 20 '12 at 12:00
  • @Remover Peace. I was just trying to show how to use weak for breaking retain cycles, and just used that as a way to show an async call. Retain cycles come in lots of different flavors, and I probably should have used a better example. – Jody Hagins Oct 20 '12 at 14:37
  • Yea, no worries... I found it a little confusing myself. Hence, the post. – Remover Oct 20 '12 at 19:11
  • This post is a little misleading as the previous commenters have said. I could not find any retain cycles in the code. It's only after reading the comments i understood your assumptions.. Maybe state this in the answer it self? – Just a coder Jul 03 '15 at 16:44