codegen: Stop taking explicit references on 'this' for captured blockswip/queue-0.41

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

5 files changed, 105 insertions, 27 deletions

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);
+}