From 223c5319649947f96f0cf0d56df26e59e5568b59 Mon Sep 17 00:00:00 2001 From: Michal Hruby Date: Tue, 20 Mar 2018 16:50:14 +0100 Subject: codegen: Stop taking explicit references on 'this' for captured blocks Use g_signal_connect_closure() and g_object_watch_closure() in GObject environments to control their life-cycles. https://bugzilla.gnome.org/show_bug.cgi?id=624624 --- codegen/valaccodebasemodule.vala | 19 +---------- codegen/valaccodemethodcallmodule.vala | 6 +--- codegen/valagsignalmodule.vala | 48 +++++++++++++++++++++++++--- tests/Makefile.am | 1 + tests/objects/bug624624.vala | 58 ++++++++++++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 27 deletions(-) create mode 100644 tests/objects/bug624624.vala diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala index 0d8629e67..5ff51e6f2 100644 --- a/codegen/valaccodebasemodule.vala +++ b/codegen/valaccodebasemodule.vala @@ -2024,13 +2024,7 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator { bool in_creation_method_with_chainup = (current_method is CreationMethod && current_class != null && current_class.base_class != null); if (get_this_type () != null && (!in_creation_method_with_chainup || current_method.body != b)) { - var ref_call = new CCodeFunctionCall (get_dup_func_expression (get_data_type_for_symbol (current_type_symbol), b.source_reference)); - ref_call.add_argument (get_result_cexpression ("self")); - - // never increase reference count for self in finalizers to avoid infinite recursion on following unref - var instance = (is_in_destructor () ? (CCodeExpression) new CCodeIdentifier ("self") : (CCodeExpression) ref_call); - - ccode.add_assignment (new CCodeMemberAccess.pointer (get_variable_cexpression ("_data%d_".printf (block_id)), "self"), instance); + ccode.add_assignment (new CCodeMemberAccess.pointer (get_variable_cexpression ("_data%d_".printf (block_id)), "self"), get_result_cexpression ("self")); } if (current_method != null) { @@ -2214,17 +2208,6 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator { unref_call.add_argument (new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data%d_".printf (block_id)), "_data%d_".printf (parent_block_id))); ccode.add_expression (unref_call); ccode.add_assignment (new CCodeMemberAccess.pointer (new CCodeIdentifier ("_data%d_".printf (block_id)), "_data%d_".printf (parent_block_id)), new CCodeConstant ("NULL")); - } else { - var this_type = get_this_type (); - if (this_type != null) { - this_type = this_type.copy (); - this_type.value_owned = true; - if (this_type.is_disposable () && !is_in_destructor ()) { - // reference count for self is not increased in finalizers - var this_value = new GLibValue (get_data_type_for_symbol (current_type_symbol), new CCodeIdentifier ("self"), true); - ccode.add_expression (destroy_value (this_value)); - } - } } var data_free = new CCodeFunctionCall (new CCodeIdentifier ("g_slice_free")); diff --git a/codegen/valaccodemethodcallmodule.vala b/codegen/valaccodemethodcallmodule.vala index 1beaa8ba4..6ab1597bb 100644 --- a/codegen/valaccodemethodcallmodule.vala +++ b/codegen/valaccodemethodcallmodule.vala @@ -315,11 +315,7 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule { ccode.add_assignment (get_this_cexpression (), new CCodeCastExpression (ccall, get_ccode_name (current_class) + "*")); if (current_method.body.captured) { - // capture self after setting it - var ref_call = new CCodeFunctionCall (get_dup_func_expression (new ObjectType (current_class), expr.source_reference)); - ref_call.add_argument (get_this_cexpression ()); - - ccode.add_assignment (new CCodeMemberAccess.pointer (get_variable_cexpression ("_data%d_".printf (get_block_id (current_method.body))), "self"), ref_call); + ccode.add_assignment (new CCodeMemberAccess.pointer (get_variable_cexpression ("_data%d_".printf (get_block_id (current_method.body))), "self"), get_this_cexpression ()); } if (!current_class.is_compact && current_class.get_type_parameters ().size > 0) { diff --git a/codegen/valagsignalmodule.vala b/codegen/valagsignalmodule.vala index ba4b4d11e..4f99941d6 100644 --- a/codegen/valagsignalmodule.vala +++ b/codegen/valagsignalmodule.vala @@ -623,7 +623,9 @@ public class Vala.GSignalModule : GObjectModule { else connect_func = get_dynamic_signal_connect_after_wrapper_name ((DynamicSignal) sig); } else { - if ((m != null && m.closure) || (dt != null && dt.value_owned)) { + if ((m != null && m.closure) && in_gobject_instance (m)) { + connect_func = "g_signal_connect_closure"; + } else if ((m != null && m.closure) || (dt != null && dt.value_owned)) { connect_func = "g_signal_connect_data"; } else if (m != null && in_gobject_instance (m)) { connect_func = "g_signal_connect_object"; @@ -721,12 +723,41 @@ public class Vala.GSignalModule : GObjectModule { ccall.add_argument (new CCodeConstant ("NULL")); } - // third resp. sixth argument: handler - ccall.add_argument (new CCodeCastExpression (get_cvalue (handler), "GCallback")); + if ((m != null && m.closure) && in_gobject_instance (m)) { + // g_signal_connect_closure - if (m != null && m.closure) { + // third argument: closure + var closure_var = get_temp_variable (new CType ("GClosure*"), false, null, false); + var closure_ref = get_variable_cexpression (closure_var.name); + emit_temp_var (closure_var); + + var closure_call = new CCodeFunctionCall (new CCodeIdentifier ("g_cclosure_new")); + // callback_func + closure_call.add_argument (new CCodeCastExpression (get_cvalue (handler), "GCallback")); + // user_data + CCodeExpression handler_destroy_notify; + closure_call.add_argument (get_delegate_target_cexpression (handler, out handler_destroy_notify)); + // destroy_data + closure_call.add_argument (new CCodeCastExpression (handler_destroy_notify, "GClosureNotify")); + + ccode.add_assignment (closure_ref, closure_call); + + var watch_call = new CCodeFunctionCall (new CCodeIdentifier ("g_object_watch_closure")); + watch_call.add_argument (new CCodeCastExpression (get_result_cexpression ("self"), "GObject *")); + watch_call.add_argument (closure_ref); + + ccode.add_expression (watch_call); + + ccall.add_argument (closure_ref); + + // fourth argument: after? + ccall.add_argument (new CCodeConstant (after ? "TRUE" : "FALSE")); + } else if (m != null && m.closure) { // g_signal_connect_data + // third argument: handler + ccall.add_argument (new CCodeCastExpression (get_cvalue (handler), "GCallback")); + // fourth argument: user_data CCodeExpression handler_destroy_notify; ccall.add_argument (get_delegate_target_cexpression (handler, out handler_destroy_notify)); @@ -743,6 +774,9 @@ public class Vala.GSignalModule : GObjectModule { // g_signal_connect_object or g_signal_handlers_disconnect_matched // or dynamic_signal_connect or dynamic_signal_disconnect + // third resp. sixth argument: handler + ccall.add_argument (new CCodeCastExpression (get_cvalue (handler), "GCallback")); + // fourth resp. seventh argument: object/user_data if (handler is MemberAccess) { var right_ma = (MemberAccess) handler; @@ -765,6 +799,9 @@ public class Vala.GSignalModule : GObjectModule { ccall.add_argument (new CCodeConstant ("G_CONNECT_AFTER")); } } else if (dt != null && dt.delegate_symbol.has_target) { + // third argument: handler + ccall.add_argument (new CCodeCastExpression (get_cvalue (handler), "GCallback")); + // fourth argument: user_data CCodeExpression handler_destroy_notify; ccall.add_argument (get_delegate_target_cexpression (handler, out handler_destroy_notify)); @@ -782,6 +819,9 @@ public class Vala.GSignalModule : GObjectModule { // g_signal_connect or g_signal_connect_after or g_signal_handlers_disconnect_matched // or dynamic_signal_connect or dynamic_signal_disconnect + // third resp. sixth argument: handler + ccall.add_argument (new CCodeCastExpression (get_cvalue (handler), "GCallback")); + // fourth resp. seventh argument: user_data ccall.add_argument (new CCodeConstant ("NULL")); } diff --git a/tests/Makefile.am b/tests/Makefile.am index 2f3490d0f..4f132eb02 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -261,6 +261,7 @@ TESTS = \ objects/bug620675.vala \ objects/bug620706.vala \ objects/bug624594.vala \ + objects/bug624624.vala \ objects/bug626038.vala \ objects/bug628639.vala \ objects/bug631267.vala \ diff --git a/tests/objects/bug624624.vala b/tests/objects/bug624624.vala new file mode 100644 index 000000000..75c6a5b2a --- /dev/null +++ b/tests/objects/bug624624.vala @@ -0,0 +1,58 @@ +class Foo : Object { + public static bool destroyed; + public signal void sig (); + + ~Foo () { + destroyed = true; + } +} + +class Bar : Object { + public static bool destroyed; + Foo foo; + + public Bar (Foo f) { + foo = f; + foo.sig.connect (() => assert_not_reached ()); + } + + ~Bar () { + destroyed = true; + } +} + +class Manam : Object { + public static bool destroyed; + public static bool reached; + Foo foo; + + public Manam (Foo f) { + foo = f; + foo.sig.connect (() => reached = true); + } + + ~Manam () { + destroyed = true; + } +} + +void bar (Foo f) { + { + var bar = new Bar (f); + // bar should be finalized here + } + assert (Bar.destroyed); +} + +void main () { + { + var foo = new Foo (); + bar (foo); + var manam = new Manam (foo); + foo.sig (); + assert (Manam.reached); + // manam and then foo should be finalized here + } + assert (Manam.destroyed); + assert (Foo.destroyed); +} -- cgit v1.2.1