GNOME Bugzilla – Bug 639054
Closures and coroutines own unowned variables
Last modified: 2012-01-06 22:36:29 UTC
Hello, take for example InputStream.read_async which has a uint8[] parameter. If you override it, Vala will copy the buffer into the async block data, thus referencing the parameter in the method will reference the copy, which is wrong. For this reason, it must be noted that the semantics of referencing the parameter is different than a non-async method.
Importantly, this can easily prevent an object from ever being finalized (which is obviously a serious problem). Eg, class Foo : Object { Foo () { var bar = new Bar (); bar.notify["qux"].connect ((a) => { this.some_func (); }); } } (Sorry it's not a little simpler - I'm trying not to deviate too much from my real-world example). The generated C creates a closure which owns a ref to this/self. This should be either a weak ref or no ref at all (unowned), since it currently keeps a Foo with ref_count == 1 when it's removed from its final structure, effectively leaking the object (since it will never be unref'd and finalized). In this case, I'm showing how it applies to closures created for signal handlers. If this is a different bug from the original comment, please split it out into its own.
(In reply to comment #1) mhr3 pointed out an existing bug that matches this specific issue (bug #624624), so please ignore my comment above.
Both closures and coroutines suffer from this copying of unowned parameters. This is the current situation: 1) Reference counted objects are referenced 2) Non-reference counted objects are copied (arrays, structs and compact classes) 3) Delegates are not copied 4) Compact class issue an error because they must be copied explicitly 5) It's planned to explicitly copy arrays as well, so there will be a deprecation warning at some time 6) Arrays and disposable structs will not modify the original data, this leads most of the time to the use of workarounds (or impossibility to implement certain async methods) Proposed (partial) solution, will break BC for arrays and structs, but for the above reasons it will be a fix in certain situations: 1) If the object is reference counted, reference it 2) Everything else is not copied
Created attachment 204172 [details] [review] codegen: Only copy captured parameters that are reference counted Partially fixes bug 639054.
Created attachment 204173 [details] [review] codegen: Only copy captured parameters that are reference counted Partially fixes bug 639054.
Created attachment 204174 [details] [review] codegen: Only copy captured parameters that are reference counted Partially fixes bug 639054. Added a test case.
Created attachment 204178 [details] [review] GAsync: Only copy parameters that are reference counted Fixes bug 639054.
commit 74f40ddf54f4a2d59e767a7ab689bc72d8936338 Author: Luca Bruno <lucabru@src.gnome.org> Date: Fri Jan 6 22:57:02 2012 +0100 codegen: Don't copy array parameters when captured Also allow uncopiable compact classes to be captured. Fixes bug 639054. This commit has less impact than the proposed patch. In particular, it avoids implicit copy only for arrays and uncopiable compact classes (which was an error before the commit). Therefore, the change only affects arrays in existing code. This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.