Created attachment 190214 [details] [review]
Function: plug a memory leak

g_callable_info_prepare_closure() allocates memory for the argument
types stored in the ffi_cif, so we need to free it.

Created attachment 190215 [details] [review]
Update suppression file

For better valgrind results

With this, valgrinding gjs-unit -p /js/EverythingBasic shows no errors.

Review of attachment 190215 [details] [review]:

::: test/gjs.supp
@@ +30,3 @@
+   Memcheck:Leak
+   ...
+   fun:g_slice_alloc

You should always run valgrind with G_SLICE=always-malloc in the environment; make valgrind-check already does this.

Review of attachment 190214 [details] [review]:

This one confuses me; I don't see g_callable_info_prepare_closure() g_malloc()ing anything.  Note the "ffi_cif cif" structure is embedded in the GjsCallbackTrampoline structure.

(In reply to comment #4)
> Review of attachment 190214 [details] [review]:
> 
> This one confuses me; I don't see g_callable_info_prepare_closure()
> g_malloc()ing anything.  Note the "ffi_cif cif" structure is embedded in the
> GjsCallbackTrampoline structure.

g_callable_info_get_ffi_arg_types() allocates an array of ffi_types*, and ffi_prep_cif() embeds it into ffi_cif

Created attachment 190293 [details] [review]
Update suppression file

For better valgrind results

(In reply to comment #5)
> (In reply to comment #4)
> > Review of attachment 190214 [details] [review] [details]:
> > 
> > This one confuses me; I don't see g_callable_info_prepare_closure()
> > g_malloc()ing anything.  Note the "ffi_cif cif" structure is embedded in the
> > GjsCallbackTrampoline structure.
> 
> g_callable_info_get_ffi_arg_types() allocates an array of ffi_types*, and
> ffi_prep_cif() embeds it into ffi_cif

Ah, you're right.  But then the bug is in gobject-introspection; we should free the types inside g_callable_info_free_closure().

Review of attachment 190293 [details] [review]:

Looks OK.  Sort of unfortunate about the fontconfig/gettext stuff; maybe it'd be worth trying to get a "standard" glib suppression file that's actually maintained, but whatever.  Let's commit this for now and move on =)

Comment on attachment 190293 [details] [review]
Update suppression file

Attachment 190293 [details] pushed as 8eb3716 - Update suppression file

Created attachment 190311 [details] [review]
Free allocated ffi_types in g_callable_info_free_closure()

g_callable_info_prepare_closure() allocates memory for the argument
types in the ffi_cif, so we need to free it.

The same fix, this time in gobject-introspection. Probably we need
to warn pygobject devs if this is merged.

Review of attachment 190311 [details] [review]:

Looks good.

I don't see pygobject freeing the args offhand.

Attachment 190311 [details] pushed as 845ccc9 - Free allocated ffi_types in g_callable_info_free_closure()