Description: Tracing memory leaks in LibreOfficeDev on Mac OSX Steps to Reproduce: 1. Start XCode, then Instruments.app 2. Choose Memory Leak profile tool 3. Select LibreOffice.app in instdir as target process 4. Click on the record button, LODev is started by the profiling tool 5. Wait for the StartCenter to load - note the occurrences of memory leaks as they occur. 6. Open a new Writer document. 7. When the document has loaded, click on the dropdown font list. 8. Stop recording. 9. Analyse the profile trace. Actual Results: The app leaks memory each time AddTempDevFont(const OUString& rFontFileURL) is invoked Expected Results: There should be no memory leaks. Reproducible: Always User Profile Reset: No Additional Info: User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
In this particular instance the code invoked is in salgdi.cxx, with the leaks occuring here : CFStringRef rFontPath = CFStringCreateWithCString(nullptr, aCFileName.getStr(), kCFStringEncodingUTF8); - 101 byte leak CFURLRef rFontURL = CFURLCreateWithFileSystemPath(nullptr, rFontPath, kCFURLPOSIXPathStyle, true); - 197 byte leak
@khaled : thought you might be interested
Similarly, at line 321 in salgdi.cxx : AddTempDevFont(aFileStatus.getFileURL()); a leak of 288 bytes
On startup, there are 101 occurrences of this on Mac OS. I assume that this corresponds to the number of fonts on the system ?
Created attachment 135205 [details] Screenshot of Instruments memory leak profile analysis
In NSURL, similar leaks are occurring, cf. enclosed screenshot of refcounts.
Created attachment 135206 [details] Ref count screenshot in NSURL
Similarly, the call to CTFontDescriptorRef pDesc = CTFontCopyFontDescriptor(pFallback); at line 80 of salgdi.cxx in the CFDictionary object.
Confirming. See the attachment link posted here: bug 99784 comment 86
https://opengrok.libreoffice.org/xref/core/vcl/quartz/salgdi.cxx#288 288 static bool AddTempDevFont(const OUString& rFontFileURL) 289 { 290 OUString aUSytemPath; 291 OSL_VERIFY( !osl::FileBase::getSystemPathFromFileURL( rFontFileURL, aUSytemPath ) ); 292 OString aCFileName = OUStringToOString( aUSytemPath, RTL_TEXTENCODING_UTF8 ); 293 294 CFStringRef rFontPath = CFStringCreateWithCString(nullptr, aCFileName.getStr(), kCFStringEncodingUTF8); 295 CFURLRef rFontURL = CFURLCreateWithFileSystemPath(nullptr, rFontPath, kCFURLPOSIXPathStyle, true); 296 297 CFErrorRef error; 298 bool success = CTFontManagerRegisterFontsForURL(rFontURL, kCTFontManagerScopeProcess, &error); 299 if (!success) 300 { 301 CFRelease(error); 302 } 303 304 return success; 305 } rFontPath and rFontURL should be released with this just before the return: CFRelease( rFontPath ); CFRelease( rFontURL ) I'll take this one. Alex: if you build from sources, could you give it a try? (1: does it build, 2: does it help for memory consumption) Indeed, I won't be able to test before getting back to home after my daytime job. BTW, great tool! :-)
I give it a try with https://gerrit.libreoffice.org/#/c/40845/
Julien Nabet committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=bc5bd201609ae197f320823880e66187ae94c98a tdf#111432: fix leaks in AddTempDevFont with CFRelease It will be available in 6.0.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Alex: https://tinderbox.libreoffice.org/cgi-bin/gunzip.cgi?tree=MASTER&full-log=1502128359.24387 shows it builds ok. (my Mac is building right now from scratch so couldn't tell) I cherry-pick the commit in 5.4 for review: see https://gerrit.libreoffice.org/#/c/40847/ I don't think a backport in 5.3 branch worths it since 5.3.5 has just been released and there's only 1 version left which will be for major bugs. When you'll build or get a daily build including this commit, could you provide some feedback so we're if it fixes it or not?
(In reply to Julien Nabet from comment #13) > > I don't think a backport in 5.3 branch worths it since 5.3.5 has just been > released and there's only 1 version left which will be for major bugs. > > When you'll build or get a daily build including this commit, could you > provide some feedback so we're if it fixes it or not? Hi Julien, sorry, I was busy with other stuff yesterday. If the change is in master already, I can pull and rebuild on my machine and run the tests again.
(In reply to Alex Thurgood from comment #14) >... > Hi Julien, sorry, I was busy with other stuff yesterday. If the change is in > master already, I can pull and rebuild on my machine and run the tests again. No problem! :-) Hopefully you can pull and rebuild because there's no daily builds (see http://nabble.documentfoundation.org/MacOs-Daily-master-builds-lack-of-MacOs-Jenkins-td4220058.html)
(In reply to Julien Nabet from comment #15) > (In reply to Alex Thurgood from comment #14) > >... > > Hi Julien, sorry, I was busy with other stuff yesterday. If the change is in > > master already, I can pull and rebuild on my machine and run the tests again. > > No problem! :-) > Hopefully you can pull and rebuild because there's no daily builds (see > http://nabble.documentfoundation.org/MacOs-Daily-master-builds-lack-of-MacOs- > Jenkins-td4220058.html) Hi Julien, Yes, your fix seems to have done the trick. Those leaks are no longer apparent. However, I notice that at line 80, salgdi.xx : CTFontDescriptorRef pDesc = CTFontCopyFontDescriptor(pFallback); is still leaking big time - perhaps this suffers from the same problem, or should I open a separate bug report ? This happens when you open and scroll down the list of fonts in the dropdown font selector Writer: bool bFound = false; CoreTextStyle rStyle(rPattern); CTFontRef pFont = static_cast<CTFontRef>(CFDictionaryGetValue(rStyle.GetStyleDict(), kCTFontAttributeName)); CFStringRef pStr = CreateCFString(rMissingChars); if (pStr) { CTFontRef pFallback = CTFontCreateForString(pFont, pStr, CFRangeMake(0, CFStringGetLength(pStr))); if (pFallback) { bFound = true; CTFontDescriptorRef pDesc = CTFontCopyFontDescriptor(pFallback); FontAttributes rAttr = DevFontFromCTFontDescriptor(pDesc, nullptr); rPattern.maSearchName = rAttr.GetFamilyName(); rPattern.SetWeight(rAttr.GetWeight()); rPattern.SetItalic(rAttr.GetItalic()); rPattern.SetPitch(rAttr.GetPitch()); rPattern.SetWidthType(rAttr.GetWidthType()); SalData* pSalData = GetSalData(); if (pSalData->mpFontList) rPattern.mpFontData = pSalData->mpFontList->GetFontDataFromId(reinterpret_cast<sal_IntPtr>(pDesc)); CFRelease(pFallback); } CFRelease(pStr); }
(In reply to Alex Thurgood from comment #16) This happens when CoreTextGlyphFallbackSubstitution::FindFontSubstitute() is invoked
I'll close this one as fixed and open a separate report for the other other font management leaks.
Thank you Alex for your feedback. I'll take a look but I suppose it's the exact same cause. I'll put it in See also since it's the same method.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-5-4": http://cgit.freedesktop.org/libreoffice/core/commit/?id=ec60beaf9e73beccf348726d0be4e28f35abe38a&h=libreoffice-5-4 tdf#111432: fix leaks in AddTempDevFont with CFRelease It will be available in 5.4.1. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
*** Bug 99929 has been marked as a duplicate of this bug. ***