iOS 7.0 and ARC: UITableView never deallocated aft

Question

I have a very simple test app with ARC. One of the view controllers contains UITableView. After making row animations (insertRowsAtIndexPaths or deleteRowsAtIndexPaths) UITableView (and all cells) never deallocated. If I use reloadData, it works fine. No problems on iOS 6, only iOS 7.0. Any ideas how to fix this memory leak?

-(void)expand {

    expanded = !expanded;

    NSArray* paths = [NSArray arrayWithObjects:[NSIndexPath indexPathForRow:0 inSection:0], [NSIndexPath indexPathForRow:1 inSection:0],nil];

    if (expanded) {
        //[table_view reloadData];
        [table_view insertRowsAtIndexPaths:paths withRowAnimation:UITableViewRowAnimationMiddle];
    } else {
        //[table_view reloadData];
        [table_view deleteRowsAtIndexPaths:paths withRowAnimation:UITableViewRowAnimationMiddle];
    }
}

-(int)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section {

    return expanded ? 2 : 0;
}

table_view is kind of class TableView (subclass of UITableView):

@implementation TableView

static int totalTableView;

- (id)initWithFrame:(CGRect)frame style:(UITableViewStyle)style
{
    if (self = [super initWithFrame:frame style:style]) {

        totalTableView++;
        NSLog(@"init tableView (%d)", totalTableView);
    }
    return self;
}

-(void)dealloc {

    totalTableView--;
    NSLog(@"dealloc tableView (%d)", totalTableView);
}

@end

Answer 1

Good news! Apple has fixed this bug as of iOS 7.0.3 (released today, Oct 22 2013).

I tested and can no longer reproduce the issue using the sample project @Joachim provided here when running iOS 7.0.3: https://github.com/jschuster/RadarSamples/tree/master/TableViewCellAnimationBug

I also cannot reproduce the issue under iOS 7.0.3 on one of the other apps I am developing, where the bug was causing problems.

It still may be wise to continue shipping any workarounds for a while, until the majority of users on iOS 7 have updated their devices to at least 7.0.3 (which may take a couple weeks). Well, that is assuming your workarounds are safe and tested!

Answer 2

Well, if you dig a little bit deeper (disable ARC, subclass tableview, override retain/release/dealloc methods then put logs/breakpoints on them), you'll find that something bad happens in an animation completion block which possibly causes the leak.
It looks like the tableview receives too many retains from a completion block after cell inserting/deleting on iOS 7, but not on iOS 6 (on iOS 6 UITableView has not yet been use block animations - you can check it too on the stack trace).

So I try to take over the tableview's animation completion block lifecycle from UIView in a dirty way: method swizzling. And this actually solves the problem.
But it does a lot more so I still looking for a more sophisticated solution.

So extend UIView:

@interface UIView (iOS7UITableViewLeak)
+ (void)fixed_animateWithDuration:(NSTimeInterval)duration delay:(NSTimeInterval)delay options:(UIViewAnimationOptions)options animations:(void (^)(void))animations completion:(void (^)(BOOL finished))completion;
+ (void)swizzleStaticSelector:(SEL)selOrig withSelector:(SEL)selNew;
@end

#import <objc/runtime.h>

typedef void (^CompletionBlock)(BOOL finished);

@implementation UIView (iOS7UITableViewLeak)

+ (void)fixed_animateWithDuration:(NSTimeInterval)duration delay:(NSTimeInterval)delay options:(UIViewAnimationOptions)options animations:(void (^)(void))animations completion:(void (^)(BOOL finished))completion {
    __block CompletionBlock completionBlock = [completion copy];
    [UIView fixed_animateWithDuration:duration delay:delay options:options animations:animations completion:^(BOOL finished) {
        if (completionBlock) completionBlock(finished);
        [completionBlock autorelease];
    }];
}

+ (void)swizzleStaticSelector:(SEL)selOrig withSelector:(SEL)selNew {
    Method origMethod = class_getClassMethod([self class], selOrig);
    Method newMethod = class_getClassMethod([self class], selNew);
    method_exchangeImplementations(origMethod, newMethod);
}

@end

As you can see the original completion block is not passed directly to the animateWithDuration: method and it is released correctly from the wrapper block (the lack of this causes leaks in tableviews). I know it looks a little bit strange but it solves the problem.

Now replace the original animation implementation with the new one in your App Delegate's didFinishLaunchingWithOptions: or wherever you want:

[UIView swizzleStaticSelector:@selector(animateWithDuration:delay:options:animations:completion:) withSelector:@selector(fixed_animateWithDuration:delay:options:animations:completion:)];

After that, all of the calls to [UIView animateWithDuration:...] leads to this modified implementation.

Answer 3

I was debugging a memory leak in my application, which turned out to be this same leak, and eventually came to the exact same conclusion as @gabbayabb -- the completion block of the animation used by UITableView never gets freed, and it has a strong reference to the table view, meaning that never gets freed either. Mine happened with a simple [tableView beginUpdates]; [tableView endUpdates]; pair of calls, with nothing in between. I did discover that disabling animations ([UIView setAnimationsEnabled:NO]...[UIView setAnimationsEnabled:YES]) around the calls avoided the leak -- the block in that case is invoked directly by UIView, and it never gets copied to the heap, and therefore never creates a strong reference to the table view in the first place. If you don't really need the animation, that approach should work. If you need the animation though... either wait for Apple to fix it and live with the leak, or attempt to solve or mitigate the leak via swizzling some methods, such as the approach by @gabbayabb above.

That approach works by wrapping the completion block with a very small one, and managing the references to the original completion block manually. I did confirm this works, and the original completion block gets freed up (and releases all of its strong references appropriately). The small wrapper block will still leak until Apple fixes their bug, but that does not retain any other objects so it will be a relatively small leak in comparison. The fact this approach works indicates that the problem is actually in the UIView code rather than the UITableView, but in testing I have not yet found that any of the other calls to this method leak their completion blocks -- it only seems to be the UITableView ones. Also, it appears that the UITableView animation has a bunch of nested animations (one for each section or row maybe), and each one has a reference to the table view. With my more involved fix below, I found we were forcibly disposing of about twelve leaked completion blocks (for a small table) for each call to begin/endUpdates.

A version of @gabbayabb's solution (but for ARC) would be:

#import <objc/runtime.h>

typedef void (^CompletionBlock)(BOOL finished);

@implementation UIView (iOS7UITableViewLeak)

+ (void)load
{
    if ([UIDevice currentDevice].systemVersion.intValue >= 7)
    {
        Method animateMethod = class_getClassMethod(self, @selector(animateWithDuration:delay:options:animations:completion:));
        Method replacement = class_getClassMethod(self, @selector(_leakbugfix_animateWithDuration:delay:options:animations:completion:));
        method_exchangeImplementations(animateMethod, replacement);
    }
}

+ (void)_leakbugfix_animateWithDuration:(NSTimeInterval)duration delay:(NSTimeInterval)delay
                                options:(UIViewAnimationOptions)options
                             animations:(void (^)(void))animations
                             completion:(void (^)(BOOL finished))completion
{
    CompletionBlock realBlock = completion;

    /* If animations are off, the block is never copied to the heap and the leak does not occur, so ignore that case. */
    if (completion != nil && [UIView areAnimationsEnabled])
    {
        /* Copy to ensure we have a handle to a heap block */
        __block CompletionBlock completionBlock = [completion copy];

        CompletionBlock wrapperBlock = ^(BOOL finished)
        {
            /* Call the original block */
            if (completionBlock) completionBlock(finished);
            /* Nil the last reference so the original block gets dealloced */
            completionBlock = nil;
        };

        realBlock = [wrapperBlock copy];
    }

    /* Call the original method (name changed due to swizzle) with the wrapper block (or the original, if no wrap needed) */
    [self _leakbugfix_animateWithDuration:duration delay:delay options:options animations:animations completion:realBlock];
}

@end

This is basically identical to @gabbayabb 's solution, except it is done with ARC in mind, and avoids doing any extra work if the passed-in completion is nil to begin with or if animations are disabled. That should be safe, and while it does not completely solve the leak, it drastically reduces the impact.

If you want to try to eliminate the leak of the wrapper blocks, something like the following should work:

#import <objc/runtime.h>

typedef void (^CompletionBlock)(BOOL finished);

/* Time to wait to ensure the wrapper block is really leaked */
static const NSTimeInterval BlockCheckTime = 10.0;


@interface _IOS7LeakFixCompletionBlockHolder : NSObject
@property (nonatomic, weak) CompletionBlock block;
- (void)processAfterCompletion;
@end

@implementation _IOS7LeakFixCompletionBlockHolder

- (void)processAfterCompletion
{        
    /* If the block reference is nil, it dealloced correctly on its own, so we do nothing.  If it's still here,
     * we assume it was leaked, and needs an extra release.
     */
    if (self.block != nil)
    {
        /* Call an extra autorelease, avoiding ARC's attempts to foil it */
        SEL autoSelector = sel_getUid("autorelease");
        CompletionBlock block = self.block;
        IMP autoImp = [block methodForSelector:autoSelector];
        if (autoImp)
        {
            autoImp(block, autoSelector);
        }
    }
}
@end

@implementation UIView (iOS7UITableViewLeak)

+ (void)load
{
    if ([UIDevice currentDevice].systemVersion.intValue >= 7)
    {
        Method animateMethod = class_getClassMethod(self, @selector(animateWithDuration:delay:options:animations:completion:));
        Method replacement = class_getClassMethod(self, @selector(_leakbugfix_animateWithDuration:delay:options:animations:completion:));
        method_exchangeImplementations(animateMethod, replacement);
    }
}

+ (void)_leakbugfix_animateWithDuration:(NSTimeInterval)duration delay:(NSTimeInterval)delay
                                options:(UIViewAnimationOptions)options
                             animations:(void (^)(void))animations
                             completion:(void (^)(BOOL finished))completion
{
    CompletionBlock realBlock = completion;

    /* If animations are off, the block is never copied to the heap and the leak does not occur, so ignore that case. */
    if (completion != nil && [UIView areAnimationsEnabled])
    {
        /* Copy to ensure we have a handle to a heap block */
        __block CompletionBlock completionBlock = [completion copy];

        /* Create a special object to hold the wrapper block, which we can do a delayed perform on */
        __block _IOS7LeakFixCompletionBlockHolder *holder = [_IOS7LeakFixCompletionBlockHolder new];

        CompletionBlock wrapperBlock = ^(BOOL finished)
        {
            /* Call the original block */
            if (completionBlock) completionBlock(finished);
            /* Nil the last reference so the original block gets dealloced */
            completionBlock = nil;

            /* Fire off a delayed perform to make sure the wrapper block goes away */
            [holder performSelector:@selector(processAfterCompletion) withObject:nil afterDelay:BlockCheckTime];
            /* And release our reference to the holder, so it goes away after the delayed perform */
            holder = nil;
        };

        realBlock = [wrapperBlock copy];
        holder.block = realBlock; // this needs to be a reference to the heap block
    }

    /* Call the original method (name changed due to swizzle) with the wrapper block (or the original, if no wrap needed */
    [self _leakbugfix_animateWithDuration:duration delay:delay options:options animations:animations completion:realBlock];
}

@end

This approach is a little bit more dangerous. It is the same as the previous solution, except it adds a small object which holds a weak reference to the wrapper block, waits 10 seconds after the animation finishes, and if that wrapper block has not been dealloced yet (which it normally should), assumes it is leaked and forces an additional autorelease call on it. The main danger is if that assumption is incorrect, and the completion block somehow really does have a valid reference elsewhere, we could be causing a crash. It seems very unlikely though, since we won't start the timer until after the original completion block has been called (meaning the animation is done), and the completion blocks really should not survive much longer than that (and nothing other than the UIView mechanism should have a reference to it). There is a slight risk, but it seems low, and this does completely get rid of the leak.

With some additional testing, I looked at the UIViewAnimationOptions value for each of the calls. When called by UITableView, the options value is 0x404, and for all of the nested animations it is 0x44. 0x44 is basically UIViewAnimationOptionBeginFromCurrentState| UIViewAnimationOptionOverrideInheritedCurve and seems OK -- I see lots of other animations go through with that same options value and not leak their completion blocks. 0x404 however... also has UIViewAnimationOptionBeginFromCurrentState set, but the 0x400 value is equivalent to (1 << 10), and the documented options only go up to (1 << 9) in the UIView.h header. So UITableView appears to be using an undocumented UIViewAnimationOption, and the handling of that option in UIView causes the completion block (plus the completion block of all nested animations) to be leaked. That leads itself to another possible solution:

#import <objc/runtime.h>

enum {
    UndocumentedUITableViewAnimationOption = 1 << 10
};

@implementation UIView (iOS7UITableViewLeak)

+ (void)load
{
    if ([UIDevice currentDevice].systemVersion.intValue >= 7)
    {
        Method animateMethod = class_getClassMethod(self, @selector(animateWithDuration:delay:options:animations:completion:));
        Method replacement = class_getClassMethod(self, @selector(_leakbugfix_animateWithDuration:delay:options:animations:completion:));
        method_exchangeImplementations(animateMethod, replacement);
    }
}

+ (void)_leakbugfix_animateWithDuration:(NSTimeInterval)duration delay:(NSTimeInterval)delay
                                options:(UIViewAnimationOptions)options
                             animations:(void (^)(void))animations
                             completion:(void (^)(BOOL finished))completion
{
    /*
     * Whatever option this is, UIView leaks the completion block, plus completion blocks in all
     * nested animations. So... we will just remove it and risk the consequences of not having it.
     */
    options &= ~UndocumentedUITableViewAnimationOption;
    [self _leakbugfix_animateWithDuration:duration delay:delay options:options animations:animations completion:completion];
}
@end

This approach simply eliminates the undocumented option bit and forwards on to the real UIView method. And this does seem to work -- the UITableView does go away, meaning the completion block is dealloced, including all nested animation completion blocks. I have no idea what the option does, but in light testing things seem to work OK without it. It's always possible that option value is vitally important in a way that's not immediately obvious, which is the risk with this approach. This fix is also not "safe" in the sense that if Apple fixes their bug, it will take an application update to get the undocumented option restored to table view animations. But it does avoid the leak.

Basically though... let's hope Apple fixes this bug sooner rather than later.

(Small update: Made one edit to explicitly call [wrapperBlock copy] in the first example -- seems like ARC did not do that for us in a Release build and so it crashed, while it worked in a Debug build.)