Jose Ibanez
Jose Ibanez

Reputation: 3345

Why this Objective-C Block leaks?

I've got a tricky memory management problem with blocks that I'm trying to make sure I understand. I'm working on an app that can play video, but needs to check if the user is actually allowed to play it first. There's several steps to verify, some of which require user interaction, so I've got a chunk of code that looks something like this:

MyVideoPlayer *videoPlayer = [[[MyVideoPlayer alloc] init] autorelease];
[videoPlayer canPlayAsset:(MyVideoAsset *)asset completionBlock:^(BOOL isAssetPlayable) {
    if (isAssetPlayable) {
        [videoPlayer playAsset:asset];
        [self presentModalViewController:videoPlayer animated:YES];
    }
}];

This method can either return instantly, or it could require some user input and some networking calls, hence the block that actually presents the player. I noticed some strange behavior, and I discovered that the video player was being leaked. Here's what I thought was happening:

  1. The videoPlayer is autoreleased.
  2. The videoPlayer is retained by the block.
  3. The block executes, and either presents the videoPlayer or doesn't.
  4. The block is released, and releases the videoPlayer.
  5. The videoPlayer is dealloc'd (immediately, if it wasn't presented, or when the modal view is dismissed).

Instead, what's happening is this:

  1. The videoPlayer is autoreleased.
  2. The videoPlayer is retained by the block.
  3. The block executes, and either presents the videoPlayer or doesn't.
  4. ????

Now, I noticed I was able to get the behavior I expected if I modified the block as follows:

MyVideoPlayer *videoPlayer = [[[MyVideoPlayer alloc] init] autorelease];
[videoPlayer canPlayAsset:(MyVideoAsset *)asset completionBlock:^(BOOL isAssetPlayable) {
    if (isAssetPlayable) {
        [videoPlayer playAsset:asset];
        [self presentModalViewController:videoPlayer animated:YES];
    }
    [videoPlayer autorelease];
}];

But I really don't want to add that without really knowing what I'm doing.

My understanding is that this is not a retain loop since videoPlayer is neither retaining nor copying the block. Is my understanding that the block will be released when it is no longer in scope incorrect? Can someone help me understand the right way to do this?

UPDATE

Just some more information, MyVideoPlayer's implementation of canPlayAsset:completionBlock: (with details removed to protect the innocent) looks a little something like this:

- (void)canPlayAsset:(MyVideoAsset *)asset completionBlock:(void(^)(BOOL isAssetPlayable))completion {
    if (!asset) {
        completion(NO);
        return;
    }
    // user is a singleton object
    if (user.isGuest) {
        if ([user allowedRating:asset.rating]) {
            completion(YES);
        } else {
            // show alert
            completion(NO);
        }
    } else {
        if ([user allowedRating:asset.rating]) {
            completion(YES);
        } else {
            // prompt for pin
        }
    }
}

As you can see, at no point am I retaining this damn block. (Let's ignore the pin part because I'm afraid it will just complicate things further. The problem still manifests itself even when that section of code doesn't execute.) If the block is an autoreleased object, why doesn't it get released when this function finishes executing?

UPDATE 2

OK, I tracked it down. The problem wasn't with this block at all. There was a networking request inside the video player that was being leaked that had a block with a reference to self.

This is the part of the cartoon where we go "I learned a valuable lesson today..."

Upvotes: 3

Views: 1810

Answers (1)

Sam
Sam

Reputation: 27354

Just change the first line to:

__block MyVideoPlayer *videoPlayer = [[[MyVideoPlayer alloc] init] autorelease];

The reason this works is because:

In a reference-counted environment, by default when you reference an Objective-C object within a block, it is retained. This is true even if you simply reference an instance variable of the object. Object variables marked with the __block storage type modifier, however, are not retained.

See Apple's Documentation on Object and Block Variables

But Why Wasn't It Being Released To Begin With?

Simple explanation is that after the completionBlock executes, it is not being released. It is being released later in dealloc, thus all the variables it is retaining are still retained. This makes sense since it is possible to execute a block as many times as you wish until it is released.

I've seen this behavior before where an object that owns a block is referenced in the block and releases the block in dealloc, which prevents the object from ever being released. The solution is to weakly reference the object that owns the block. This way the owning type, like MyVideoPlayer, reaches dealloc, which subsequently releases the block (completionBlock in this example).

An alternative to using the __block keyword to avoid retaining a type is to wrap the object in an NSValue by using the valueWithNonretainedObject: and nonretainedObjectValue methods. For instance:

MyVideoPlayer *videoPlayer = [[[MyVideoPlayer alloc] init] autorelease];
NSValue *videoPlayerRef = [NSValue valueWithNonretainedObject:videoPlayer];
[videoPlayer canPlayAsset:(MyVideoAsset *)asset 
          completionBlock:^(BOOL isAssetPlayable) {
              if (isAssetPlayable) {
                  MyVideoPlayer *tempVideoPlayer = 
                      (MyVideoPlayer*)[videoPlayerRef nonretainedObjectValue];
                  [tempVideoPlayer playAsset:asset];
                  [self presentModalViewController:tempVideoPlayer animated:YES];
              }
}];

Edit / Conversation

This section is to expand further into why the block is not being released. I am going to include what I assume is happening behind MyVideoPlayer. It would be great to see actual code, but this should suffice.

Header file...

typedef void(^MyVideoPlayerCompletionBlock)(BOOL isAssetPlayable);

@interface MyVideoPlayer : NSObject
@property(nonatomic, copy) MyVideoPlayerCompletionBlock completion;
... // other property definitions (or ivars)    

-(void)canPlayAsset:(MyVideoAsset*)asset 
    completionBlock:(MyVideoPlayerCompletionBlock)completion;
... // other methods defined
@end

Implementation...

@implementation MyVideoPlayer

@synthesize completion = _completion;

-(void)dealloc {
    // Note: Block is released in dealloc like any other property or variable
    [_completion release], _completion = nil;
    ...  // other variables and properties not shown here are released
    [super dealloc];
}

-(void)canPlayAsset:(MyVideoAsset*)asset 
    completionBlock:(MyVideoPlayerCompletionBlock)completion {
    ... // Does something with asset.  Plays it, stores it, whatever

    // Saves completion block to call at a later time.
    // Note that this code could alternatively look like
    // _completion = [completion copy];
    // Blocks are usually copied and not retained
    self.completion = completion;
}

... AT SOME POINT THE COMPLETION BLOCK FIRES ... (probably in an event handler or some sort)

-(void)SomeEventHandlerOrFuncThatCallsCompletionInMyVideoPlayer {
    // Time to call completion!
    if (self.completion) {
        self.completion(YES);  // OR no, doesn't matter
        //
        // WHOA! :: self.completion is not released
        //
        //     i.e. self.completion is not nil, and all
        //          variables inside it are still retained
        //          because calling a block doesn't also
        //          release the block.  To do that you would
        //          need to do:     self.completion = nil;
        //          AFTER calling:  self.completion(...);
        //
        //    SO...
        //
        //          Because the block was not released, it is
        //          still retaining variables (such as the current
        //          instance of MyViewPlayer).  Note that the
        //          Block will never be released until dealloc
        //          is called.  :(  So..., if you want the block
        //          to retain the current MyViewPlayer until
        //          execution of the completion block is over, you
        //          will want to call:  self.completion = nil;
        //          to release the block and all associated variables
        //          after calling the completion block.
    }
}

@end

Upvotes: 5

Related Questions