From 9f2d548584c3f2905544f72424875819a5fe9bd7 Mon Sep 17 00:00:00 2001 From: Rico Tzschichholz Date: Thu, 31 Dec 2020 09:55:16 +0100 Subject: [PATCH] codegen: Don't leak memory of already assigned out-parameter on error Fixes https://gitlab.gnome.org/GNOME/vala/issues/1123 --- codegen/valaccodebasemodule.vala | 11 ++++++ codegen/valagasyncmodule.vala | 4 ++ codegen/valagerrormodule.vala | 6 +++ tests/Makefile.am | 2 + .../out-parameter-free-on-error.vala | 31 +++++++++++++++ .../methods/parameter-out-free-on-error.vala | 39 +++++++++++++++++++ 6 files changed, 93 insertions(+) create mode 100644 tests/asynchronous/out-parameter-free-on-error.vala create mode 100644 tests/methods/parameter-out-free-on-error.vala diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala index 0199ebba4..93236afe1 100644 --- a/codegen/valaccodebasemodule.vala +++ b/codegen/valaccodebasemodule.vala @@ -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; diff --git a/codegen/valagasyncmodule.vala b/codegen/valagasyncmodule.vala index 8eca2d531..47e3db6d3 100644 --- a/codegen/valagasyncmodule.vala +++ b/codegen/valagasyncmodule.vala @@ -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); diff --git a/codegen/valagerrormodule.vala b/codegen/valagerrormodule.vala index d1d6d3393..b362687cd 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); + 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__")); diff --git a/tests/Makefile.am b/tests/Makefile.am index bcc861152..5c84e74d4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -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 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); + } +} -- 2.47.2