Am 11.11.2010 um 15:40 schrieb Bill Cheeseman:
On Nov 11, 2010, at 8:42 AM, Christiaan Hofman wrote: However, you have neglected to CFRelease (or release, or autorelease) the value stored in axValue by the AXUIElementCopyAttributeValue() function call in the body of your method The Cocoa convention is that a method like your -valueForAttributeCopy: always returns an object autoreleased. So, you might want to add '[axValue autorelease]' or '[value autorelease]' before the return, or change the last line to 'return [value autorelease]', and see what happens. (If the caller of your method does releases its return value when done with it, remove that release because it would be automatically released by the autorelease added in your method).
Actually, no. The docs (Memory Management Programming Guide) say that the returned value in this case should be retained:
"You “create” an object using a method whose name begins with “alloc” or “new” or contains “copy” "
Note the word "contains", which only applies to "copy".
(I personally think this rule is wrong, or at least formulated wrongly and inconsistently in the docs. For instance compare a method initWithObjects:copyItems: which clearly *contains* the word "copy" but whose return value isn't explicitly retained. But Apple does not agree with me, and they refuse to be unambiguous in the docs, even though it's pretty straightforward to be clear.)
Apart from this, I would advice to just avoid any confusion, and rename the method: either name it -valueForAttribute: and autorelease the value, or name it -copyValueForAttribute:.
If -copyValueForAttribute: is more clear than -valueForAttributeCopy: I'll rename it... I had exactly the same thing in mind, that's why I included "copy", trying to stay with naming conventions. In my class, there's an alternative -valueForAttribute: method which returns an autoreleased object, as we would expect. In this case, however, I prefer getting a retained object and releasing it straight away when the comparison is done, since the caller recursively iterates over the whole AXUIElement tree. When using autoreleased objects, there'd be thousands of objects added to the autorelease pool until the caller returns extending my app's memory footprint by about factor 10.
I wasn't clear in my response. I was addressing the Core Foundation "copy" rule, which is similar to but perhaps theoretically separate from the Cocoa naming convention you cite.
When I cited the rule that values returned by Core Foundation functions containing the word "copy" must be CFReleased when no longer needed, I was referring to the OP's call to the Accessibility API function AXUIElementCopyAttributeValue() in the body of his method. That function contains the word "copy," which tells us that its indirectly returned value has been CFRetained. He is therefore responsible for CFReleasing the object that is returned indirectly in the function's axValue argument, but he does not do that in the body of his method.
It is entirely possible, of course, that the OP did in fact intend to return it from his Cocoa method CFRetained, and that he therefore deliberately included the word "copy" in his method name in order to comply with the Cocoa naming convention. That would be perfectly correct and appropriate, as you point out. But in that case, it would be his responsibility to release the return value from his method when he is done with it. Perhaps he in fact did so (he didn't show us his calling code), but I was guessing that the memory leak he was experiencing came from a failure to do so.
Ok, I didn't want to bother you with my calling code, but I mentioned: "In the calling method, I compare the returned object against the value I'm looking for and then release it by calling [value release]."
// -valueForAttributeCopy: // Returns an object typed id representing the value for a given attribute of the receiver's AXUIElement or nil if the attribute doesn't exist. // The sender is responsible for releasing the object. Let's assume that someone who types these lines has some understanding of what they mean :)
Once again: I'm not so much confused about memory management in my code, but about Mail and other apps not releasing memory until a click is performed on a menu item in Mail.
I agree with your suggestion that he should remove the word "copy" from the name of his method if he decides to autorelease the return value. However, I fear that your suggestion that the value returned by his method as it is currently named should be retained because it does contain the word "copy" may confuse him. You have correctly described the naming convention, but in this case the return object of his method is already retained (or, I should say, CFRetained) because it was obtained from an Accessibility API function containing the word "copy." If this was his intent and he properly released the value later, then he doesn't have to change anything and his memory leak is caused by something else entirely.
Turning to your comment on a hypothetical -initWithObjects:copyItems: method in Cocoa, I wonder if you aren't taking the Cocoa naming convention a little too literally. It is true that the method signature contains the word "copy," but it seems clear to me in this case that "copy" is associated with "items" and does not refer to the returned object. The Cocoa methods that include "copyItems" in their signatures, such as -initWithArray:copyItems:, are documented as copying (not retaining) the items in the array. This might be distinguished from the Application Services function PMWorkflowCopyItems(), which is documented to require that the caller release the result.
I absolutely agree that "copy" might be somewhat misleading in this case, since the returned object is not a deep copy of the CFTypeRef but a retained "original". In the case of -initWithArray:copyItems: the items are deep-copied if the copyItems flag is YES, otherwise the "originals" are retained. The CFTypeRef objects don't understand the copy message even when they are cast to type id anyway. I'd use it if possible (and avoid the naming dilemma)... You might agree that beginning the method name with "new" would be even less appropriate. I'll be happy to follow any suggestions on how to make more transparent what my method actually does.
Besides from the important naming conventions discussion, let me repeat my key question: Why do some apps (Mail, Firefox) accumulate allocated memory in their own memory space when my app scans their AXUIElements? while other apps don't!
Any clues?
Peter
p.s. While writing this (obviously, I'm too slow:)), I received some further mails from Christiaan and Bill, I'll send this nevertheless and read your mails thereafter...
|