4

I'm trying to understand why this code is leaking, using ARC:

- (IBAction)block2:(id)sender {
    NSMutableString *aString = [[NSMutableString alloc] init];

    void (^aBlock)() = ^{
        NSMutableString __unused *anotherString = aString;
    };

    NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:aBlock forKey:@"Key"];

}

As you can see, I put a block inside a collection (NSMutableDictionary, but it's the same if I use NSDictionary, NSArray ecc...), then the method returns and the dictionary is deallocated. The block should then be released. But, using instruments, I see a leak

leaks

leaks2

leaks3

"just to be sure" that the block has no other references, I added this line at the end of the method:

[dict setObject:[NSNull null] forKey:@"Key"];

same result.

I've found this post but the answers point to another problem: Blocks inside NSMutableArray leaking (ARC)

Then, this is the magic: If I change this line:

NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:aBlock forKey:@"Key"];

to:

NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:[aBlock copy] forKey:@"Key"];

the leak disappear.

I know that, under non-ARC, before passing a reference of a block literal, I must copy it (when declared literal, it's on the stack, so I need to copy it to the heap before passing outside the scope of the function where is declared)...but using ARC I shouldn't care about it. Any indication? This is happening with all versions from 5.0 to 6.1.

EDIT: I've made some tests, trying to understand if I'm doing something wrong or if there is some bug...

First: Am I reading wrong instruments informations? I don't think, the leak is real and not my mistake. Look at this image...after executing the method 20 times:

instruments leak

Second: what happens if I try to do the same thing in a non arc environment? this adds some strange behavior:

same function in NON-ARC environment:

- (IBAction)block2:(id)sender {
    NSMutableString *aString = [[NSMutableString alloc] init];

    void (^aBlock)() = ^{
        NSMutableString __unused *anotherString = aString;
    };

    [aString release];

    NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:[[aBlock copy] autorelease] forKey:@"Key"];
}

With the previous non-arc implementation, I have a leak only for the block (not for the string) Changing the implementation to use an autorelease on the mutable string declaring solves the leak!!! I can't understand why, and I'm not sure if it could be related to the main post issue

// version without leak
- (IBAction)block2:(id)sender {
    NSMutableString *aString = [[[NSMutableString alloc] init] autorelease];

    void (^aBlock)() = ^{
        NSMutableString __unused *anotherString = aString;
    };

    NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:[[aBlock copy] autorelease] forKey:@"Key"];
}

CONCLUSIONS:

After various answers and further investigating, I understood some things:

1- Apple docs says that you must use [^{} copy] when you pass a block to a collection. This is because ARC doesn't add the copy itself. If you don't, the collection (array, dictionary..) sends a retain on a STACK ALLOCATED OBJECT - which does nothing. When the method ends, the block goes out of scope and becomes invalid. You will probably receive a bad access when using it. But note: this is not my case, I'm experiencing a different problem

2- the problem I'm experiencing is different: the block is over-retained (the opposite problem --> the block is still alive even when it shoulnd't be). Why? I've found this: in my example, I'm using this code

void (^aBlock)() = ^{
    NSMutableString __unused *anotherString = aString;
};

this code, under NON-ARC, stores a reference (aBlock) to the literal block. The block is allocated on the stack, so if you NSLog(@"%p", aBlock) -> you will see a stack memory address

But, this is the "strange" (I don't find any indication in Apple docs), if you use the same code under ARC and NSLog aBlock address, you will see that now it's on the HEAP! For this reason the behavior is different (no bad access)

So, both incorrect but different behavior:

// this causes a leak
- (IBAction)block2:(id)sender {
    NSMutableString *aString = [[NSMutableString alloc] init];

    void (^aBlock)() = ^{
        NSMutableString __unused *anotherString = aString;
    };

    NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithObject:aBlock forKey:@"Key"];

}

// this would cause a bad access trying to retrieve the block from the returned dictionary
- (NSMutableDictionary *)block2:(id)sender {
    NSMutableString *aString = [[NSMutableString alloc] init];

    return [NSMutableDictionary dictionaryWithObject:^{
        NSMutableString __unused *anotherString = aString;
    } forKey:@"Key"];

}

3 - about my last test under NON-ARC, I think that the release is in the wrong place. I released the string before adding the block to the dictionary with copy-autorelease. The block automatically retains the variables referenced inside the block, but the retain message is sent at the copy moment, not at the declaration. So, If I release aString before copying the block, it's retain count goes to 0, then the block sends a retain message to the "zombie" object (with unexpected behavior, it can leak the block, crash, ecc ecc)

Community
  • 1
  • 1
LombaX
  • 17,265
  • 5
  • 52
  • 77

2 Answers2

3

See this question for reference iOS 5 Blocks ARC bridged cast; it demonstrates the nightmare that are Blocks and ARC.

Typically, if you assign a block to a variable that lives beyond your current scope, the compiler will be automatically able to copy the block to the heap. This means when you fall out of scope, you still have the block hanging around. Similarly, the same goes with block paramaters. The compiler is aware that it'll need to make a copy of those parameters, and hence does so.

The issue with classes such as NSArray is that they don't usually need to copy an object to keep it correctly; typically they only retain the object. Whereas an object going out of scope is part of the language (hence it copies), keeping it within an object like NSArray is an application level operation. As such, the compiler isn't clever enough yet to determine that the block needs copying (Blocks are standard Obj-C objects after all, it thinks all it needs to do is retain it). In a similar vain, thats why any properties that hold blocks need to specify the copy keyword. The automatic synthesis of the property methods aren't aware a block is being stored, and need to be given a nudge to copy them when being set.

This demonstrates why the whole thing works when you use - copy on your block, you're doing what the compiler should be doing, but is not clever enough to do so...Apple even recommends this technique within its Transitioning to ARC documentation, see the FAQs.

Bootnote: In case you're wondering why I'm on about retaining, even when you're using ARC, is that this is what ARC does under the hood. The memory management model is still the same as before, but the onus is now on the system to manage it for us based on naming and conventions, whereas previously the onus was on the developer to manage their memory correctly. It's just that for blocks, the system isn't able to manage it as fully as it should, and hence the developer needs to step in from time to time.

Community
  • 1
  • 1
WDUK
  • 18,870
  • 3
  • 64
  • 72
  • I've made the same assumption, but finally I don't think that the problem is this: NSArray (for example) sends a retain message to the objects inside the collection. A retain message for an object on the stack doesn't increase the retain count, so when the object (the block) goes out of scope, it is destroyed. This is the reason why you should use copy (adding a block to array) in non-arc --> if you don't, you'll have a pointer to an object on the stack (can go out of scope). In thi case I have the opposite problem, a leak, an "over retained" object. Is there something wrong in my assumption? – LombaX Feb 05 '13 at 10:24
  • Is it leaking though; where in your images does it suggest its leaking? The block has a retain count of 0 at the end, that's not a leak, or am I missing something? – WDUK Feb 05 '13 at 10:34
  • This is another thing I don't understand. The refct of the block is 0, but instruments shows that the object is alive --> as you can see, even the mutablestring is retained by the alive block (in this case, the retain count of the string is 1) – LombaX Feb 05 '13 at 10:47
  • Do you mean persistent count? That's not a straight indication that there is a leak. The system may cache some things on your behalf, and you'll need to run the application through a few loops to see if they are accumulating. If they are, then it's a leak (see http://developer.apple.com/library/mac/#documentation/developertools/Conceptual/InstrumentsUserGuide/MemoryManagementforYouriOSApp/MemoryManagementforYouriOSApp.html for some information). Whilst I can't see any leaks in your example, there is a specific `Leaks` instrument that will flag any leaks very clearly, so you could try that. – WDUK Feb 05 '13 at 11:07
  • You also have to think, what is over-retaining the object to cause a leak? I don't see anything at all, and believe it's a misinterpretation of the information Instruments is providing. – WDUK Feb 05 '13 at 11:08
  • The objects appears even using the leak instrument, didn't mention in the main post. I update it – LombaX Feb 05 '13 at 11:09
  • I encourage people not to use ARC before they write several applications with MRC. Blocks and ARC are really hard to understand. – Sulthan Feb 05 '13 at 11:28
  • @Sulthan, I've wrote several application without ARC, however I can't figure why this is happening. I updated my post with more details and a test with MRC, and a similar strange behaviour is happening in some cases, look at the original post – LombaX Feb 05 '13 at 11:32
  • With the leaks instrument, are the leaks fixed when you copy the block to the array? If so, then that's great, because that's how blocks should be used with Arrays. I think we're diving too deep into the issue here; it could be a variety of issues causing this (use of `__unused`, no use of `__block` on the string etc...). – WDUK Feb 05 '13 at 11:38
  • I am pretty sure the answer is valid and should be accepted. The MRC case as trivial because the leak is immediately visible. – Sulthan Feb 05 '13 at 11:42
  • @Sulthan in the MRC case I've made a cut-paste mistake. The last line has a copy-autorelease, not only a copy. The code leaks even in this case. I'll look at both answers after the lunch, I don't know which is the correct one. In this moment, Tammo Freese has given me a documentation reference which says "you must copy"...and leaves no space to interpretation – LombaX Feb 05 '13 at 12:12
  • Sorry but I saw too late the additions you made to your answer and marked the other as correct. They are both correct... :-( – LombaX Feb 05 '13 at 12:32
2

Blocks begin their life on the stack for performance reasons. If they should live longer than the stack is around, they have to be copied to the heap.

In MRR, you had to do that copying yourself. ARC is doing that automatically for you if you pass a block up the stack (i.e. return it from a method). But if pass a block down the stack (for example, store it in an NSMutableDictionary or NSMutableArray), you have to copy it yourself.

This is documented in Apple's Transitioning to ARC documentation, search for "How do blocks work in ARC" inside that document.

For your Non-ARC examples (as you wrote in your conclusion), the copy of the block should happen before releasing aString, as aString is retained when the block is copied. Otherwise your code will show undefined behavior, it may even crash. Here is some code that demonstrates the problem with Non-ARC:

NSObject *object = [[NSObject alloc] init];
void (^aBlock)() = ^{
    NSLog(@"%@", object);
};
[object release];
aBlock(); // undefined behavior. Crashes on my iPhone.
Tammo Freese
  • 10,514
  • 2
  • 35
  • 44
  • Oh...this is embarassing...I've opened that document a lot of times and never looked at the FAQ part :-( – LombaX Feb 05 '13 at 11:54
  • ;) That can happen. As for your non-ARC-example, I'll add answers to those now! – Tammo Freese Feb 05 '13 at 11:59
  • Thanks, I have another question if you know... the docs says "you must do", but doesn't go into the details (which are what I like to know :-) ... if ARC doesn't add the copy by itself (adding the block to the array), shouldn't I have a different behavior? I'll have a retain sent to a stack object --> the object is not really retained nor copied...so it should be "popped" out the stack when the method returns. The reference in the array should be invalid and no leaks should happen... – LombaX Feb 05 '13 at 12:04
  • Whoops, sorry, about the non-arc part, I made a mistake with cut-paste in the answer. I corrected the code, the last line has a copy-autorelease, not only a copy, and it's still leaking. Try the code now – LombaX Feb 05 '13 at 12:07
  • The automatic copy that happens here is undocumented behavior. I would not rely on it – the leak you are seeing is a good indication that it should not be relied upon. – Tammo Freese Feb 05 '13 at 12:10
  • Have to go now, I'll recheck later if I can reproduce the leak. – Tammo Freese Feb 05 '13 at 12:18
  • Thanks. All is becoming to make sense, I think that the non-arc problem is not related to the original one, but if you can have a look I'll be thankful. This is the correct answer for me – LombaX Feb 05 '13 at 12:22