Re: [gtk-osx-devel] [PATCH] gtk-mac-integration reference counting

[Richard Procter <richard n procter gmail com>]

On 30/01/2012, at 11:32 AM, John Ralls wrote:
> 
> ARC is the feature added in 10.7. There were also memory management changes between 10.5 and 10.6. It would be wise to build against the 10.4SDK and compare the analysis from that with the results from building against SDK 10.6. It might be necessary to #ifdef some of the changes by version. I don't know, the only way to be sure is to do the check.

I would but I don't have easy access to a 10.4 machine. 

The only memory management changes I have found in the 'what's new' notes for 10.6 refer to garbage collection, which is not enabled for gtk-mac-intergration (via the -fobjc-gc[-only] flags). If it were enabled I don't believe this would affect the correctness of a manual refcounting implementation. Let's know if you were referring to something else. 

As far as I can tell, all my changes are very much bread and butter and follow from the rules outlined in [2] and [3]. If Apple changed these basic semantics between 10.4 and 10.7 they would have broken a lot of existing code in a rather hairy-to-support-both-versions way. So I see it as very unlikely. But you're right, the only way to be sure is to verify it on 10.4. 

> For the autoreleased objects, did you make sure that the receivers retain them in all cases?

Yes. I have re-checked it and the only areas of potential doubt in my mind are: 

- In gtk_osxapplication_quartz.c:create_apple_menu(), an NSMenu is created and passed to GNSMenuItem via setSubMenu:, which I presume takes ownership of it. 
- ditto for gtk_osxapplication_quartz.c:gtk_osxapplication_set_menu_bar() and [NSApp setMainMenu:] 
- my only other assumption is that class method [NSString stringWithUTF8String:] returns an autorelease object; stackoverflow says it does, but I haven't come across this in the apple documentation [2] 

Lastly, my rationale for removing the existing [id release] of after obtaining id:  

   NSObject *id = [[NSBundle mainBundle] objectForInfoDictionaryKey: [NSString stringWithUTF8String: key]]

... following the OS X advanced memory management guide: "You own any object you create: You create an object using a method whose name begins with “alloc”, “new”, “copy”, or “mutableCopy” (for example, alloc, newObject, or mutableCopy)." and as this is not the case here, that implies that id should not be released [3]. That's also supported by "Collections Own the Objects They Contain" [3]

> Clang is reporting a potential memory leak, not offering advice about naming conventions. Hmm, that's going to cause memory trouble with introspection, too. I need to make a note about that.

I believe clang is flagging unconventional naming here, at least, indirectly: it does not flag a potential leak if the returning function contains 'create' in its name (following the Core Foundation create rule [1], which I assume applies to the CGImage). 

If the patch is a bit of a mouthful to verify, let's know if there's anyway I can make it easier. 

best, 
Richard. 


[1] http://developer.apple.com/library/mac/#documentation/CoreFOundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html
[2] http://stackoverflow.com/questions/6578/understanding-reference-counting-with-cocoa-and-objective-c
[3] http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmRules.html#//apple_ref/doc/uid/20000994-SW1