Apache OpenOffice (AOO) Bugzilla – Full Text Issue Listing |
Summary: | fix memory leaks on Aqua ( Carbon ) version | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | porting | Reporter: | eric.bachard | ||||||
Component: | MacOSX | Assignee: | ericb | ||||||
Status: | CLOSED FIXED | QA Contact: | issues@porting <issues> | ||||||
Severity: | Trivial | ||||||||
Priority: | P3 | CC: | issues, tino.rachui | ||||||
Version: | 680m225 | Keywords: | aqua | ||||||
Target Milestone: | OOo 3.0 | ||||||||
Hardware: | Mac | ||||||||
OS: | Mac OS X, all | ||||||||
Issue Type: | DEFECT | Latest Confirmation in: | --- | ||||||
Developer Difficulty: | --- | ||||||||
Attachments: |
|
Description
eric.bachard
2007-08-25 12:04:44 UTC
Added Tino on CC Created attachment 47816 [details]
a little patch for sal. No problem, but needs confirmation
Created attachment 47817 [details]
another patch for dtrans
The real issue is imho in dtrans ( leaks confirmed) , but this is a complicated issue: objects including CFStrings or CFAreaRef or .. are instanciated, and I found nothing obvious explaining how everything is freed. e.g; every time we copy paste, new leaks appear, confirming the root cause is in dtrans aqua. ericb->tino In dtrans/source/aquaDataFlavorMapping.cxx, getLegacyClipboardId() is not used at all. Can you confirm ? tra->ericb: I reviewed my code and am not sure if 'leaks' is not reporting false positives. In the first case OSXTransferable.cxx I'm using the function 'CFArrayGetValueAtIndex'. According to the documentation "...Return Value The value at the idx index in theArray. If the return value is a Core Foundation Object, ownership follows the Get Rule...". The "Get Rule" says "If you receive an object from any Core Foundation function other than a creation or copy function—such as a Get function—you do not own it and cannot be certain of the object’s life span. If you want to ensure that such an object is not disposed of while you are using it, you must claim ownership (with the CFRetain function). You are then responsible for relinquishing ownership when you have finished with it." I would interpret this as "I don't have to call CFRelease in my concrete case. Similar arguments count for the second case where I return a CFStringRef to a static immutable CFString object in 'DataFlavorMapping::openOfficeToSystemDataFlavor' so I don't see a reason for CFRelease here as well even in the exception case. But anyway leave the issue open I will re-investigate and try what happens when I use CFReleases nevertheless at the places you are suggesting when I'm done with the D&D implementation. tra->ericb: Regarding 'getLegacyClipboardId' it seems as if this function is really not used atm. I think planned to use it but didn't in the end. But the code needs some restructuring yet making the function necessary again so I would first remove it when the D&D implementation is done. If you would create two separate issue for the leaks one for sal and another one for dtrans I would add the dtrans issue to cws macosxdnd. tra->ericb: Regarding the sal patch: Although the docu doesn't say so I could imagine that the use of 'CFStringConvertEncodingToIANACharSetName' requires a CFRelease for the use of 'CFArrayGetValueAtIndex' the same comments as above apply in my opinion. ericb->tra I'll answer you for the first part asap (currently searching for a breaker in m226 ) About sal patch, if you read just two lines above, we have (yes, an extra empty line interfered): CFStringRef sCFEncodingName; sCFEncodingName = CFStringConvertEncodingToIANACharSetName( cfEncoding ); CFStringGetCString( sCFEncodingName, buffer, bufferLen, cfEncoding ); if ( sCFEncodingName ) CFRelease( sCFEncodingName); What I understand, is you are the owner of the CFStringRef, and you must release it. For the second case: CFStringRef lang = (CFStringRef)CFArrayGetValueAtIndex(subs, 0); CFStringGetCString(lang, locale, bufferLen, kCFStringEncodingASCII); if ( lang ) CFRelease( lang); I understand the same, and imho, you are the owner of the CFString too. ( waiting, I'll read again Get and SET rules anyway :-) ericb->tra In sal, I have removed the if it the two hunks, and so far, no crash, copy paste and everything works. I think I'd have at least one if ever the CFRelease() were wrong. To complete, I'll trace a bit, insetring a breakpoint just before the release ericb->tra I discussed both patches with pl In sal, release lang is wrong, because the array is autoreleased when the function gets out of scope. For the second, pl agreed CFRelease( sCFEncodingName) is correct, and the change is really needed. In dtrans, things are not clear, because entries are released, but not the Array itself, and more investigations are needed. To summarize, on change can be applied in sal, and we'll see later what exactly needs to be done. Setting keyword @tino : can you please confirm ? Since macosxdnd, the dtrans patch seems to be obsolete I'll ask pl if I can add the change in sal only. aquavcl05 could be ok Tjhe change about CFRelease( sCFEncodingName), discussed on IRC (long time ago) with Philipp has been commited. Issue fixed target verified in CWS aquavcl05 This seems to be a developer issue, I cant edit it. please help, and confirm or close it - thanks Closing resolved issue. |