Memory leak analysis and self destructing objects

While running memory leak analysis I've noticed that it all objects which are self-destructing cause static analysis tool to produce false positives. By self-destructing I mean situations where ownership of an object is moved to the object itself, which then releases itself after certain operation has been completed (e.g. consider NSWindow, which can be set to release itself automatically after it's closed). Static analysis tool doesn't have this knowledge, and when it sees only an allocation without release, it reports memory leak.

For example:
-(void)awakeFromNib
{
NSWindow *window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0,0,100,100) styleMask:NSTitledWindowMask|NSClosableWindowMask backing: NSBackingStoreBuffered defer:NO]; // isReleasedWhenClosed is set to YES by default
  [window orderFrontRegardless];

  // False memory leak reported - window will be released when user closes it
}

Fortunately self-destructing objects are not very widely used, and vast majority of objects use the traditional ownership semantics. However, these false positives could be suppressed if a special attribute could be used to mark objects with self-destructing semantics. This attribute would also serve as a documentation for these non-traditional ownership semantics, which would otherwise require a trip to documentation viewer. Maybe something like NSWindow* __attribute__((self_ownership)) window = ...? Unlike NSWindow, where self-destruction can be enabled or disabled at runtime, some classes use self-destruction all the time, so maybe some class-level annotation could be used.

In addition to this case there're some other cases where this attribute won't help. For example, a delegate object may be set to NSWindow, and NSWindow will notify the delegate object when window is closing. Delegate may then release the window by sending autorelease to it. This case will probably cause false positive memory leak report as well.

Any thoughts? Is there some generic way to suppress these false positives?

- Nikita

I noticed that you’ve added the support for self-owning objects into lib/Analysis/CFRefCount.cpp in r52741. Thanks! I guess that commit was the “longer response” you were talking about.

I actually was planning on following up with you with an email after testing that patch further. Does it silence the false warnings? (at least in the case of NSWindow and its subclasses)

I was also investigating other issues that you mentioned; e.g. delegates. I’m not an Objective-C applications programmer, so while I understand much of the concepts and the common idioms, I’m lazily learning much of the common design practices. This makes feedback like yours especially useful.

What would help me with such feedback is to always provide concrete code examples. These give me a clear idea of what to implement in the static analyzer, and it provides test cases we can put into the test suite.

Are there any plans to generalize this checking logic so it could be used with other classes than NSWindow? In my own source code I have several classes which use similar self-ownership semantics as NSWindow, and it would be nice to avoid getting false memory leak reports for objects of those classes.

Absolutely. My thought is that we can implement an attribute that can be affixed to the class interface. I would need to implement parsing support for this in clang, but essentially it would something like this:

attribute((checker(“objc_owns_self”))) @interface@end

Of course GCC doesn’t recognize the attribute, so it could be wrapped in a macro:

#if clang
#define OBJC_OWNS_SELF attribute((checker(“objc_owns_self”)))
#else
#define OBJC_OWNS_SELF
#endif

and do the following in your code:

OBJC_OWNS_SELF
@interface
@end

Does this sound like a reasonable strategy? Your feedback is invaluable here. (you’re basically driving this feature!)

What would help me is also some code examples illustrating the idiom of self-ownership. That way I know I’m solving the right problem.

As completely offtopic question, does cfe-dev mailing list has some rules which specify when one should reply to mailing list and when to reply just directly to the original poster? I didn’t find any rules at general mailing list page at [1]. I’ve seen both behaviors, and I would like to follow the correct one.

[1] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

  • Nikita

I think the rule of thumb is to keep a conversation on the list, so that it is archived and allows other people to wade in. As long as the conversation stays clang related it is on-topic. I take things off list when it comes down to issues only involving two people (e.g., how to send a particular patch over email to a specific person, etc.). Most of the time the communication can be on the list itself. For example, the email that you just sent me could have gone directly to the list as is. In fact, I’ve CC’ed cfe-dev on this reply so that this can again become an active thread.

Best,
Ted

I was writing you a long reply describing these idioms in more detail with some concrete examples, but then I took a look how analyzer actually works with an open source project called Adium and I got little confused.

Adium contains several classes which use the self-ownership idiom, most of them are subclasses of NSWindowController. Each such controller object configures, displays and manages input and output of a single window. Each controller object is created usually in a class method of its own class. Each controller object releases itself later at some point (usually when the window or sheet it manages is closed by the user).

So I was expecting memory leak warning for each case where such controller object gets created. However, analyzer somehow does seem to handle those cases correctly.

For example, check:
http://trac.adiumx.com/browser/trunk/Source/AIDockIconSelectionSheet.m#L38

An object of class 'AIDockIconSelectionSheet' is created and stored into 'controller' variable. It's then passed to the [NSApp beginSheet:...] method. It's not released (or autoreleased) explicitly. After user closes the sheet, sheetDidEnd:... callback is called by AppKit (line 58), and the controller object created on line 40 releases itself. Analyzer doesn't report any memory leaks here (which is correct, but I actually expected to get false positive here).

However, when I duplicated the whole + (void)showDockIconSelectorOnWindow:... method and made it an instance method, without any other changes, analyzer did report a memory leak: "Object allocated on line 40 and stored into 'controller' is no longer referenced after this point and has a retain count of +1 (object leaked)".

I tried to reproduce this behavior in a simplified test case (so one wouldn't have to build the whole Adium to test this), but there seem to be some factors which I'm unaware of which cause memory leak errors to be suppressed in Adium but not in my test cases. So what am I missing here?

- Nikita

Hi Nikita,

Thanks for the excellent example. I’m not going to get a chance to look at it until next week, but the first thing I am going to do is verify why the analyzer is not emitting a warning when showDockIconSelectorOnWindow:… is a class method versus when it is an instance method. The analyzer appearing to “get it right” in some cases could just be it not tracking an allocation correctly (or even a bug in Clang), or something else that I’m not remembering off the top of my head. Once I investigate this a little further I’ll follow up again on this message.

Thanks again.

Ted

I'm reopened this issue as a new Bugzilla report:

http://llvm.org/bugs/show_bug.cgi?id=2588

It turns out that an NSWindow object doesn't self-own itself until it is displayed, so the code snippet above is actually a real leak. Right now the analyzer has been tweaked to consider an NSWindow object to be self-owning once it is initialized, but this is not correct. The bugzilla report has more information.

Ted

-orderFrontRegardless is a method that displays the window, so no, this code snippet is not a leak.

Thanks Jean-Daniel. I had missed that method call when I glanced over the code. The analyzer still doesn't handle NSWindow properly.