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

Brad Larson picture Brad Larson · Dec 29, 2011 · Viewed 8.1k times · Source

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

Chris Devereux picture Chris Devereux · Dec 29, 2011

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