]> git.ipfire.org Git - thirdparty/vala.git/commitdiff
codegen: Don't leak memory of already assigned out-parameter on error
authorRico Tzschichholz <ricotz@ubuntu.com>
Thu, 31 Dec 2020 08:55:16 +0000 (09:55 +0100)
committerRico Tzschichholz <ricotz@ubuntu.com>
Sun, 3 Jan 2021 12:29:00 +0000 (13:29 +0100)
Fixes https://gitlab.gnome.org/GNOME/vala/issues/1123

codegen/valaccodebasemodule.vala
codegen/valagasyncmodule.vala
codegen/valagerrormodule.vala
tests/Makefile.am
tests/asynchronous/out-parameter-free-on-error.vala [new file with mode: 0644]
tests/methods/parameter-out-free-on-error.vala [new file with mode: 0644]

index 0199ebba40d75ee383c3a25a54ee10a564578c34..93236afe1e94c875c2e00092e90b4e410ac203ff 100644 (file)
@@ -3810,6 +3810,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;
index 8eca2d53198f7cc11546af860051e94b5adcb6c4..47e3db6d3ca19c12c19fe1cecaa723402b57c3a1 100644 (file)
@@ -821,8 +821,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);
index d1d6d33930b8cb4b72b3c5d36297aad5fb18e58f..b362687cd0b814832c7d900c5b7bdf71a7d3985f 100644 (file)
@@ -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);
+
                var ccritical = new CCodeFunctionCall (new CCodeIdentifier ("g_critical"));
                ccritical.add_argument (new CCodeConstant (unexpected ? "\"file %s: line %d: unexpected error: %s (%s, %d)\"" : "\"file %s: line %d: uncaught error: %s (%s, %d)\""));
                ccritical.add_argument (new CCodeConstant ("__FILE__"));
index bcc8611522dbed03c50402a1560b41546fdf89a4..5c84e74d462249a14efaad9fdefe771c567bab7a 100644 (file)
@@ -133,6 +133,7 @@ TESTS = \
        methods/contains.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 \
@@ -524,6 +525,7 @@ TESTS = \
        asynchronous/closures.vala \
        asynchronous/constructor-argument-check.vala \
        asynchronous/generator.vala \
+       asynchronous/out-parameter-free-on-error.vala \
        asynchronous/result-pos.vala \
        asynchronous/variadic-invalid.test \
        asynchronous/variadic-invalid-2.test \
diff --git a/tests/asynchronous/out-parameter-free-on-error.vala b/tests/asynchronous/out-parameter-free-on-error.vala
new file mode 100644 (file)
index 0000000..a81d3df
--- /dev/null
@@ -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 (file)
index 0000000..ee9716b
--- /dev/null
@@ -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);
+       }
+}