From: Rico Tzschichholz <ricotz@ubuntu.com>
Date: Thu, 31 Dec 2020 08:55:16 +0000 (+0100)
Subject: codegen: Don't leak memory of already assigned out-parameter on error
X-Git-Tag: 0.51.1~118
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=af63cf9ac9243d404beb59100316ba2b2f914f68;p=thirdparty%2Fvala.git

codegen: Don't leak memory of already assigned out-parameter on error

Fixes https://gitlab.gnome.org/GNOME/vala/issues/1123
---

diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala
index b1f7e6869..b7db31835 100644
--- a/codegen/valaccodebasemodule.vala
+++ b/codegen/valaccodebasemodule.vala
@@ -3972,6 +3972,17 @@ public abstract class Vala.CCodeBaseModule : CodeGenerator {
 		}
 	}
 
+	public void append_out_param_free (Method? m) {
+		if (m == null) {
+			return;
+		}
+		foreach (Parameter param in m.get_parameters ()) {
+			if (param.direction == ParameterDirection.OUT && param.variable_type.is_disposable ()) {
+				ccode.add_expression (destroy_parameter (param));
+			}
+		}
+	}
+
 	public bool variable_accessible_in_finally (LocalVariable local) {
 		if (current_try == null) {
 			return false;
diff --git a/codegen/valagasyncmodule.vala b/codegen/valagasyncmodule.vala
index f39f86c6e..c9946599f 100644
--- a/codegen/valagasyncmodule.vala
+++ b/codegen/valagasyncmodule.vala
@@ -743,8 +743,12 @@ public class Vala.GAsyncModule : GtkModule {
 		set_error.add_argument (error_expr);
 		ccode.add_expression (set_error);
 
+		// free local variables
 		append_local_free (current_symbol);
 
+		// free possibly already assigned out-parameter
+		append_out_param_free (current_method);
+
 		// We already returned the error above, we must not return anything else here.
 		var unref = new CCodeFunctionCall (new CCodeIdentifier ("g_object_unref"));
 		unref.add_argument (async_result_expr);
diff --git a/codegen/valagerrormodule.vala b/codegen/valagerrormodule.vala
index fedc424ae..d724c3c3a 100644
--- a/codegen/valagerrormodule.vala
+++ b/codegen/valagerrormodule.vala
@@ -104,6 +104,9 @@ public class Vala.GErrorModule : CCodeDelegateModule {
 		// free local variables
 		append_local_free (current_symbol);
 
+		// free possibly already assigned out-parameter
+		append_out_param_free (current_method);
+
 		if (current_method is CreationMethod && current_method.parent_symbol is Class) {
 			var cl = (Class) current_method.parent_symbol;
 			ccode.add_expression (destroy_value (new GLibValue (new ObjectType (cl), new CCodeIdentifier ("self"), true)));
@@ -123,6 +126,9 @@ public class Vala.GErrorModule : CCodeDelegateModule {
 			append_local_free (current_symbol);
 		}
 
+		// free possibly already assigned out-parameter
+		append_out_param_free (current_method);
+
 		cfile.add_include ("glib.h");
 
 		var ccritical = new CCodeFunctionCall (new CCodeIdentifier ("g_critical"));
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 517d5746a..3d41840ed 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -162,6 +162,7 @@ TESTS = \
 	methods/extern.vala \
 	methods/iterator.vala \
 	methods/parameter-fixed-array-initializer.vala \
+	methods/parameter-out-free-on-error.vala \
 	methods/parameter-ref-array-resize.vala \
 	methods/parameter-ref-array-resize-captured.vala \
 	methods/parameter-ref-delegate.vala \
@@ -632,6 +633,7 @@ TESTS = \
 	asynchronous/constructor-argument-check.vala \
 	asynchronous/finish-name.vala \
 	asynchronous/generator.vala \
+	asynchronous/out-parameter-free-on-error.vala \
 	asynchronous/out-parameter-invalid.test \
 	asynchronous/params-array-invalid.test \
 	asynchronous/result-pos.vala \
diff --git a/tests/asynchronous/out-parameter-free-on-error.vala b/tests/asynchronous/out-parameter-free-on-error.vala
new file mode 100644
index 000000000..a81d3df55
--- /dev/null
+++ b/tests/asynchronous/out-parameter-free-on-error.vala
@@ -0,0 +1,31 @@
+errordomain FooError {
+	FAIL
+}
+
+class Manam : Object {
+}
+
+async void foo_async (Manam i, out Manam o) throws FooError {
+	o = i;
+	throw new FooError.FAIL ("foo");
+}
+
+async void run () {
+	var manam = new Manam ();
+	assert (manam.ref_count == 1);
+	try {
+		Manam minim;
+		yield foo_async (manam, out minim);
+	} catch {
+	}
+	assert (manam.ref_count == 2);
+	loop.quit ();
+}
+
+MainLoop loop;
+
+void main () {
+	loop = new MainLoop ();
+	run.begin ();
+	loop.run ();
+}
diff --git a/tests/methods/parameter-out-free-on-error.vala b/tests/methods/parameter-out-free-on-error.vala
new file mode 100644
index 000000000..ee9716b87
--- /dev/null
+++ b/tests/methods/parameter-out-free-on-error.vala
@@ -0,0 +1,39 @@
+errordomain FooError {
+	FAIL
+}
+
+class Manam : Object {
+}
+
+void foo (Manam i, out Manam o) throws FooError {
+	o = i;
+	throw new FooError.FAIL ("foo");
+}
+
+void bar (Manam i, out unowned Manam o) throws FooError {
+	o = i;
+	throw new FooError.FAIL ("bar");
+}
+
+void main () {
+	{
+		var manam = new Manam ();
+		assert (manam.ref_count == 1);
+		try {
+			Manam minim;
+			foo (manam, out minim);
+		} catch (FooError e) {
+		}
+		assert (manam.ref_count == 1);
+	}
+	{
+		var manam = new Manam ();
+		assert (manam.ref_count == 1);
+		try {
+			unowned Manam minim;
+			bar (manam, out minim);
+		} catch (FooError e) {
+		}
+		assert (manam.ref_count == 1);
+	}
+}