GNOME Bugzilla – Bug 736774
Memory leak with Vala 0.25.4
Last modified: 2017-08-25 12:49:52 UTC
After recompiling Geary with Vala 0.25.4, it will quickly grow to consume system memory while running. This didn't occur with 0.25.3.
Another problem I've seen is repeated "unable to open database file" errors in the log. This takes some time to happen, however. I suspect it's an out-of-memory issue, not a bona fide SQLite problem.
Using git bisect I've determined the problem is with the commit at https://git.gnome.org/browse/vala/commit/?id=ba1fa0759989dcbb9046c7dfc06cce6c4aa23411 Unfortunately, I don't have a minimal test case at the moment. I know the issues likes there simply by watching Geary's memory usage over a short period of time. I also don't know if this is a Geary problem or a compiler issue; it's possible Geary is doing something wrong with owned references that was hidden or masked by a compiler bug that's now been corrected.
The associated bug for that commit is bug #736016.
I've tried some usage of geary 0.6.3 with both vala versions, but I couldn't find anything evident. What should I do to reproduce the problem?
I'm doing this on a Gmail account. It's easiest to see if you have an account with a lot of mail and an All Mail folder. * Open System Monitor or top, whatever you prefer. * Start Geary and compose a new message. * Type a few characters and let the composer auto-save your draft. Type more, let it save. Repeat a number of times. * Unfortunately, now you have to wait a few minutes. In the background, Geary will auto-synchronize with All Mail. If you run Geary like this: $ geary --debug You will see similar lines in the log while it's synchronizing with All Mail: [deb] 11:16:34 0.000931 imap-engine-minimal-folder.vala:123: Gmail:jim@yorba.org:[Gmail]/All Mail: Begin normalizing remote and local folders ** [deb] 11:16:34 0.041060 imap-engine-minimal-folder.vala:252: Gmail:jim@yorba.org:[Gmail]/All Mail: Messages appended/removed (local/remote UIDNEXT=157415/157416 total=109011/109011 diff=1), gathering mail UIDs 84793:157415 [deb] 11:16:41 7.085519 imap-engine-minimal-folder.vala:275: Gmail:jim@yorba.org:[Gmail]/All Mail: Loaded local (52659) and remote (52667) UIDs, normalizing... [deb] 11:16:41 0.092151 imap-engine-minimal-folder.vala:314: Gmail:jim@yorba.org:[Gmail]/All Mail: changes since last seen: removed=0 appended=0 inserted=8 [deb] 11:16:41 0.305100 imap-engine-minimal-folder.vala:367: Gmail:jim@yorba.org:[Gmail]/All Mail: Finished creating/merging 8 emails [deb] 11:16:41 0.031220 imap-engine-minimal-folder.vala:411: Gmail:jim@yorba.org:[Gmail]/All Mail: Notifying of 8 inserted emails since last opened [deb] 11:16:41 0.000051 imap-engine-minimal-folder.vala:446: Gmail:jim@yorba.org:[Gmail]/All Mail: Notifying of 2h count change reason (109011 remote messages) [deb] 11:16:41 0.000015 imap-engine-minimal-folder.vala:451: Gmail:jim@yorba.org:[Gmail]/All Mail: Completed normalize_folder There's a delay after the one marked with two stars. During that moment, Geary's memory usage drives up from ~150MB to ~300MB. If you keep performing the test, memory usage will continue to drive upward. This doesn't happen under the prior commit of Vala. I wish I had an easier way to repro the problem, but that's the best I can do for now. I'm looking into the code path that's followed in between the two log calls and don't see anything obvious. If I can narrow down the issue, I will let you know.
Will retry tomorrow. Can you run with valgrind?
Alright, I pinpointed it. It is a very particular bug. It turns out there are several hot code paths in Geary which use this code pattern, leading to an aggregate number of memory leaks and not one source in particular. I'll attach a test case which I developed. In essence, with 0.25.4, if you have a factory method that throws an Error and the returned object is passed directly to another method, the interim reference is not unref'd. So: Object create() { return new Object(); } Object create_throws() throws Error { return new Object(); } void func(Object o) { } void main() { func(create()); func(create_throws()); } ... the Object from create_throws() will be dangling when the app exits. The interim reference *must* be passed directly to a method. There are no dangling references in these forms: create_throws(); Object o = create_throws(); func(o); If my test program is built with Vala as of the commit just prior to ba1fa075, all references are dropped properly.
Created attachment 286430 [details] Test program This is a verbose test app which demonstrates the bug. In particular, the program should print "6" and it does not. Examining the C code, I can see that the interim reference is being NULL'd prior to being unref'd.
Can you try the bug736774 branch and check whether your software works correctly?
As soon as I run Geary I get a fasttop assertion: *** Error in `./geary': double free or corruption (fasttop): 0x00000000012c5da0 *** Stack trace:
+ Trace 234108
Looking at the generated .c: _tmp10_ = g_file_get_path (_tmp9_); _tmp8_ = _tmp10_; _tmp11_ = _tmp8_; _tmp12_ = _tmp11_ == NULL; _g_free0 (_tmp11_); if (_tmp12_) { gchar* _tmp13_ = NULL; _tmp13_ = g_strdup (""); _g_free0 (_tmp8_); _tmp8_ = _tmp13_; } _tmp14_ = key_file; _tmp15_ = _tmp8_; g_key_file_load_from_file (_tmp14_, _tmp15_, G_KEY_FILE_NONE, &_inner_error_); _g_free0 (_tmp15_); _tmp10 is being triple-freed (aliased as _tmp11_, _tmp8_, and _tmp15_).
Thanks for testing. Can you show me the relevant vala code? Btw it's double-freed.
Just caught that, my analysis wasn't quite right, there is no triple-free, but there is a double-free in both code paths. That line of Vala is this (src/engine/api/geary-account-information.vala:138): key_file.load_from_file(file.get_path() ?? "", KeyFileFlags.NONE); Maybe the ?? operator is causing the issue?
Yes, I thought I fixed that. Perhaps it's some other case. This thing is trickier than what it seems. Will work on it tomorrow, thanks.
I've reverted: commit c08dd2c82dcfe42f2f6fb23333eadf63ed2d7f22 Author: Luca Bruno <luca.bruno@immobiliare.it> Date: Fri Sep 19 14:43:45 2014 +0200 Revert "Fix regression when assigning owned expressions to unowned variables." This reverts commit ba1fa0759989dcbb9046c7dfc06cce6c4aa23411. Due to bug #736774 To be sure we don't release a vala stable with this commit in case I'm not able to fix it in time.
Created attachment 339681 [details] [review] Fix regression introduced by ba1fa07 Hope this is the last time we touch this code. In the owned->unowned case, we want the variable to be floating: - If we assign to a local variable we get an error. - Otherwise the value is destroyed when goes out of scope. In the owned->owned case, we want to avoid copies and thus we transfer the ownership and make the variable non-floating. Fixes bug 736774
*** Bug 786388 has been marked as a duplicate of this bug. ***
Created attachment 358083 [details] [review] codegen: Don't transfer ownership of local-variable if target-type is unknown
Attachment 358083 [details] pushed as 1d867ed - codegen: Don't transfer ownership of local-variable if target-type is unknown