Hi release team,I'd like to request a break of the hard code freeze for https://bugzilla.gnome.org/show_bug.cgi?id=780171 . This is a fix for a memory leak in GJS which has been present in the code since February 15.
I'm the maintainer of GJS, and I am comfortable committing it. It does not break API, change any features, or change any strings.
The memory leak was spotted by Luke Jones yesterday and brought to my attention with a Valgrind log. Both Luke and Hussam Al-Tayeb have confirmed that the fix prevents GNOME Shell from leaking hundreds of megabytes of memory per hour. Therefore, I believe this fix is necessary to improve the stability of the GNOME 3.24 release.Cosimo Cecchi has reviewed the patch and Florian Müllner already reviewed a previous version. Due to the short time, it has not undergone extensive testing, but I have been running GJS with the patch since yesterday. In addition, the fix is in a function that is called thousands of times during a run of GJS's test suite. I have determined this using code coverage tools. So if it were going to cause crashes, it would be apparent, and by this method I did already discover and fix a mistake in an earlier version of the patch. (The tests don't yet cover all the function's exits, but the uncovered paths are exceptional and unlikely to come up in normal usage.)
Best,Philip CPatch follows:From 9050118c9e77849a29d0193aea8f710038bfd5fa Mon Sep 17 00:00:00 2001 From: Philip Chimento <philip endlessm com>Date: Thu, 16 Mar 2017 15:43:09 -0700Subject: [PATCH] object: Fix memory leak in resolveThe "name" string, allocated in gjs_get_string_id(), wasn't getting freedat every exit point of the function. (It will be nice to clean this up in1.49.x with our autoptr classes...)After a certain point in the function it's not needed, so we just freethe string there.---gi/object.cpp | 15 +++++++++++----1 file changed, 11 insertions(+), 4 deletions(-)diff --git a/gi/object.cpp b/gi/object.cppindex c68ad4c6..63731df5 100644--- a/gi/object.cpp+++ b/gi/object.cpp@@ -607,7 +607,7 @@ is_vfunc_unchanged(GIVFuncInfo *info,static GIVFuncInfo *find_vfunc_on_parents(GIObjectInfo *info, - gchar *name,+ const char *name,bool *out_defined_by_parent){GIVFuncInfo *vfunc = NULL;@@ -774,12 +774,14 @@ object_instance_resolve(JSContext *context, * rest.*/- gchar *name_without_vfunc_ = &name[6];+ const char *name_without_vfunc_ = &name[6]; /* lifetime tied to name */GIVFuncInfo *vfunc;bool defined_by_parent;vfunc = find_vfunc_on_parents(priv->info, name_without_vfunc_, &defined_by_parent); if (vfunc != NULL) {+ g_free(name);+/* In the event that the vfunc is unchanged, let regular* prototypal inheritance take over. */if (defined_by_parent && is_vfunc_unchanged(vfunc, priv->gtype)) {@@ -818,8 +820,13 @@ object_instance_resolve(JSContext *context, * this could be done better. See*/- if (method_info == NULL)- return object_instance_resolve_no_info(context, obj, resolved, priv, name); + if (method_info == NULL) {+ bool retval = object_instance_resolve_no_info(context, obj, resolved, priv, name); + g_free(name);+ return retval;+ }++ g_free(name);#if GJS_VERBOSE_ENABLE_GI_USAGE_gjs_log_info_usage((GIBaseInfo*) method_info);