[vala/staging: 3/3] codegen: Stop taking explicit references on 'this' for captured blocks

[Rico Tzschichholz <ricotz src gnome org>]

commit 75e61cfbadb3d98f44835665d25fa3b836cbceb5
Author: Michal Hruby <michal mhr gmail com>
Date:   Tue Mar 20 16:50:14 2018 +0100

    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(-)
---
diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index 0d8629e..5ff51e6 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 1beaa8b..6ab1597 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 ba4b4d1..4f99941 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 2f3490d..4f132eb 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 0000000..75c6a5b
--- /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);
+}