[gjs/gtk4-builderscope-memory-leak] arg: Don't sink GClosure ref if it's a return value



commit 26b9b4f092e0b9a35b1f3469cc2ec69cc74703b9
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat May 2 15:09:32 2020 -0700

    arg: Don't sink GClosure ref if it's a return value
    
    This fixes a memory leak when implementing
    GtkBuilderScope.vfunc_create_closure. The caller of vfunc_create_closure
    consumes the floating ref on the GClosure on the C side, by passing it
    to g_signal_connect_closure_by_id().
    
    Since g-i doesn't say whether the ref is supposed to be floating or not,
    we have to guess that GClosures going from JS to C are going to be
    passed to some C API that will consume the floating reference.

 gi/arg.cpp      | 22 +++++++++++++---------
 gi/function.cpp | 11 +++--------
 2 files changed, 16 insertions(+), 17 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 18f8267b..4cadd3ec 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -1520,12 +1520,10 @@ intern_gdk_atom(const char *name,
                            nullptr);
 }
 
-static bool value_to_interface_gi_argument(JSContext* cx, JS::HandleValue value,
-                                           GIBaseInfo* interface_info,
-                                           GIInfoType interface_type,
-                                           GITransfer transfer,
-                                           bool expect_object, GIArgument* arg,
-                                           bool* report_type_mismatch) {
+static bool value_to_interface_gi_argument(
+    JSContext* cx, JS::HandleValue value, GIBaseInfo* interface_info,
+    GIInfoType interface_type, GITransfer transfer, bool expect_object,
+    GIArgument* arg, GjsArgumentType arg_type, bool* report_type_mismatch) {
     g_assert(report_type_mismatch);
     GType gtype = G_TYPE_NONE;
 
@@ -1649,8 +1647,14 @@ static bool value_to_interface_gi_argument(JSContext* cx, JS::HandleValue value,
                 if (g_type_is_a(gtype, G_TYPE_CLOSURE)) {
                     GClosure* closure = gjs_closure_new_marshaled(
                         cx, JS_GetObjectFunction(obj), "boxed");
-                    g_closure_ref(closure);
-                    g_closure_sink(closure);
+                    // GI doesn't know about floating GClosure references. We
+                    // guess that if this is a return value going from JS::Value
+                    // to GArgument, it's intended to be passed to a C API that
+                    // will consume the floating reference.
+                    if (arg_type != GJS_ARGUMENT_RETURN_VALUE) {
+                        g_closure_ref(closure);
+                        g_closure_sink(closure);
+                    }
                     arg->v_pointer = closure;
                     return true;
                 }
@@ -1960,7 +1964,7 @@ gjs_value_to_g_argument(JSContext      *context,
 
             if (!value_to_interface_gi_argument(
                     context, value, interface_info, interface_type, transfer,
-                    expect_object, arg, &report_type_mismatch))
+                    expect_object, arg, arg_type, &report_type_mismatch))
                 wrong = true;
         }
         break;
diff --git a/gi/function.cpp b/gi/function.cpp
index fd71934a..10b254fc 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -418,14 +418,9 @@ static void gjs_callback_closure(ffi_cif* cif G_GNUC_UNUSED, void* result,
             if (!JS_GetElement(context, out_array, elem_idx, &elem))
                 goto out;
 
-            if (!gjs_value_to_g_argument(context,
-                                         elem,
-                                         &ret_type,
-                                         "callback",
-                                         GJS_ARGUMENT_ARGUMENT,
-                                         transfer,
-                                         true,
-                                         &argument))
+            if (!gjs_value_to_g_argument(context, elem, &ret_type, "callback",
+                                         GJS_ARGUMENT_RETURN_VALUE, transfer,
+                                         true, &argument))
                 goto out;
 
             set_return_ffi_arg_from_giargument(&ret_type,


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]