Brad S
Brad S

Reputation: 605

Analyzer is complaining about a possible resource leak in multithreaded Cocoa app

Ok, I am an experienced C++ developer. I am learning Objective-C on the fly while trying to build a fairly substantial Cocoa application. I have done some simpler apps with Cocoa while gearing up for this project and I think I have a good handle on most of the concepts.

The memory management paradigm is still a little vague to me however so I build with the memory analyzer to help me find issues right away, and to be honest it has been awesome and has done more to help me understand Objective-C memory management that any of the documentation.

So here is my question.

I have two threads that communicate with each other using the performSelector:onThread:withObject:waitUntilDone: method. They pass objects between each other passed in as the withObject: parameter of the method.

Sample code is below:

- (BOOL)postMessage:(id <MessageTarget>)sender messageId:(NSInteger)msgId messageData:(void*)data
{
    // message is allocated and retained here. retain count will be +2
    Message* message = [[[Message alloc] initWithSender:sender messageId:msgId messageData:data] retain];
    if(message)
    {
        // message will be released in other thread.
        [self performSelectorOnMainThread:@selector(messageHandler:) withObject:message waitUntilDone:NO];
        // message is released here and retain count will be +1 or 0 depending on thread ordering
        [message release];
        return YES;
    }
    return NO;
}

- (BOOL)sendMessage:(id <MessageTarget>)sender messageId:(NSInteger)msgId messageData:(void*)data messageResult:(void**)result
{
    // message is allocated and retained here. retain count will be +2
    Message* message = [[[Message alloc] initWithSender:sender messageId:msgId messageData:data] retain];
    if(message)
    {
        // message will be released in other thread. retain count will be +1 on return
        [self performSelectorOnMainThread:@selector(messageHandler:) withObject:message waitUntilDone:YES];
        if(result)
            *result = [message result];
        // message is released here and retain count will be 0 triggering deallocation
        [message release];
        return YES;
    }
    return NO;
}

- (void)messageHandler:(Message*)message
{
    // message will have a retain count of +1 or +2 in here based on thread execution order
    if(message)
    {
        switch ([message messageId])
        {
            case 
               ...
            break;

            default:
               ...
            break;
        }
        // message is released here bringing retain count to +1 or 0 depending on thread execution ordering
        [message release];
    }
}

The analyzer is complaining about a possible leak of the Message object allocated in postMessage: and sendMessage: but the object is released in messageHandler:. The code runs correctly and does not leak, and I suspect the analyzer is simply not able to see that the Message object is being released in a different thread.

Now in case you are wondering why I do the second retain in the post/send methods and not in the messageHandler: method, it is because the postMessage: method is meant to be asynchronous and [message release] in the post method may get executed before the [message retain] in messageHandler: would, leaving me with an invalid object. It would work just fine if I did that in the case of the sendMessage: method because it is synchronous.

So is there a better way to do this that would satisfy the memory analyzer? Or maybe a way to give the memory analyzer a hint that the object is in fact being released?

Update:

Torrey provided the answer below but I wanted to clarify what I had to do different from what he suggested.

He suggested using an attribute on my messageHandler: method as below

- (void)messageHandler:(Message*) __attribute__((ns_consumed)) message;

This did not quite work since the object is being passed into performSelector: and the analyzer does not see it being passed along to messageHandler:

Since the performSelector: call is defined by Cocoa and not by me I can not add the attribute to it.

The way around this is to wrap the call to performSelector: as follows:

- (void)myPerformSelector:(SEL)sel onThread:(NSThread*)thread withObject:(id) __attribute__((ns_consumed)) message waitUntilDone:(BOOL)wait;
{
    [self performSelector:sel onThread:thread withObject:message waitUntilDone:wait];
}

Then you can call the wrapper function and the analyzer will see the attribute and not complain about an unbalanced retain/release pair. In the end I did not like the extra indirection just to get rid of the warning so I used the preprocessor as explained in my comment below. But I could see situations where it could be useful to use this method.

Upvotes: 1

Views: 197

Answers (2)

justin
justin

Reputation: 104698

You do not need to explicitly transfer reference count ops to the secondary thread when using this API.

  • a) Your caller holds a reference when waitUntilFinished is true,
  • b) and the implementation behind + [NSThread performSelector:… also does (see: - [NSRunLoop performSelector:target:argument:order:modes:]).

There is no need to pass reference count duties for the parameters (or self) across threads in this case.

It will either be performed immediately, or self and the parameter (it's objc-typed) will be retained while in the other thread's run loop queue (and self and the parameter are released after the object's performed the selector).

(don't use ref-op __attribute__s to shut it up)

Upvotes: 2

torrey.lyons
torrey.lyons

Reputation: 5589

You should be able to make the analyzer happy by judicious use of Clang's ns_consumed attribute. As you suggested, this gives the memory analyzer a hint that a release message will be sent to the parameter upon completion of the function call. You would use it like:

- (void)messageHandler:(Message*) __attribute__((ns_consumed)) message

There is more information on Cocoa memory management annotations in the Clang analyzer documentation. You may want to wrap the attribute setting in an NS_COSUMED macro for compatibility with other compilers as suggested on that page.

Upvotes: 0

Related Questions