Why does the ARC migrator say that NSInvocation's -setArgument: is not safe unless the argument is __unsafe_unretained?

Question

I was migrating a block of code to automatic reference counting (ARC), and had the ARC migrator throw the error

NSInvocation's setArgument is not safe to be used with an object with ownership other than __unsafe_unretained

on code where I had allocated an object using something like

NSDecimalNumber *testNumber1 = [[NSDecimalNumber alloc] initWithString:@"1.0"];

then set it as an NSInvocation argument using

[theInvocation setArgument:&testNumber1 atIndex:2];

Why is it preventing you from doing this? It seems just as bad to use __unsafe_unretained objects as arguments. For example, the following code causes a crash under ARC:

NSDecimalNumber *testNumber1 = [[NSDecimalNumber alloc] initWithString:@"1.0"];
NSMutableArray *testArray = [[NSMutableArray alloc] init];

__unsafe_unretained NSDecimalNumber *tempNumber = testNumber1;

NSLog(@"Array count before invocation: %ld", [testArray count]);
//    [testArray addObject:testNumber1];    
SEL theSelector = @selector(addObject:);
NSMethodSignature *sig = [testArray methodSignatureForSelector:theSelector];
NSInvocation *theInvocation = [NSInvocation invocationWithMethodSignature:sig];
[theInvocation setTarget:testArray];
[theInvocation setSelector:theSelector];
[theInvocation setArgument:&tempNumber atIndex:2];
//        [theInvocation retainArguments];

// Let's say we don't use this invocation until after the original pointer is gone
testNumber1 = nil;

[theInvocation invoke];
theInvocation = nil;

NSLog(@"Array count after invocation: %ld", [testArray count]);
testArray = nil;

due to the overrelease of testNumber1, because the temporary __unsafe_unretained tempNumber variable is not holding on to it after the original pointer is set to nil (simulating a case where the invocation is used after the original reference to an argument has gone away). If the -retainArguments line is uncommented (causing the NSInvocation to hold on to the argument), this code does not crash.

The exact same crash happens if I use testNumber1 directly as an argument to -setArgument:, and it's also fixed if you use -retainArguments. Why, then, does the ARC migrator say that using a strongly held pointer as an argument to NSInvocation's -setArgument: is unsafe, unless you use something that is __unsafe_unretained?

Answer 1

This is a complete guess, but might it be something to do with the argument being passed in by reference as a void*?

In the case you've mentioned, this doesn't really seem a problem, but if you were to call, eg. getArgument:atIndex: then the compiler wouldn't have any way of knowing whether the returned argument needed to be retained.

From NSInvocation.h:

- (void)getArgument:(void *)argumentLocation atIndex:(NSInteger)idx;
- (void)setArgument:(void *)argumentLocation atIndex:(NSInteger)idx;

Given that the compiler doesn't know whether the method will return by reference or not (these two method declarations have identical types and attributes), perhaps the migrator is being (sensibly) cautious and telling you to avoid void pointers to strong pointers?

Eg:

NSDecimalNumber* val;
[anInvocation getArgument:&val atIndex:2];
anInvocation = nil;
NSLog(@"%@", val); // kaboom!


__unsafe_unretained NSDecimalNumber* tempVal;
[anInvocation getArgument:&tempVal atIndex:2];
NSDecimalNumber* val = tempVal;
anInvocation = nil;
NSLog(@"%@", val); // fine

Answer 2

An NSInvocation by default does not retain or copy given arguments for efficiency, so each object passed as argument must still live when the invocation is invoked. That means the pointers passed to -setArgument:atIndex: are handled as __unsafe_unretained.

The two lines of MRR code you posted got away with this: testNumber1 was never released. That would have lead to a memory leak, but would have worked. In ARC though, testNumber1 will be released anywhere between its last use and the end of the block in which it is defined, so it will be deallocated. By migrating to ARC, the code may crash, so the ARC migration tool prevents you from migrating:

NSInvocation's setArgument is not safe to be used with an object with ownership other than __unsafe_unretained

Simply passing the pointer as __unsafe_unretained won't fix the problem, you have to make sure that the argument is still around when the invocation gets called. One way to do this is call -retainArguments as you did (or even better: directly after creating the NSInvocation). Then the invocation retains all its arguments, and so it keeps everything needed for being invoked around. That may be not as efficient, but it's definitely preferable to a crash ;)