GNOME Bugzilla – Bug 789249
Reentrancy issue if async functions get callback called before yield.
Last modified: 2018-05-22 15:56:13 UTC
Created attachment 361955 [details] testcase showing the error. The way valac works, the same function (suffixed _co()) handles both the first steps of the async function and the after effect, it just increments _data_->_state_ to 1 right before returning for the next time it's called. There is a problem though if the async method manages to be completed before the yield call (eg. because of it being quickly dispatched in another thread). The _data_->_state_ change happens after the callback function is called, so it enters again on the _co() function with _state_ == 0, which does the first part of the function again. It would be expected that at the time of reentering the _co() function, _state_ is 1 so it does handle the second part inside the first one, doing _data_->_state_ = 1 early in the _state0: path seems a thread safe way to get this. I'm attaching a simple test case that asserts on the async foo() method if the portions before the yield get called more than once.
Created attachment 362002 [details] [review] codegen: Make every co-routine state its own CodeBlock and bump state early
Of course this patch breaks the async/generator test. :(
AFAICS the error is "legit", the patch is breaking code flow of that test. The unconditional ccode.close() before state>1 labels results in the wrong code block being closed, because the _state_1 label is nested inside the if (i % 2) check. Manually correcting that fixes the testcase. Not sure if there's a better place to close the blocks though... I guess could be fixed with more intelligent tracking of the current block to ensure we are closing it if it matches the previous _state_X label's.
Created attachment 362485 [details] [review] codegen: Make every co-routine state its own CodeBlock and bump state early
Thanks for looking into it. Seems like this can be accomplished easier than I expected, see the updated patch.
Nevermind, don't bother to test it.
Created attachment 364605 [details] [review] codegen: Make every co-routine state its own CodeBlock and bump state early
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/601.