Did the Target-Action design pattern became bad practice under ARC?

Question

For years I've been following a great pattern called Target-Action which goes like this:

An object calls a specified selector on a specified target object when the time comes to call. This is very useful in lots of different cases where you need a simple callback to an arbitrary method.

Here's an example:

- (void)itemLoaded {
    [specifiedReceiver performSelector:specifiedSelector];
}

Under ARC it now turns out that doing something like this all of a sudden became dangerous.

Xcode throws a warning that goes like this:

PerformSelector may cause a leak because its selector is unknown

Of course the selector is unknown since as part of the Target-Action design pattern you can specify whatever selector you want in order to get a call when something interesting happens.

What bugs me most about this warning is that it says there can be a potential memory leak. From my understanding ARC doesn't bend the memory management rules but instead simply automates the insertion of retain/release/autorelease messages at the right locations.

Another thing to note here: -performSelector: does have an id return value. ARC analyzes method signatures to figure out through application of naming conventions if the method returns a +1 retain count object or not. In this case ARC doesn't know if the selector is a -newFooBar factory or simply calling an unsuspicious worker method (which is almost always the case with Target-Action anyways). Actually ARC should have recognized that I don't expect a return value, and therefore forget about any potential +1 retain counted return value. Looking at it from that point of view I can see where ARC is coming from, but still there is too much uncertainty about what this really means in practice.

Does that now mean under ARC something can go wrong which would never happen without ARC? I don't see how this could produce a memory leak. Can someone give examples of situations in which this is dangerous to do, and how exactly a leak is created in that case?

I really googled the hell out of the internet but didn't find any site explaining why.

Answer 1

The problem with performSelector is that ARC doesn't know what the selector which will performed, does. Consider the following:

id anotherObject1 = [someObject performSelector:@selector(copy)];
id anotherObject2 = [someObject performSelector:@selector(giveMeAnotherNonRetainedObject)];

Now, how can ARC know that the first returns an object with a retain count of 1 but the second returns an object which is autoreleased? (I'm just defining a method called giveMeAnotherNonRetainedObject here which returns something autoreleased). If it didn't add in any releases then anotherObject1 would leak here.

Obviously in my example the selectors to be performed are actually known, but imagine that they were chosen at run time. ARC really could not do its job of putting in the right number of retains or releases here because it simply doesn't know what the selector is going to do. You're right that ARC is not bending any rules and it's just adding in the correct memory management calls for you, but that's precisely the thing it can't do here.

You're right that the fact you're ignoring the return value means that it's going to be OK, but in general ARC is just being picky and warning. But I guess that's why it's a warning and not an error.

Edit:

If you're really sure your code is ok, you could just hide the warning like so:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-performSelector-leaks"
[specifiedReceiver performSelector:specifiedSelector];
#pragma clang diagnostic pop

Answer 2

The warning should read like this:

PerformSelector may cause a leak because its selector is unknown. ARC doesn't know if the returned id has a +1 retain count or not, and therefore can't properly manage the memory of the returned object.

Unfortunately, it's just the first sentence.

Now the solution:

If you receive a return value from a -performSelector method, you can't do anything about the warning in code, except ignoring it.

NSArray *linkedNodes = [startNode performSelector:nodesArrayAccessor];

Your best bet is this:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-performSelector-leaks"
NSArray *linkedNodes = [startNode performSelector:nodesArrayAccessor];
#pragma clang diagnostic pop

Same goes for the case in my initial question, where I completely ignore the return value. ARC should be intelligent enough to see that I don't care about the returned id, and therefore the anonymous selector is almost guaranteed not to be a factory, convenience constructor or whatsoever. Unfortunately ARC is not, so the same rule applies. Ignore the warning.

It can also be done for the whole project by setting the -Wno-arc-performSelector-leaks compiler flag under "Other Warning Flags" in project build settings.

Alternatively, you can surpress the warning on a per-file basis when you add that flag under Your Target > "Build Phases" > "Compile Sources" on the right-hand side next to the desired file.

All three solutions are very messy IMHO so I hope someone comes up with a better one.

Answer 3

As described above you get that warning because the compiler does not know where (or if) to put the retain/release of the performSelector: return value.

But note that if you use [someObject performSelector:@selector(selectorName)] it will not generate warnings (at least in Xcode 4.5 with llvm 4.1) because the exact selector is easy to be determined (you set it explicitly) and that's why compiler is able to put the retain/releases in the correct place.

That's why you will get warning only if you pass the selector using SEL pointer because in that case the compiler is unable to determine in all case what to do. So using the following

SEL s = nil;
if(condition1) SEL = @selector(sel1)
else SEL = @selector(sel2)

[self performSelector:s];

will generate warning. But refactoring it to be:

if(condition1) [self performSelector:@selector(sel1)]
else [self performSelector:@selector(sel2)]

will not generate any warnings