Description: sd::TemplateScanner::ScanEntry() leaks memory (or rather, doesn't release memory correctly) Steps to Reproduce: 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. 6. Click on the Impress button to load a new Impress document. 7. Cancel the template selection to accept a default blank template. 8. In the sidebar, click on the Switch Master Template button. 9. Close the Impress document without saving. 10. Stop recording Actual Results: Notice how sd::TemplateScanner::ScanEntry() allocates multiple memory blocks, but doesn't release them: Malloc 16 Bytes 12 < multiple > 192 Bytes libsdlo.dylib sd::TemplateScanner::ScanEntry() Expected Results: It should not leak memory 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
sd::TemplateScanner::ScanEntry() is invoked from TemplateScanner.cxx at line 242 in the following block : { // Check whether the entry is an impress template. If so // add a new entry to the resulting list (which is created // first if necessary). // These strings are used to find impress templates in the tree of // template files. Should probably be determined dynamically. if ( (sContentType == MIMETYPE_OASIS_OPENDOCUMENT_PRESENTATION_TEMPLATE_ASCII) || (sContentType == MIMETYPE_OASIS_OPENDOCUMENT_PRESENTATION_ASCII) || (sContentType == "application/vnd.stardivision.impress") || (sContentType == MIMETYPE_VND_SUN_XML_IMPRESS_ASCII) // The following id comes from the bugdoc in #i2764#. || (sContentType == "Impress 2.0")) { OUString sLocalisedTitle = SfxDocumentTemplates::ConvertResourceString(sTitle); mpLastAddedEntry = new TemplateEntry(sLocalisedTitle, sTargetURL); mpTemplateDirectory->InsertEntry(mpLastAddedEntry); } }
Confirming with 5.4.0.3
Julien Nabet committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=fe195f2c7b0c22169f265b731981bef47119f166 Related tdf#111892: revamp TemplateScanner 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: I'm not sure it'll remove the leak but at least, the code will be simpler to check/debug. I let you try with the new code. If it really helps for the leak, I'll try to backport it on 5.4 branch.
Alex: any better with a build including http://cgit.freedesktop.org/libreoffice/core/commit/?id=fe195f2c7b0c22169f265b731981bef47119f166 ?
(In reply to Julien Nabet from comment #5) > Alex: any better with a build including > http://cgit.freedesktop.org/libreoffice/core/commit/ > ?id=fe195f2c7b0c22169f265b731981bef47119f166 ? I haven't rebuilt in a few days, will start a build again and test to report back.
(In reply to Julien Nabet from comment #4) > Alex: I'm not sure it'll remove the leak but at least, the code will be > simpler to check/debug. I let you try with the new code. If it really helps > for the leak, I'll try to backport it on 5.4 branch. Hi Julien, It still seems to allocate without releasing, but in a different place, now at line 198 TemplateScanner.cxx : mpTemplateEntries.push_back(new TemplateEntry(sLocalisedTitle, sTargetURL));
Sorry Alex, I gave a new try but it was wrong https://gerrit.libreoffice.org/#/c/41650/ It seems I can't help here.
Noel: following abandon of https://gerrit.libreoffice.org/#/c/41650/, if I remember what you told about vector members, during destruction of the object, vectors are automatically cleared. When they're cleared, their objects are cleared too. But here, since we've got a vector of pointers of TemplateEntry objects (see https://opengrok.libreoffice.org/xref/core/sd/source/ui/inc/TemplateScanner.hxx#115), shouldn't we add a loop to call delete on each entry of the vector during destruction? (https://opengrok.libreoffice.org/xref/core/sd/source/ui/dlg/TemplateScanner.cxx#128)
(In reply to Julien Nabet from comment #9) > But here, since we've got a vector of pointers of TemplateEntry objects (see > https://opengrok.libreoffice.org/xref/core/sd/source/ui/inc/TemplateScanner. > hxx#115), shouldn't we add a loop to call delete on each entry of the vector > during destruction? > (https://opengrok.libreoffice.org/xref/core/sd/source/ui/dlg/TemplateScanner. > cxx#128) Yes, it looks like the TemplateEntry objects are being leaked. As near as I can tell, maFolderList is not doing anything useful currently, and can be removed. After that, mpTemplateEntries can be converted to std::vector<std::unique_ptr<T>>
I restored the patch and updated it, so let's wait for review now.
Julien Nabet committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=864f89e92e31c239afdea5d429e5f569b3894d1a Related tdf#111892: use unique_ptr 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: waiting for your feedback for backport (I'll have to merge both patches).
Indeed fixed when tested against Version: 6.0.0.0.alpha0+ Build ID: 595371e520ce4f64ad9d99a7866bdb8404271b6e CPU threads: 4; OS: Mac OS X 10.12.6; UI render: default; Locale: fr-FR (fr_FR.UTF-8); Calc: group Thanks Julien !
Sorry it seems 5.4 branch is too different from master to backport the fix.