GNOME Bugzilla – Bug 599488
Trying to call unowned delegate functions from async functions doesn't produce a good error message
Last modified: 2010-01-09 12:51:10 UTC
Created attachment 146173 [details] Test program which erroneously tries to call an unowned delegate function from an async function Trying to compile the attached test program with valac 0.7.7 results in the following error message. It is not clear from this that the test_async function declaration is missing an owned keyword for TestFunc f from this: ** (valac:5128): CRITICAL **: vala_data_type_get_value_owned: assertion `self != NULL' failed /home/ph5/src/vala/async-delegate/async-delegate.c:44: error: expected specifier-qualifier-list before ‘AsyncDelegateTestFunc’ /home/ph5/src/vala/async-delegate/async-delegate.c: In function ‘async_delegate_test_async_data_free’: /home/ph5/src/vala/async-delegate/async-delegate.c:89: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f_target_destroy_notify’ /home/ph5/src/vala/async-delegate/async-delegate.c:89: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f_target_destroy_notify’ /home/ph5/src/vala/async-delegate/async-delegate.c:89: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f_target’ /home/ph5/src/vala/async-delegate/async-delegate.c:90: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f’ /home/ph5/src/vala/async-delegate/async-delegate.c:91: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f_target’ /home/ph5/src/vala/async-delegate/async-delegate.c:92: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f_target_destroy_notify’ /home/ph5/src/vala/async-delegate/async-delegate.c: In function ‘async_delegate_test_async’: /home/ph5/src/vala/async-delegate/async-delegate.c:103: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f’ /home/ph5/src/vala/async-delegate/async-delegate.c:104: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f_target’ /home/ph5/src/vala/async-delegate/async-delegate.c: In function ‘async_delegate_test_async_co’: /home/ph5/src/vala/async-delegate/async-delegate.c:140: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f’ /home/ph5/src/vala/async-delegate/async-delegate.c:140: error: ‘AsyncDelegateTestAsyncData’ has no member named ‘f_target’ error: cc exited with status 256 Compilation failed: 1 error(s), 0 warning(s)
Created attachment 146180 [details] [review] semantic analyzer: do not allow unowned delegate parameters to async methods The attached patch adds an error for unowned delegates given as parameters to async functions. The result is the following nice error message: gio-2.0.vapi:982.3-982.39: error: Missing `owned' keyword for delegate parameter to async method public abstract async bool copy_async (GLib.File destination, GLib.FileCopyFlags flags, int io_priority, GLib.Cancellable? cancellable, GLib.FileProgressCallback? progress_callback) throws GLib.Error; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ gio-2.0.vapi:1022.3-1022.47: error: Missing `owned' keyword for delegate parameter to async method public async bool load_partial_contents_async (GLib.Cancellable? cancellable, GLib.FileReadMoreCallback read_more_callback, out string contents, out size_t length, out string etag_out) throws GLib.Error; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ async-unowned-delegate.vala:8.2-8.22: error: Missing `owned' keyword for delegate parameter to async method async void test_async (TestFunc f) { ^^^^^^^^^^^^^^^^^^^^^ Compilation failed: 3 error(s), 0 warning(s) All fine and well, except that it should probably mark the parameter, not the method. And I guess those two errors in gio-2.0.vapi are very much false positives?
Created attachment 146188 [details] [review] semantic analyzer: do not allow unowned delegate parameters to async methods Otherwise vala can't guarantee that the target is still valid when the async method gets to call the delegate function. The check is moved from Method.check to FormalParameter.check and the error is only emitted if the delegate symbol is not external (to avoid the abovementioned errors in gio-2.0.vapi).
After seeing Bug 597294 and the fix to it I'm not so sure anymore if this is necessarily true. I guess unowned delegate parameters might be valid if and only if the delegate is called in the first part of the async function only (i.e. not anymore after the first yield)?
Unowned delegate parameters may also be valid after the first part of the async function, it depends on the lifetime of the delegate target. It's certainly true that this may produce buggy code, however, not in all cases, and never accepting it will break some code without providing an alternative. I'm closing this as WONTFIX for now as I don't see a possible solution for more safety without breaking compatibility with existing libraries.