3

I've encountered an error when running with release configuration, which seems to be the premature release of local variable tmp.

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid index path for use with UITableView. Index paths passed to table view must contain exactly two indices specifying the section and row. Please use the category on NSIndexPath in UITableView.h if possible.'

Related code:

@property (nonatomic, strong) NSIndexPath *selectedCellIndexPath;

...

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
{
    if (_selectedCellIndexPath != nil && [_selectedCellIndexPath isEqual:indexPath]) {        
        self.selectedCellIndexPath = nil;
        [tableView reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationFade];
    } else if (_selectedCellIndexPath != nil && ![_selectedCellIndexPath isEqual:indexPath]) {

//--- problematic code
        NSIndexPath *tmp = _selectedCellIndexPath;
        self.selectedCellIndexPath = indexPath;
        [tableView reloadRowsAtIndexPaths:@[tmp, _selectedCellIndexPath] withRowAnimation:UITableViewRowAnimationFade];
//--- problematic code

    } else {
        self.selectedCellIndexPath = indexPath;
        [tableView reloadRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationFade];
    }
}

I had an impression that local variable tmp should have strong reference here or I am not right?

Btw, changing code to

NSIndexPath *tmp = self.selectedCellIndexPath;

Or changing

@[tmp, _selectedCellIndexPath] to [NSArray arrayWithObjects:tmp,_selectedCellIndexPath,nil] fixes the problem.

What would be the explanation what goes wrong here?

Mindaugas
  • 1,707
  • 13
  • 20
  • I don't really know. Maybe self.selectedCellIndexPath creates a copy? The difference is that self.selectedCellIndexPath invokes the getter method as [self selectedCellIndexPath] would do. Accessing _selectedIndexPath means using or changing the iVar itself without giving ARC a chance to properly manage the memory. In general, without in getters or setters itself you should prefer using the setters/getters and not access the iVar directly – Hermann Klecker Mar 03 '13 at 15:18

2 Answers2

3

tmp is a strong reference; locals are by default in ARC.

Given that your two suggested fixes should by all rights be the same result (assuming you're not overriding the setter or getter for selectedCellIndexPath to do something strange), I would guess you've found an ARC bug. (I've found a couple of these, so it doesn't surprise me that much.)

Try running your code with zombies turned on. If it tells you you're accessing a deallocated object, I would say it's an ARC bug. It would be nice to try and create a simplified test case and submit a bug if that's the case.

Edit:

I think this is an ARC bug. The problem seems to be that objects are stored into a temporary (on-stack) buffer before the call to arrayWithObjects:count: but they're not retained while in that buffer. In your code, those objects lose their last strong reference after being added to the buffer (in -O1 or higher) and so you get an early dealloc. Here's an even simpler test case that causes a crash by causing a dangling reference to the first object in that temporary buffer (since the assignment causes the first object to be released):

NSObject *foo = [[NSObject alloc] init];
@[foo, (foo = [[NSObject alloc] init])];
Community
  • 1
  • 1
Jesse Rusak
  • 56,530
  • 12
  • 101
  • 102
  • As an aside, my guess is that the bug is caused by your code referring to the ivar directly sometimes and via the accessors other times. This shouldn't cause trouble, but is a bit unusual. – Jesse Rusak Mar 03 '13 at 16:03
  • Yes, there is a message sent to deallocated instance. I do have a simple test case (default master/detail project template without storyboard, and just paste in my code. And run it with release configuration). I guess code optimizations for release configuration are messing something up. – Mindaugas Mar 03 '13 at 16:16
  • @Mindaugas Trying that now. Wow, you're not kidding. Just looking through the disassembly; one sec. – Jesse Rusak Mar 03 '13 at 16:37
  • thanks for your investigation, I guess this has something to do with Objective-C literals for array. – Mindaugas Mar 03 '13 at 17:12
  • This bug has been fixed in clang r178721. – Jesse Rusak Apr 20 '13 at 20:53
-1

Have you done Run->Analyze on your code? Clang does a very good job of pointing out potential ARC issues (with explanations of why there is a problem). Treat Analyze warnings as ERRORS.

Also, your 'tmp' var should probably be an iVar if you dont want ARC to release it. This might not always be the most elegant solution, but it tells ARC, 'as long as this class instance exists, hold on to this value.'

You are accessing the _selectedIndexPath through an iVar reference, avoiding the getters and setters. This means you are skipping over ARC-managed calls to retain, release, and autorelease.

So you should probably use 'self'.

Basically, you cant always predict what ARC will do. It will attempt to cleanup at the end of every method. So sometimes you need to give it hints that something will be retained by creating a property. Sometimes i miss calling alloc/release myself.... sometimes lol

G. Shearer
  • 2,175
  • 17
  • 19
  • 1
    local variables by default have `__strong` annotation, so i shouldn't need to hold onto them – Mindaugas Mar 03 '13 at 16:19
  • And Run->Analyze give no issues – Mindaugas Mar 03 '13 at 16:22
  • Not necessarily true though. This problem shows up with popover code sometimes. The fix is to store the popover in a iVar and set it to nil when you are done. That problem is the source of my suggestion – G. Shearer Mar 03 '13 at 16:31
  • Two other potential problems. The array literal is an autoreleased object. And you are declaring it within the closure of an if-block. ARC is most likely going to see this and think it should be immediately released. Declaring the pointer for tmp outside of the if block may help ARC realize it should retain the value you assign in the closure. – G. Shearer Mar 03 '13 at 17:21