Revert "codegen: Stop taking explicit references on 'this' for captured blocks"

This reverts commit 75e61cfbadb3d98f44835665d25fa3b836cbceb5.

See https://gitlab.gnome.org/GNOME/vala/issues/654

5 files changed, 27 insertions, 105 deletions

diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index 83ef3d43b..57d277de4 100644
--- a/codegen/valaccodebasemodule.vala
+++ b/codegen/valaccodebasemodule.vala
@@ -2039,7 +2039,13 @@ 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)) {
-					ccode.add_assignment (new CCodeMemberAccess.pointer (get_variable_cexpression ("_data%d_".printf (block_id)), "self"), get_result_cexpression ("self"));
+					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);
 				}
 
 				if (current_method != null) {
@@ -2223,6 +2229,17 @@ 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 56e19e412..0e800cba8 100644
--- a/codegen/valaccodemethodcallmodule.vala
+++ b/codegen/valaccodemethodcallmodule.vala
@@ -321,7 +321,11 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
 			ccode.add_assignment (get_this_cexpression (), new CCodeCastExpression (ccall, get_ccode_name (current_class) + "*"));
 
 			if (current_method.body.captured) {
-				ccode.add_assignment (new CCodeMemberAccess.pointer (get_variable_cexpression ("_data%d_".printf (get_block_id (current_method.body))), "self"), get_this_cexpression ());
+				// 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);
 			}
 
 			if (!current_class.is_compact && current_class.get_type_parameters ().size > 0) {
diff --git a/codegen/valagsignalmodule.vala b/codegen/valagsignalmodule.vala
index 7bf71f344..f9a3b9faf 100644
--- a/codegen/valagsignalmodule.vala
+++ b/codegen/valagsignalmodule.vala
@@ -623,9 +623,7 @@ public class Vala.GSignalModule : GObjectModule {
 				else
 					connect_func = get_dynamic_signal_connect_after_wrapper_name ((DynamicSignal) sig);
 			} else {
-				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)) {
+				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";
@@ -723,41 +721,12 @@ public class Vala.GSignalModule : GObjectModule {
 			ccall.add_argument (new CCodeConstant ("NULL"));
 		}
 
-		if ((m != null && m.closure) && in_gobject_instance (m)) {
-			// g_signal_connect_closure
+		// third resp. sixth argument: handler
+		ccall.add_argument (new CCodeCastExpression (get_cvalue (handler), "GCallback"));
 
-			// 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) {
+		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));
@@ -774,9 +743,6 @@ 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;
@@ -799,9 +765,6 @@ 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));
@@ -819,9 +782,6 @@ 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 3488b443c..312c88a42 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -268,7 +268,6 @@ 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
deleted file mode 100644
index 75c6a5b2a..000000000
--- a/tests/objects/bug624624.vala
+++ /dev/null
@@ -1,58 +0,0 @@
-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);
-}