GNOME Bugzilla – Bug 780171
Memory leak in object_instance_resolve()
Last modified: 2017-03-19 05:35:04 UTC
Valgrind log from bug 642652: https://paste.gnome.org/pem1d4cvn
Created attachment 348135 [details] [review] object: Fix memory leak in resolve The "name" string, allocated in gjs_get_string_id(), wasn't getting freed at every exit point of the function. (It will be nice to clean this up in 1.49.x with our autoptr classes...) After a certain point in the function it's not needed, so we just free the string there.
Review of attachment 348135 [details] [review]: Looks like it misses another early return, otherwise LGTM ::: gi/object.cpp @@ +792,3 @@ *resolved = true; g_base_info_unref((GIBaseInfo *)vfunc); + g_free(name); There's another exit path right above this block - that'll need another free(), no?
Created attachment 348136 [details] [review] object: Fix memory leak in resolve The "name" string, allocated in gjs_get_string_id(), wasn't getting freed at every exit point of the function. (It will be nice to clean this up in 1.49.x with our autoptr classes...) After a certain point in the function it's not needed, so we just free the string there.
I changed it to g_free(name) inside that block before either of those exit points are reached.
Created attachment 348138 [details] valgrind output run with; valgrind --log-file="valgrind-shell" --tool=memcheck --track-origins=yes --leak-check=full --show-leak-kinds=definite,possible --leak-resolution=med --num-callers=40 gnome-shell -r
For some time now I've been seeing a 150MB an hour leak (which wasn't in gnome 3.22) when the screen is locked but not dimmed/off because I am usually running some application that inhibits 'screensavers'. With this patch, that particular leak is down to ~25MB an hour. This is a major step :) The leak in bug 778368 is still there. Nevertheless this is a massive improvement. Thank you very much for caring to spend time and improve the situation.
Review of attachment 348136 [details] [review]: Looks good to me. Probably worth asking for a freeze break exception for this?
This is embarrassing, but I discovered a case where it would crash, while running under code coverage...
Created attachment 348203 [details] [review] object: Fix memory leak in resolve The "name" string, allocated in gjs_get_string_id(), wasn't getting freed at every exit point of the function. (It will be nice to clean this up in 1.49.x with our autoptr classes...) After a certain point in the function it's not needed, so we just free the string there.
Review of attachment 348203 [details] [review]: ACK
Attachment 348203 [details] pushed as 9050118 - object: Fix memory leak in resolve