316449 2013-03-10 06:56:11 +0000 Large memory leak when updating entries via data sources 2013-03-12 03:58:29 +0000 1 1 2 Applications tellico general 2.3.7 Unlisted Binaries Linux RESOLVED FIXED NOR major --- 0 dominik.stadler robby http://commits.kde.org/tellico/301ed4cce8a4c4aafa46de647bcfc1d0d55d4306 2.3.8 0 oldest_to_newest 1349382 0 dominik.stadler 2013-03-10 06:56:11 +0000 I have a large collection and when I update many entries via data sources in one go, I see a steady increase in memory usage until available memory runs out and finally the linux-oom-killer kicks in and forcefully closes tellico. I see aprox. 1MB of memory leak per 10 entries being updated. Reproducible: Always Steps to Reproduce: 1. Open a sufficiently large collection of books 2. Select many entries and choose "update entries -> from all sources" 3. Wait until memory runs out Actual Results: tellico crashes/is killed by the linux oom-killer Expected Results: memory should keep steady 1349703 1 dominik.stadler 2013-03-10 23:25:24 +0000 I did some investigation and I have two suspects here: * xslthandler.cpp:233: The members m_docIn and m_docOut are only freed in the destructor, but overwritten with newly allocated objects, thus leaking two XML documents for every XSLT parsing. I am currently testing a patch which get's rid of the members m_docIn and m_docOut completely and just keeps the documents locally and frees them immediately after the parse, they are not used any more anyway and thus it is generally a good idea to throw them away quickly rather than keeping them around. * I don't know much about KDE's signals, but it looks to me like signalResultFound leaks FetchResults if the FetchDialog is not shown. When updating entries, the dialog is not displayed and thus the slot for "ResultFound" is not connected. However each fetcher allocates a FetchResult and passes it to the signal. I could not yet verify that it actually leaks, but valgrind reports some lost memory around that area... FetchResult* r = new FetchResult(...); emit signalResultFound(r); I'll update here with patches if my changes work for my collections. 1349728 2 robby 2013-03-11 00:50:24 +0000 (In reply to comment #1) > * xslthandler.cpp:233: The members m_docIn and m_docOut are only freed in > the destructor, but overwritten with newly allocated objects, thus leaking > two XML documents for every XSLT parsing. > > I am currently testing a patch which get's rid of the members m_docIn and > m_docOut completely and just keeps the documents locally and frees them > immediately after the parse, they are not used any more anyway and thus it > is generally a good idea to throw them away quickly rather than keeping them > around. Sound appropriate. I did some refactoring of that class a while back and probably didn't realize I could potentially make m_docIn and m_docOut local variables. > * I don't know much about KDE's signals, but it looks to me like > signalResultFound leaks FetchResults if the FetchDialog is not shown. > When updating entries, the dialog is not displayed and thus the slot for > "ResultFound" is not connected. However each fetcher allocates a FetchResult > and passes it to the signal. I could not yet verify that it actually leaks, > but valgrind reports some lost memory around that area... > > FetchResult* r = new FetchResult(...); > emit signalResultFound(r); Even though the dialog isn't shown, the updater still gets the new information from signalResultFound(). It could certainly be leaking, but it's not just because the dialog isn't used. The update job may not be deleting something that it should. > I'll update here with patches if my changes work for my collections. Absolutely, thanks! Just in general, the fetch jobs aren't really written with large update loops in mind, but any memory leaks should definitely be fixed. The update job loop isn't all that efficient...one of these days, I may need to learn threads, too... 1349990 3 77962 dominik.stadler 2013-03-11 18:08:26 +0000 Created attachment 77962 Fix large memory leak for every xslt transformation that is done, e.g. as part of fetches I tested with the attached patch to free up the memory for the XML documents as early as possible. With this in place memory increase is a lot less, maybe even zero, fetching many entries at once now works for up to 1000 entries without much memory increase. I will probably take another look which items are still reported by valgrind now, but for my usage it should already suffice for now. 1350117 4 robby 2013-03-12 03:58:29 +0000 Git commit 301ed4cce8a4c4aafa46de647bcfc1d0d55d4306 by Robby Stephenson. Committed on 12/03/2013 at 04:56. Pushed by rstephenson into branch '2.3'. Free xmlDoc memory as soon as possible in XSLTHandler Make the xmlDoc pointers local and free them as soon as possible instead of waiting for the XSLTHandler destructor Patch from Dominik Stadler FIXED-IN: 2.3.8 M +20 -24 src/translators/xslthandler.cpp M +1 -3 src/translators/xslthandler.h http://commits.kde.org/tellico/301ed4cce8a4c4aafa46de647bcfc1d0d55d4306 77962 2013-03-11 18:08:26 +0000 2013-03-11 18:08:26 +0000 Fix large memory leak for every xslt transformation that is done, e.g. as part of fetches tellico-Bug-316449-XSLTHandler-Memory-Leak.patch text/plain 3116 dominik.stadler ZGlmZiAtLWdpdCBhL3NyYy90cmFuc2xhdG9ycy94c2x0aGFuZGxlci5jcHAgYi9zcmMvdHJhbnNs YXRvcnMveHNsdGhhbmRsZXIuY3BwCmluZGV4IDA0YmI5Y2YuLmY1YzcxZmYgMTAwNjQ0Ci0tLSBh L3NyYy90cmFuc2xhdG9ycy94c2x0aGFuZGxlci5jcHAKKysrIGIvc3JjL3RyYW5zbGF0b3JzL3hz bHRoYW5kbGVyLmNwcApAQCAtODAsOSArODAsNyBAQCBYU0xUSGFuZGxlcjo6WE1MT3V0cHV0QnVm ZmVyOjp+WE1MT3V0cHV0QnVmZmVyKCkgewogaW50IFhTTFRIYW5kbGVyOjpzX2luaXRDb3VudCA9 IDA7CiAKIFhTTFRIYW5kbGVyOjpYU0xUSGFuZGxlcihjb25zdCBRQnl0ZUFycmF5JiB4c2x0Rmls ZV8pIDoKLSAgICBtX3N0eWxlc2hlZXQoMCksCi0gICAgbV9kb2NJbigwKSwKLSAgICBtX2RvY091 dCgwKSB7CisgICAgbV9zdHlsZXNoZWV0KDApIHsKICAgaW5pdCgpOwogICBRQnl0ZUFycmF5IGZp bGUgPSBRVXJsOjp0b1BlcmNlbnRFbmNvZGluZyhRU3RyaW5nOjpmcm9tTG9jYWw4Qml0KHhzbHRG aWxlXykpOwogICBpZighZmlsZS5pc0VtcHR5KCkpIHsKQEAgLTk3LDkgKzk1LDcgQEAgWFNMVEhh bmRsZXI6OlhTTFRIYW5kbGVyKGNvbnN0IFFCeXRlQXJyYXkmIHhzbHRGaWxlXykgOgogfQogCiBY U0xUSGFuZGxlcjo6WFNMVEhhbmRsZXIoY29uc3QgS1VybCYgeHNsdFVSTF8pIDoKLSAgICBtX3N0 eWxlc2hlZXQoMCksCi0gICAgbV9kb2NJbigwKSwKLSAgICBtX2RvY091dCgwKSB7CisgICAgbV9z dHlsZXNoZWV0KDApIHsKICAgaW5pdCgpOwogICBpZih4c2x0VVJMXy5pc1ZhbGlkKCkgJiYgeHNs dFVSTF8uaXNMb2NhbEZpbGUoKSkgewogICAgIHhtbERvY1B0ciB4c2x0RG9jID0geG1sUmVhZEZp bGUoeHNsdFVSTF8uZW5jb2RlZFBhdGhBbmRRdWVyeSgpLnRvVXRmOCgpLCBOVUxMLCB4c2x0X29w dGlvbnMpOwpAQCAtMTEzLDkgKzEwOSw3IEBAIFhTTFRIYW5kbGVyOjpYU0xUSGFuZGxlcihjb25z dCBLVXJsJiB4c2x0VVJMXykgOgogfQogCiBYU0xUSGFuZGxlcjo6WFNMVEhhbmRsZXIoY29uc3Qg UURvbURvY3VtZW50JiB4c2x0RG9jXywgY29uc3QgUUJ5dGVBcnJheSYgeHNsdEZpbGVfLCBib29s IHRyYW5zbGF0ZV8pIDoKLSAgICBtX3N0eWxlc2hlZXQoMCksCi0gICAgbV9kb2NJbigwKSwKLSAg ICBtX2RvY091dCgwKSB7CisgICAgbV9zdHlsZXNoZWV0KDApIHsKICAgaW5pdCgpOwogICBRQnl0 ZUFycmF5IGZpbGUgPSBRVXJsOjp0b1BlcmNlbnRFbmNvZGluZyhRU3RyaW5nOjpmcm9tTG9jYWw4 Qml0KHhzbHRGaWxlXykpOwogICBpZigheHNsdERvY18uaXNOdWxsKCkgJiYgIWZpbGUuaXNFbXB0 eSgpKSB7CkBAIC0xMjgsMTQgKzEyMiw2IEBAIFhTTFRIYW5kbGVyOjp+WFNMVEhhbmRsZXIoKSB7 CiAgICAgeHNsdEZyZWVTdHlsZXNoZWV0KG1fc3R5bGVzaGVldCk7CiAgIH0KIAotICBpZihtX2Rv Y0luKSB7Ci0gICAgeG1sRnJlZURvYyhtX2RvY0luKTsKLSAgfQotCi0gIGlmKG1fZG9jT3V0KSB7 Ci0gICAgeG1sRnJlZURvYyhtX2RvY091dCk7Ci0gIH0KLQogICAtLXNfaW5pdENvdW50OwogICBp ZihzX2luaXRDb3VudCA9PSAwKSB7CiAgICAgeHNsdFVucmVnaXN0ZXJFeHRNb2R1bGUoRVhTTFRf U1RSSU5HU19OQU1FU1BBQ0UpOwpAQCAtMjMwLDEyICsyMTYsMTMgQEAgUVN0cmluZyBYU0xUSGFu ZGxlcjo6YXBwbHlTdHlsZXNoZWV0KGNvbnN0IFFTdHJpbmcmIHRleHRfKSB7CiAgICAgcmV0dXJu IFFTdHJpbmcoKTsKICAgfQogCisgIHhtbERvY1B0ciBtX2RvY0luOwogICBtX2RvY0luID0geG1s UmVhZERvYyhyZWludGVycHJldF9jYXN0PHhtbENoYXIqPih0ZXh0Xy50b1V0ZjgoKS5kYXRhKCkp LCBOVUxMLCBOVUxMLCB4bWxfb3B0aW9ucyk7CiAKLSAgcmV0dXJuIHByb2Nlc3MoKTsKKyAgcmV0 dXJuIHByb2Nlc3MobV9kb2NJbik7CiB9CiAKLVFTdHJpbmcgWFNMVEhhbmRsZXI6OnByb2Nlc3Mo KSB7CitRU3RyaW5nIFhTTFRIYW5kbGVyOjpwcm9jZXNzKHhtbERvY1B0ciBtX2RvY0luKSB7CiAg IGlmKCFtX2RvY0luKSB7CiAgICAgbXlEZWJ1ZygpIDw8ICJlcnJvciBwYXJzaW5nIGlucHV0IHN0 cmluZyEiOwogICAgIHJldHVybiBRU3RyaW5nKCk7CkBAIC0yNTIsMTAgKzIzOSwxNSBAQCBRU3Ry aW5nIFhTTFRIYW5kbGVyOjpwcm9jZXNzKCkgewogICAgIGkgKz0gMjsKICAgfQogICAvLyByZXR1 cm5zIE5VTEwgb24gZXJyb3IKKyAgeG1sRG9jUHRyIG1fZG9jT3V0OwogICBtX2RvY091dCA9IHhz bHRBcHBseVN0eWxlc2hlZXQobV9zdHlsZXNoZWV0LCBtX2RvY0luLCBwYXJhbXMuZGF0YSgpKTsK ICAgZm9yKGludCBpID0gMDsgaSA8IDIqbV9wYXJhbXMuY291bnQoKTsgKytpKSB7CiAgICAgZGVs ZXRlW10gcGFyYW1zW2ldOwogICB9CisgIAorICB4bWxGcmVlRG9jKG1fZG9jSW4pOworICBtX2Rv Y0luID0gMDsKKwogICBpZighbV9kb2NPdXQpIHsKICAgICBteURlYnVnKCkgPDwgImVycm9yIGFw cGx5aW5nIHN0eWxlc2hlZXQhIjsKICAgICByZXR1cm4gUVN0cmluZygpOwpAQCAtMjY4LDYgKzI2 MCwxMCBAQCBRU3RyaW5nIFhTTFRIYW5kbGVyOjpwcm9jZXNzKCkgewogICAgICAgbXlEZWJ1Zygp IDw8ICJlcnJvciBzYXZpbmcgb3V0cHV0IGJ1ZmZlciEiOwogICAgIH0KICAgfQorCisgIHhtbEZy ZWVEb2MobV9kb2NPdXQpOworICBtX2RvY091dCA9IDA7CisgIAogICByZXR1cm4gb3V0cHV0LnJl c3VsdCgpOwogfQogCmRpZmYgLS1naXQgYS9zcmMvdHJhbnNsYXRvcnMveHNsdGhhbmRsZXIuaCBi L3NyYy90cmFuc2xhdG9ycy94c2x0aGFuZGxlci5oCmluZGV4IGNkNTYyMjcuLjdlODc1ZWQgMTAw NjQ0Ci0tLSBhL3NyYy90cmFuc2xhdG9ycy94c2x0aGFuZGxlci5oCisrKyBiL3NyYy90cmFuc2xh dG9ycy94c2x0aGFuZGxlci5oCkBAIC0xMDgsMTEgKzEwOCw5IEBAIHB1YmxpYzoKIAogcHJpdmF0 ZToKICAgdm9pZCBpbml0KCk7Ci0gIFFTdHJpbmcgcHJvY2VzcygpOworICBRU3RyaW5nIHByb2Nl c3MoeG1sRG9jUHRyIG1fZG9jSW4pOwogCiAgIHhzbHRTdHlsZXNoZWV0UHRyIG1fc3R5bGVzaGVl dDsKLSAgeG1sRG9jUHRyIG1fZG9jSW47Ci0gIHhtbERvY1B0ciBtX2RvY091dDsKIAogICBRSGFz aDxRQnl0ZUFycmF5LCBRQnl0ZUFycmF5PiBtX3BhcmFtczsKIAo=