performSelector may cause a leak because its selector is unknown
Asked Answered
H

19

1305

I'm getting the following warning by the ARC compiler:

"performSelector may cause a leak because its selector is unknown".

Here's what I'm doing:

[_controller performSelector:NSSelectorFromString(@"someMethod")];

Why do I get this warning? I understand the compiler can't check if the selector exists or not, but why would that cause a leak? And how can I change my code so that I don't get this warning anymore?

Hodeida answered 10/8, 2011 at 20:23 Comment(14)
It MAY cause a leak. To avoid the warning, you should pass/store selectors as strings except for the moment you are assigning it as an action. If the line above is where you assign it as an action, I am also wondering why not just use @selector(someMethod:) ??Molliemollify
The name of the variable is dynamic, it depends on a lot of other things. There's the risk that I call something that doesn't exist, but that's not the problem.Hodeida
That... is bad practice. Selector names shouldn't be dynamic as the method they are tied to can do whatever you want. Hence, the risk that you could call something that doesn't exist is what the warning is about.Molliemollify
@matt what do you mean by storing "except for the moment you are assigning it as an action". I'm storing the value as a string because it comes from configuration, and changes during execution of the app, so using @selector(method:) is not possible.Hodeida
@matt why would calling a method dynamically on an object be bad practice? Isn't the whole purpose of NSSelectorFromString() to support this practice?Hodeida
Hmm I guess you're right. Does this clear the warning? SEL mySelector = NSSelectorFromString(@"someMethod"); if (mySelector != nil) { [_controller performSelector:mySelector]; }Molliemollify
You should/could also test [_controller respondsToSelector:mySelector] before setting it via performSelector:Molliemollify
Thanks Matt. Good point on the respondsToSelector. Just tried, and it doesn't get rid of the warning, unfortunately. :)Hodeida
@Molliemollify Wish I could vote down: "That... is bad practice."Gaelic
If you know the string is a literal, just use @selector() so the compiler can tell what the selector name is. If your actual code is calling NSSelectorFromString() with a string that’s constructed or provided at runtime, then you must use NSSelectorFromString().Pennywise
Curiously, you don't get this warning for all flavors of performSelector. The versions defined off of class NSObject vs protocol NSObject don't appear to provoke this message.Shote
@Gaelic naysayers everywhere...Datary
@morkrom yes, and many have tried to transform the significantly dynamic Objective-C language into a rigidly static one for years. Glad they have finally moved onto a different language.Gaelic
If you want to use a private API which is, of course, disapproved by Apple, you can use perform selector. Also, to avoid your selector string to be scanned and identified, you should divide the string into multiple parts and concatenate them before passing it to the NSSelectorFromString.Poniard
P
1247

Solution

The compiler is warning about this for a reason. It's very rare that this warning should simply be ignored, and it's easy to work around. Here's how:

if (!_controller) { return; }
SEL selector = NSSelectorFromString(@"someMethod");
IMP imp = [_controller methodForSelector:selector];
void (*func)(id, SEL) = (void *)imp;
func(_controller, selector);

Or more tersely (though hard to read & without the guard):

SEL selector = NSSelectorFromString(@"someMethod");
((void (*)(id, SEL))[_controller methodForSelector:selector])(_controller, selector);

Explanation

What's going on here is you're asking the controller for the C function pointer for the method corresponding to the controller. All NSObjects respond to methodForSelector:, but you can also use class_getMethodImplementation in the Objective-C runtime (useful if you only have a protocol reference, like id<SomeProto>). These function pointers are called IMPs, and are simple typedefed function pointers (id (*IMP)(id, SEL, ...))1. This may be close to the actual method signature of the method, but will not always match exactly.

Once you have the IMP, you need to cast it to a function pointer that includes all of the details that ARC needs (including the two implicit hidden arguments self and _cmd of every Objective-C method call). This is handled in the third line (the (void *) on the right hand side simply tells the compiler that you know what you're doing and not to generate a warning since the pointer types don't match).

Finally, you call the function pointer2.

Complex Example

When the selector takes arguments or returns a value, you'll have to change things a bit:

SEL selector = NSSelectorFromString(@"processRegion:ofView:");
IMP imp = [_controller methodForSelector:selector];
CGRect (*func)(id, SEL, CGRect, UIView *) = (void *)imp;
CGRect result = _controller ?
  func(_controller, selector, someRect, someView) : CGRectZero;

Reasoning for Warning

The reason for this warning is that with ARC, the runtime needs to know what to do with the result of the method you're calling. The result could be anything: void, int, char, NSString *, id, etc. ARC normally gets this information from the header of the object type you're working with.3

There are really only 4 things that ARC would consider for the return value:4

  1. Ignore non-object types (void, int, etc)
  2. Retain object value, then release when it is no longer used (standard assumption)
  3. Release new object values when no longer used (methods in the init/ copy family or attributed with ns_returns_retained)
  4. Do nothing & assume returned object value will be valid in local scope (until inner most release pool is drained, attributed with ns_returns_autoreleased)

The call to methodForSelector: assumes that the return value of the method it's calling is an object, but does not retain/release it. So you could end up creating a leak if your object is supposed to be released as in #3 above (that is, the method you're calling returns a new object).

For selectors you're trying to call that return void or other non-objects, you could enable compiler features to ignore the warning, but it may be dangerous. I've seen Clang go through a few iterations of how it handles return values that aren't assigned to local variables. There's no reason that with ARC enabled that it can't retain and release the object value that's returned from methodForSelector: even though you don't want to use it. From the compiler's perspective, it is an object after all. That means that if the method you're calling, someMethod, is returning a non object (including void), you could end up with a garbage pointer value being retained/released and crash.

Additional Arguments

One consideration is that this is the same warning will occur with performSelector:withObject: and you could run into similar problems with not declaring how that method consumes parameters. ARC allows for declaring consumed parameters, and if the method consumes the parameter, you'll probably eventually send a message to a zombie and crash. There are ways to work around this with bridged casting, but really it'd be better to simply use the IMP and function pointer methodology above. Since consumed parameters are rarely an issue, this isn't likely to come up.

Static Selectors

Interestingly, the compiler will not complain about selectors declared statically:

[_controller performSelector:@selector(someMethod)];

The reason for this is because the compiler actually is able to record all of the information about the selector and the object during compilation. It doesn't need to make any assumptions about anything. (I checked this a year a so ago by looking at the source, but don't have a reference right now.)

Suppression

In trying to think of a situation where suppression of this warning would be necessary and good code design, I'm coming up blank. Someone please share if they have had an experience where silencing this warning was necessary (and the above doesn't handle things properly).

More

It's possible to build up an NSMethodInvocation to handle this as well, but doing so requires a lot more typing and is also slower, so there's little reason to do it.

History

When the performSelector: family of methods was first added to Objective-C, ARC did not exist. While creating ARC, Apple decided that a warning should be generated for these methods as a way of guiding developers toward using other means to explicitly define how memory should be handled when sending arbitrary messages via a named selector. In Objective-C, developers are able to do this by using C style casts on raw function pointers.

With the introduction of Swift, Apple has documented the performSelector: family of methods as "inherently unsafe" and they are not available to Swift.

Over time, we have seen this progression:

  1. Early versions of Objective-C allow performSelector: (manual memory management)
  2. Objective-C with ARC warns for use of performSelector:
  3. Swift does not have access to performSelector: and documents these methods as "inherently unsafe"

The idea of sending messages based on a named selector is not, however, an "inherently unsafe" feature. This idea has been used successfully for a long time in Objective-C as well as many other programming languages.


1 All Objective-C methods have two hidden arguments, self and _cmd that are implicitly added when you call a method.

2 Calling a NULL function is not safe in C. The guard used to check for the presence of the controller ensures that we have an object. We therefore know we'll get an IMP from methodForSelector: (though it may be _objc_msgForward, entry into the message forwarding system). Basically, with the guard in place, we know we have a function to call.

3 Actually, it's possible for it to get the wrong info if declare you objects as id and you're not importing all headers. You could end up with crashes in code that the compiler thinks is fine. This is very rare, but could happen. Usually you'll just get a warning that it doesn't know which of two method signatures to choose from.

4 See the ARC reference on retained return values and unretained return values for more details.

Poyssick answered 18/11, 2013 at 21:44 Comment(31)
@Poyssick If your code solves the retaining problem, I wonder why performSelector: methods aren’t implemented this way. They have strict method signature (returning id, taking one or two ids), so no primitive types need to be handled.Thirteen
@Poyssick With MRC you didn’t know that either. Whether the (dynamic) selector returns retained object or even consumes the argument. I use selectors only for target+action pattern and I never pass there copy or alloc, so I silence that warning globally rather than doing pragma push/pop.Thirteen
@iMartin the short of it is, turning the warning off globally requires all team members to understand (and remember) that you're limited to using this method only for methods that return objects that need not be released. I believe this warning to be helpful because it does this for you, and I leave it enabled.Poyssick
@Poyssick But if you pass variable SEL to performSelector you don’t know what semantics to use manually. What I’m trying to say is, that this is not related to ARC, but MRC as well. It seems like this was always an issue (since NeXT), but people just ignored it? Or used it correctly by convention?Thirteen
So can I call performSelector for dynamic selector with single argument without return value expected?Ribband
@Andy due to the way the compiler handles void returns, this doesn't result in any issues right now, but the future-safe option with ARC is for the return type to match.Poyssick
@Poyssick but what about argument, in my case it's always object. I am just afraid the argument will leak somehow. :-) Is this issue related to arguments or only to return value? Right now I use NSInvocation which is safest way I assume.Ribband
@Andy the argument is handled based on the definition of the method's prototype (it won't be retained/released). The concern is mostly based on the return type.Poyssick
The "Complex Example" gives an error Cannot initialize a variable of type 'CGRect (*)(__strong id, SEL, CGRect, UIView *__strong)' with an rvalue of type 'void *' when using latest Xcode. (5.1.1) Still, I learned a lot!Sidetrack
@StanJames it still works for me in a new project in Xcode. Perhaps you have extra warning flags enabled?Poyssick
void (*func)(id, SEL) = (void *)imp; does not compile, I have replaced it with void (*func)(id, SEL) = (void (*)(id, SEL))imp;Ative
@davyd in Xcode 5.1.1 with the default configuration, void (*func)(id, SEL) = (void *)imp; does compile. Perhaps you have extra compile time flags enabled?Poyssick
@Poyssick I created a new OS X Cocoa Application project in 5.1.1, SDK 10.9, 64-bit target, so the settings should be default. Never mind, it works with typecasting, I hope it does not cause any problems. Thanks a lot for the article.Ative
change void (*func)(id, SEL) = (void *)imp; to <…> = (void (*))imp; or <…> = (void (*) (id, SEL))imp;Stagner
Writing the provide code is suppression. It may be easier to read the code if you just add the #pragma - if that's the entire need.Shawannashawl
Amazing answer. In practice, using a typedef could increase legibility a lot. In my case I put typedef void(*progress_cb)(id, SEL, NSNumber*); in the header, and then use progress_cb func = (progress_cb)[target methodForSelector:selector];Stomatic
The solution produces EXC_BAD_ACCESS crashes on arm64 if func returns id. Apple docs say the cast is required: developer.apple.com/library/ios/documentation/General/… may only be edited for 5 minutes×Comments may only be edited for 5 minutes×Comments may only be edited for 5 minutesMarkettamarkey
@IvanM the complex example shows how to properly handle return types.Poyssick
@Poyssick thanks for this complete answer. Just a question: is this solution (complex example code) safe to use for App Store? is there any risk to get my app rejected due to using this code?Instate
Since you are only suppressing warnings for the call, I don't see any difference except for the greater complexity of the IMP related code. Given two blocks of code that work the same, prefer the simpler code... If the code ever works, it will always work.Hyperphysical
This answer is not very good because it is just silencing the warning with strange code. Look to the answer by "SwiftArchitect" to see many reasonable ways of doing this same thing.Ellison
The one justification I can think of for suppressing a warning message, and the one case where I will do it, is if it is coming from a 3rd party library.Plebeian
and how do you call a selector with multiple arguments through this and be safe in the same time?Dress
Is this still the best method, in xCode 8 building for iOS 10? I believe I am getting errors when using the static method by the way (which I think is different from what you said above - did this change?) What is the best way to proceed in xCode 8 and iOS 10?Kurman
Excellent answer, and explains why I'm getting a crash. Just happens to occur when the invoked method has a (void) return type. Changing the signature to (id) and returning nil removes the crash. … I'm guessing your retain explanation is happening.Tichonn
In my case, I'm not sure if I have a void return method or an id one. But you can check this with: [self methodSignatureForSelector:selector].methodReturnLength > 0.Tichonn
This is clearly the best answer as to why the warning exists. However using the code of this solution hides the problem from the compiler without solving it. If you know the return type of all selectors you may be given you can use that with this code safely, or you can use the answer given by Scott Thomson. If you want to verify your assumption at runtime, see the answer by Chris Prince that extracts the result type of the selector.Vivle
The explanation here is completely mistaken. The only problem is whether the selector is a create method (per cocoa naming convention, e.g. having name prefix make... or create...), therefore whether an object should be returned with +1 retain count. If there is no return value there cannot be any problem happening. The common solution is to silence the warning.Janus
What is _controller here?Etrem
Linking a similar unsolved issue for Xcode12.5 with MagicalRecord (open source CoreData lib): stackoverflow.com/q/67812022/12413715Freakish
In Reasoning for Warning section, I think "The call to methodForSelector: assumes that" should be "The call to performSelector: assumes that". Please let me know if I misunderstood what you said. @PoyssickExclude
V
1189

In the LLVM 3.0 compiler in Xcode 4.2 you can suppress the warning as follows:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-performSelector-leaks"
    [self.ticketTarget performSelector: self.ticketAction withObject: self];
#pragma clang diagnostic pop

If you're getting the error in several places, and want to use the C macro system to hide the pragmas, you can define a macro to make it easier to suppress the warning:

#define SuppressPerformSelectorLeakWarning(Stuff) \
    do { \
        _Pragma("clang diagnostic push") \
        _Pragma("clang diagnostic ignored \"-Warc-performSelector-leaks\"") \
        Stuff; \
        _Pragma("clang diagnostic pop") \
    } while (0)

You can use the macro like this:

SuppressPerformSelectorLeakWarning(
    [_target performSelector:_action withObject:self]
);

If you need the result of the performed message, you can do this:

id result;
SuppressPerformSelectorLeakWarning(
    result = [_target performSelector:_action withObject:self]
);
Vatican answered 28/10, 2011 at 19:30 Comment(11)
This method can cause memory leaks when the optimization is set to anything other than None.Chinachinaberry
@Chinachinaberry No it cannot, unless you're invoking funny methods like "initSomething" or "newSomething" or "somethingCopy".Klagenfurt
@Andrey, in our experience it did at the time. Apple may have made some fixes to Xcode.Chinachinaberry
I figured out that it is enough to use #pragma clang diagnostic ignored "-Warc-performSelector-leaks" at the beginning of the class implementation.Supervisor
@Julian That does work, but that turns off the warning for the entire file – you might not need or want that. Wrappping it with the pop and push-pragmas are much cleaner and more secure.Jurdi
All this does is it silences up the compiler. This does not solve the problem. If the selector doesn't exist you're pretty much screwed.Haughay
This should be used only when wrapped by an if ([_target respondsToSelector:_selector]) { or similar logic.Emmott
@ScottThompson so if the method in question doesn't return a value, this is safe to use? Thanks.Vanny
@ScottThompson THANKS for that, this is kind of what I imagined, but good to have it confirmed.Vanny
Why did you use a do ... while loop?Slither
I did not add the technique for using macros to hide the pragmas as I would never use that mechanism myself. I prefer the pragmas be explicit and commented rather than obfuscated. Having said that, The do-while loop would provide context and scope for any code in “Stuff” and essentially turns the macro into a statement rather than a floating code block.Vatican
A
208

My guess about this is this: since the selector is unknown to the compiler, ARC cannot enforce proper memory management.

In fact, there are times when memory management is tied to the name of the method by a specific convention. Specifically, I am thinking of convenience constructors versus make methods; the former return by convention an autoreleased object; the latter a retained object. The convention is based on the names of the selector, so if the compiler does not know the selector, then it cannot enforce the proper memory management rule.

If this is correct, I think that you can safely use your code, provided you make sure that everything is ok as to memory management (e.g., that your methods do not return objects that they allocate).

Alvord answered 10/8, 2011 at 20:43 Comment(14)
Thanks for the answer, I'll look more into this to see what is going on. Any idea on how I can bypass the warning though and make it disappear? I would hate to have the warning sitting in my code forever for what is a safe call.Hodeida
Workaround: what about a wrapper of performSelector that receives a string (the method name) and call the original performSelector by indexing into an array of SEL objects that are known to the compiler (or maybe switching among those SELs)? a little clumsy, maybe, but should work...Alvord
I don't really have the list of all SEL objs, as those can be implemented in subclasses. Only way would be go iterate over all methods and create an NSArray of all methods to find the proper one, but then I'll fall in the same situation, no?Hodeida
I don't know... since ObjC is dynamic, I think that having a SEL could allow the ARC mechanism to build some code around it... you can only try it and maybe report here so that everyone knows...Alvord
So I got confirmation from somebody at Apple in their forums that this is indeed the case. They'll be adding a forgotten override to allow people to disable this warning in future releases. Thanks.Hodeida
@sergio, great workaround to use NSSelectorFromString but the compiler warning is the same :(Vanny
This answer raises some questions, like if ARC tries to make determinations on when to release something based upon convention and method names, then how is it "reference counting"? The behavior you describe sounds only marginally better than completely arbitrary, if ARC is assuming the code follows a certain convention as opposed to actually keeping track of the references no matter what convention is followed.Disparagement
ARC automates the process of adding retains and releases at compile. It is not garbage collection (which is why it is so incredibly fast and low overhead). It is not arbitrary at all. The default rules are based on well-established ObjC conventions that have been consistently applied for decades. This avoids the need to explicitly add an __attribute to every method explaining its memory management. But it also makes it impossible for the complier to properly handle this pattern (a pattern which used to be very common, but has been replaced with more robust patterns in recent years).Straightlaced
So we can no longer have an ivar of type SEL and assign different selectors depending on the situation? Way to go, dynamic language...Privet
In my case, the selector is one of two possible values. I can use a switch statement and call each selector explicitly on each case...Privet
@NicolasMiari You certainly can do that, you just need to disable the warning (see another answer).Klagenfurt
@aroth, this whole thing did feel arbitrary until I actually read the spec. It makes me feel a lot more confident. clang.llvm.org/docs/AutomaticReferenceCounting.htmlHarday
Why not calling this ominous convention by name? There should only be issues in case you are setting object properties by using selectors: As a simple rule, if you can avoid performSelector:withObject: ARC should be fine and the warning can be silenced without fear.Misdeal
Curiously, you don't get this warning for all flavors of performSelector. The versions defined off of class NSObject vs protocol NSObject don't appear to provoke this message.Shote
N
121

In your project Build Settings, under Other Warning Flags (WARNING_CFLAGS), add
-Wno-arc-performSelector-leaks

Now just make sure that the selector you are calling does not cause your object to be retained or copied.

Nomenclature answered 31/10, 2011 at 13:57 Comment(1)
Note you can add the same flag for specific files rather than the entire project. If you look under Build Phases->Compile Sources, you can set per file Compiler Flags (just like you want to do for excluding files from ARC). In my project just one file should use selectors this way, so I just excluded it and left the others.Stray
I
110

As a workaround until the compiler allows overriding the warning, you can use the runtime.

You need header:

#import <objc/message.h>

Then try below:

// For strict compilers.
((id(*)(id,SEL))objc_msgSend)(_controller, sel_getUid("someMethod"));

OR

// Old answer's code:
objc_msgSend(_controller, NSSelectorFromString(@"someMethod"));

Instead of:

[_controller performSelector:NSSelectorFromString(@"someMethod")];
Intuit answered 16/8, 2011 at 4:56 Comment(5)
ARC recognizes Cocoa conventions and then adds retains and releases based on those conventions. Because C does not follow those conventions, ARC forces you to use manual memory management techniques. If you create a CF object, you must CFRelease() it. If you dispatch_queue_create(), you must dispatch_release(). Bottom line, if you want to avoid the ARC warnings, you can avoid them by using C objects and manual memory management. Also, you can disable ARC on a per-file basis by using the -fno-objc-arc compiler flag on that file.Intuit
Not without casting, you can't. Varargs is not the same as an explicitly typed argument list. It'll generally work by coincidence, but I don't consider "by coincidence" to be correct.Catercorner
Don't do that, [_controller performSelector:NSSelectorFromString(@"someMethod")]; and objc_msgSend(_controller, NSSelectorFromString(@"someMethod")); are not equivalent! Have a look at Method Signature Mismatches and A big weakness in Objective-C's weak typing they are explaining the problem in depth.Nomenclature
@Nomenclature In this case, it's fine. objc_msgSend will not create a method signature mismatch for any selector that would have worked correctly in performSelector: or its variants since they only ever take objects as parameters. As long as all your parameters are pointers (incl. objects), doubles and NSInteger/long, and your return type is void, pointer or long, then objc_msgSend will work correctly.Ez
ObjC does not have function overloading like c++ has. So even thought mikeash's website is expressing real concerns, you should get a compiler warning when you try to overload (not meaning override - in case someone mixes those words) methods that can not be overloaded because of ObjC.Demp
C
88

To ignore the error only in the file with the perform selector, add a #pragma as follows:

#pragma clang diagnostic ignored "-Warc-performSelector-leaks"

This would ignore the warning on this line, but still allow it throughout the rest of your project.

Colostomy answered 18/1, 2012 at 21:31 Comment(3)
I gather that you can also turn the warning back on immediately after the method in question with #pragma clang diagnostic warning "-Warc-performSelector-leaks". I know if I turn off a warning, I like to turn it back on at the soonest possible moment, so I don't accidentally let another unanticipated warning slip by. It's unlikely that this is a problem, but it's just my practice whenever I turn off a warning.Andyane
You can also restore your previous compiler configuration state by using #pragma clang diagnostic warning push before you make any changes and #pragma clang diagnostic warning pop to restore the previous state. Useful if you are turning off loads and don't want to have lots of re-enable pragma lines in your code.Monogamist
It will only ignore the following line?Reft
S
71

Strange but true: if acceptable (i.e. result is void and you don't mind letting the runloop cycle once), add a delay, even if this is zero:

[_controller performSelector:NSSelectorFromString(@"someMethod")
    withObject:nil
    afterDelay:0];

This removes the warning, presumably because it reassures the compiler that no object can be returned and somehow mismanaged.

Splenius answered 11/11, 2012 at 19:19 Comment(3)
Do you know if this actually resolves the related memory management issues, or does it have the same issues but Xcode isn't smart enough to warn you with this code?Bluegrass
This is semantically not the same thing! Using performSelector:withObject:AfterDelay: will perform the selector in the next run of the runloop. Therefore, this method returns immediately.Bergh
@Bergh Of course it's not the same! Read my answer: I say if acceptable, because the result is void and the runloop cycles. That's the first sentence of my answer.Splenius
B
34

Here is an updated macro based on the answer given above. This one should allow you to wrap your code even with a return statement.

#define SUPPRESS_PERFORM_SELECTOR_LEAK_WARNING(code)                        \
    _Pragma("clang diagnostic push")                                        \
    _Pragma("clang diagnostic ignored \"-Warc-performSelector-leaks\"")     \
    code;                                                                   \
    _Pragma("clang diagnostic pop")                                         \


SUPPRESS_PERFORM_SELECTOR_LEAK_WARNING(
    return [_target performSelector:_action withObject:self]
);
Bonnet answered 7/5, 2013 at 14:58 Comment(1)
return doesn't have to be inside the macro; return SUPPRESS_PERFORM_SELECTOR_LEAK_WARNING([_target performSelector:_action withObject:self]); also works and looks saner.Laaland
D
31

This code doesn't involve compiler flags or direct runtime calls:

SEL selector = @selector(zeroArgumentMethod);
NSMethodSignature *methodSig = [[self class] instanceMethodSignatureForSelector:selector];
NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:methodSig];
[invocation setSelector:selector];
[invocation setTarget:self];
[invocation invoke];

NSInvocation allows multiple arguments to be set so unlike performSelector this will work on any method.

Diploblastic answered 1/2, 2012 at 15:46 Comment(2)
Do you know if this actually resolves the related memory management issues, or does it have the same issues but Xcode isn't smart enough to warn you with this code?Bluegrass
You could say it solves the memory management issues; but this is because it basically lets you specify the behavior. For example, you can choose to let invocation retain the arguments or not. To my current knowledge, it attempts to fix the signature mismatch problems that might appear by trusting that you know what you are doing and don't provide it with incorrect data. I'm not sure if all the checks can be performed at runtime. As mentiones in another comment, mikeash.com/pyblog/… nicely explaines what mismatches can do.Finnegan
H
21

Well, lots of answers here, but since this is a little different, combining a few answers I thought I'd put it in. I'm using an NSObject category which checks to make sure the selector returns void, and also suppresses the compiler warning.

#import <Foundation/Foundation.h>
#import <objc/runtime.h>
#import "Debug.h" // not given; just an assert

@interface NSObject (Extras)

// Enforce the rule that the selector used must return void.
- (void) performVoidReturnSelector:(SEL)aSelector withObject:(id)object;
- (void) performVoidReturnSelector:(SEL)aSelector;

@end

@implementation NSObject (Extras)

// Apparently the reason the regular performSelect gives a compile time warning is that the system doesn't know the return type. I'm going to (a) make sure that the return type is void, and (b) disable this warning
// See https://mcmap.net/q/45398/-performselector-may-cause-a-leak-because-its-selector-is-unknown

- (void) checkSelector:(SEL)aSelector {
    // See https://mcmap.net/q/46469/-objective-c-gt-gt-is-there-a-way-to-check-a-selector-return-value
    Method m = class_getInstanceMethod([self class], aSelector);
    char type[128];
    method_getReturnType(m, type, sizeof(type));

    NSString *message = [[NSString alloc] initWithFormat:@"NSObject+Extras.performVoidReturnSelector: %@.%@ selector (type: %s)", [self class], NSStringFromSelector(aSelector), type];
    NSLog(@"%@", message);

    if (type[0] != 'v') {
        message = [[NSString alloc] initWithFormat:@"%@ was not void", message];
        [Debug assertTrue:FALSE withMessage:message];
    }
}

- (void) performVoidReturnSelector:(SEL)aSelector withObject:(id)object {
    [self checkSelector:aSelector];

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-performSelector-leaks"
    // Since the selector (aSelector) is returning void, it doesn't make sense to try to obtain the return result of performSelector. In fact, if we do, it crashes the app.
    [self performSelector: aSelector withObject: object];
#pragma clang diagnostic pop    
}

- (void) performVoidReturnSelector:(SEL)aSelector {
    [self checkSelector:aSelector];

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Warc-performSelector-leaks"
    [self performSelector: aSelector];
#pragma clang diagnostic pop
}

@end
Hanford answered 21/2, 2014 at 4:32 Comment(1)
Should 'v' be replaced by _C_VOID? _C_VOID is declared in <objc/runtime.h>.Vivle
M
16

For posterity's sake, I've decided to throw my hat into the ring :)

Recently I've been seeing more and more restructuring away from the target/selector paradigm, in favor of things such as protocols, blocks, etc. However, there is one drop-in replacement for performSelector that I've used a few times now:

[NSApp sendAction: NSSelectorFromString(@"someMethod") to: _controller from: nil];

These seem to be a clean, ARC-safe, and nearly identical replacement for performSelector without having to much about with objc_msgSend().

Though, I have no idea if there is an analog available on iOS.

Minica answered 26/2, 2012 at 4:17 Comment(3)
Thanks for including this.. It is available in iOS: [[UIApplication sharedApplication] sendAction: to: from: forEvent:]. I looked into it once, but it kind of feels awkward to use a UI-related class in the middle of your domain or service just to do a dynamic call.. Thanks for including this though!Hodeida
Ew! It'll have more overhead (since it needs to check whether the method is available and walk up the responder chain if it isn't) and have different error behaviour (walking up the responder chain and returning NO if it can't find anything which responds to the method, instead of simply crashing). It also doesn't work when you want the id from -performSelector:...Dissatisfied
@Dissatisfied It doesn't "walk up the responder chain" unless to: is nil, which it isn't. It just goes straight to the targeted object with no checking beforehand. So there isn't "more overhead". It's not a great solution, but the reason you give isn't the reason. :)Splenius
W
15

Matt Galloway's answer on this thread explains the why:

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?

It seems that it is generally safe to suppress the warning if you are ignoring the return value. I'm not sure what the best practice is if you really need to get a retained object from performSelector -- other than "don't do that".

Worthy answered 15/5, 2012 at 20:17 Comment(0)
R
14

@c-road provides the right link with problem description here. Below you can see my example, when performSelector causes a memory leak.

@interface Dummy : NSObject <NSCopying>
@end

@implementation Dummy

- (id)copyWithZone:(NSZone *)zone {
  return [[Dummy alloc] init];
}

- (id)clone {
  return [[Dummy alloc] init];
}

@end

void CopyDummy(Dummy *dummy) {
  __unused Dummy *dummyClone = [dummy copy];
}

void CloneDummy(Dummy *dummy) {
  __unused Dummy *dummyClone = [dummy clone];
}

void CopyDummyWithLeak(Dummy *dummy, SEL copySelector) {
  __unused Dummy *dummyClone = [dummy performSelector:copySelector];
}

void CloneDummyWithoutLeak(Dummy *dummy, SEL cloneSelector) {
  __unused Dummy *dummyClone = [dummy performSelector:cloneSelector];
}

int main(int argc, const char * argv[]) {
  @autoreleasepool {
    Dummy *dummy = [[Dummy alloc] init];
    for (;;) { @autoreleasepool {
      //CopyDummy(dummy);
      //CloneDummy(dummy);
      //CloneDummyWithoutLeak(dummy, @selector(clone));
      CopyDummyWithLeak(dummy, @selector(copy));
      [NSThread sleepForTimeInterval:1];
    }} 
  }
  return 0;
}

The only method, which causes memory leak in my example is CopyDummyWithLeak. The reason is that ARC doesn't know, that copySelector returns retained object.

If you'll run Memory Leak Tool you can see the following picture: enter image description here ...and there are no memory leaks in any other case: enter image description here

Receptionist answered 25/12, 2012 at 10:23 Comment(0)
M
7

Do not suppress warnings!

There are no less than 12 alternative solutions to tinkering with the compiler.
While you are being clever at the time the first implementation, few engineer on Earth can follow your footsteps, and this code will eventually break.

Safe Routes:

All these solutions will work, with some degree of of variation from your original intent. Assume that param can be nil if you so desire:

Safe route, same conceptual behavior:

// GREAT
[_controller performSelectorOnMainThread:selector withObject:anArgument waitUntilDone:YES];
[_controller performSelectorOnMainThread:selector withObject:anArgument waitUntilDone:YES modes:@[(__bridge NSString *)kCFRunLoopDefaultMode]];

[_controller performSelector:selector onThread:[NSThread mainThread] withObject:anArgument waitUntilDone:YES];
[_controller performSelector:selector onThread:[NSThread mainThread] withObject:anArgument waitUntilDone:YES modes:@[(__bridge NSString *)kCFRunLoopDefaultMode]];

Safe route, slightly different behavior:

(See this response)
Use any thread in lieu of [NSThread mainThread].

// GOOD
[_controller performSelector:selector withObject:anArgument afterDelay:0];
[_controller performSelector:selector withObject:anArgument afterDelay:0 inModes:@[(__bridge NSString *)kCFRunLoopDefaultMode]];

[_controller performSelectorOnMainThread:selector withObject:anArgument waitUntilDone:NO];
[_controller performSelectorOnMainThread:selector withObject:anArgument waitUntilDone:NO];
[_controller performSelectorOnMainThread:selector withObject:anArgument waitUntilDone:NO modes:@[(__bridge NSString *)kCFRunLoopDefaultMode]];

[_controller performSelectorInBackground:selector withObject:anArgument];

[_controller performSelector:selector onThread:[NSThread mainThread] withObject:anArgument waitUntilDone:NO];
[_controller performSelector:selector onThread:[NSThread mainThread] withObject:anArgument waitUntilDone:NO modes:@[(__bridge NSString *)kCFRunLoopDefaultMode]];

Dangerous Routes

Requires some kind of compiler silencing, which is bound to break. Note that at present time, it did break in Swift.

// AT YOUR OWN RISK
[_controller performSelector:selector];
[_controller performSelector:selector withObject:anArgument];
[_controller performSelector:selector withObject:anArgument withObject:nil];
Moonshiner answered 31/12, 2015 at 3:58 Comment(5)
The wording is very wrong. The safe routes are not safer than dangerous at all. It is arguably more dangerous because it hides the warning implicitly.Brownfield
I will fix the wording to not be insulting, but I stand by my word. The only time I find silencing warning acceptable is if I do not own the code. No engineer can safely maintain silenced code without understanding all the consequences, which would mean read this argument, and this practice is plain risky ; especially if you consider the 12, plain English, robust alternatives.Moonshiner
No. You didn't get my point. Using performSelectorOnMainThread is not a good way to silence the warning and it has side effects. (it doesn't solve the memory leak) The extra #clang diagnostic ignored explicitly suppress the warning in a very clear way.Brownfield
True that performing a selector on a non - (void) method is the real issue.Moonshiner
and how do you call a selector with multiple arguments through this and be safe in the same time? @MoonshinerDress
C
6

To make Scott Thompson's macro more generic:

// String expander
#define MY_STRX(X) #X
#define MY_STR(X) MY_STRX(X)

#define MYSilenceWarning(FLAG, MACRO) \
_Pragma("clang diagnostic push") \
_Pragma(MY_STR(clang diagnostic ignored MY_STR(FLAG))) \
MACRO \
_Pragma("clang diagnostic pop")

Then use it like this:

MYSilenceWarning(-Warc-performSelector-leaks,
[_target performSelector:_action withObject:self];
                )
Chainsmoke answered 3/8, 2015 at 21:46 Comment(2)
FWIW, I did not add the macro. Someone added that to my response. Personally, I wouldn't use the macro. The pragma is there to work around a special case in the code and the pragmas are very explicit and direct about what's going on. I prefer to keep them in place rather than hiding, or abstracting them behind a macro, but that's just me. YMMV.Vatican
@ScottThompson That's fair. For me it's easy to search for this macro across my code base and I generally also add an un-silenced warning to deal with the underlying issue.Chainsmoke
B
4

Because you are using ARC you must be using iOS 4.0 or later. This means you could use blocks. If instead of remembering the selector to perform you instead took a block, ARC would be able to better track what is actually going on and you wouldn't have to run the risk of accidentally introducing a memory leak.

Belting answered 23/11, 2011 at 2:33 Comment(2)
Actually, blocks make it very easy to accidentally create a retain cycle which ARC does not solve. I still wish that there was a compiler warning when you implicitly used self via an ivar (e.g. ivar instead of self->ivar).Dissatisfied
You mean like -Wimplicit-retain-self ?Passmore
A
2

Instead of using the block approach, which gave me some problems:

    IMP imp = [_controller methodForSelector:selector];
    void (*func)(id, SEL) = (void *)imp;

I will use NSInvocation, like this:

    -(void) sendSelectorToDelegate:(SEL) selector withSender:(UIButton *)button 

    if ([delegate respondsToSelector:selector])
    {
    NSMethodSignature * methodSignature = [[delegate class]
                                    instanceMethodSignatureForSelector:selector];
    NSInvocation * delegateInvocation = [NSInvocation
                                   invocationWithMethodSignature:methodSignature];


    [delegateInvocation setSelector:selector];
    [delegateInvocation setTarget:delegate];

    // remember the first two parameter are cmd and self
    [delegateInvocation setArgument:&button atIndex:2];
    [delegateInvocation invoke];
    }
Anodyne answered 21/5, 2015 at 8:52 Comment(0)
T
2

If you don't need to pass any arguments an easy workaround is to use valueForKeyPath. This is even possible on a Class object.

NSString *colorName = @"brightPinkColor";
id uicolor = [UIColor class];
if ([uicolor respondsToSelector:NSSelectorFromString(colorName)]){
    UIColor *brightPink = [uicolor valueForKeyPath:colorName];
    ...
}
Trevethick answered 15/2, 2017 at 4:8 Comment(0)
O
-1

You could also use a protocol here. So, create a protocol like so:

@protocol MyProtocol
-(void)doSomethingWithObject:(id)object;
@end

In your class that needs to call your selector, you then have a @property.

@interface MyObject
    @property (strong) id<MyProtocol> source;
@end

When you need to call @selector(doSomethingWithObject:) in an instance of MyObject, do this:

[self.source doSomethingWithObject:object];
Oft answered 3/8, 2013 at 11:34 Comment(1)
Hey Wu, thanks, but the point of using the NSSelectorFromString is when you don't know which selector you want to call during runtime.Hodeida

© 2022 - 2024 — McMap. All rights reserved.