[gjs/gtk4-builderscope-memory-leak] arg: Don't sink GClosure ref if it's a return value
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs/gtk4-builderscope-memory-leak] arg: Don't sink GClosure ref if it's a return value
- Date: Sat, 2 May 2020 22:10:12 +0000 (UTC)
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]