Specify that async functions start executing synchronously.

Specify that async functions start executing synchronously.

R=eernst@google.com

Committed: https://github.com/dart-lang/sdk/commit/2170830a9e41fa5b4067fde7bd44b76f5128c502

[Lasse Reichstein Nielsen, 2017-08-16 15:51:09]

lrn@google.com changed reviewers:
+ eernst@google.com

[Lasse Reichstein Nielsen, 2017-08-16 15:51:10]

This seems too simple. I was expecting more complication, so am I missing
something?

[eernst, 2017-08-17 17:02:22]

One thing I was wondering about: This means that some `async` functions will
_never_ suspend. You could simply write synchronous-only code (e.g.,
`Future<void> foo() async {}`). So `async` functions don't just "start faster",
they may also "terminate sooner than expected".

We already discussed that this may be a source of bugs, just because it is
different. It might be true that these bugs only occur in code which is written
in a bad (error-prone) style, and developers just have to use a better style.

But if this means that "`await` may not always wait", that might be bug-prone in
itself, also for "reasonable" code. Presumably, that was exactly the reason why
Erik Meijer suggested in the first place that "`await` should always wait".

How do we know that this is actually an improvement? Erik Meijer would have seen
a lot of issues with exactly this type of bug, or otherwise I suppose he
wouldn't have suggested to make that change, relative to C#.

https://codereview.chromium.org/3000063002/diff/1/docs/language/dartLangSpec.tex
File docs/language/dartLangSpec.tex (right):

https://codereview.chromium.org/3000063002/diff/1/docs/language/dartLangSpec....
docs/language/dartLangSpec.tex:3794: The future $o$ is completed when execution
of the body of $f$ completes (\ref{completion}). If execution of the body
returns a value, $o$ is completed with that value, if it completes normally or
returns without a value, $o$ is completed with the \NULL{} value, and if it
throws an exception $e$ and stack trace $t$, $o$ is completed with the error $e$
and stack trace $t$, possibly at a later time to allow the caller to attach
error handlers to $o$.
Might as well say `exception $e$` twice rather than `exception $e$` first and
then later `error $e$`.

I don't understand `, possibly..`, though. It does not seem to be normative (for
a normative part of the specification I wouldn't expect to see a word like
'possibly'). Doesn't the rest of the spec say enough to make that scenario
possible such that it is already commentary, semantically? In that case I'd
suggest that we put it into `\commentary{}` syntactically as well.

[Lasse Reichstein Nielsen, 2017-08-17 17:22:35]

> One thing I was wondering about: This means that some `async` functions will
_never_ suspend. You could simply write synchronous-only code (e.g.,
`Future<void> foo() async {}`). So `async` functions don't just "start faster",
they may also "terminate sooner than expected".

Correct. That's where the "possibly" comes from. 
If an async function completes synchronously by throwing, the returned future is
completed with an error at a later time. We cannot complete it immediately
because the future hasn't been returned yet, so there hasn't been time to add
error handlers yet.
I guess this means that I must find the place where the future is completed due
to the body ending with an exception and say that the future is completed in "a
later microtask".

> We already discussed that this may be a source of bugs, just because it is
different. It might be true that these bugs only occur in code which is written
in a bad (error-prone) style, and developers just have to use a better style.

We must ensure that the early completion doesn't affect the calling code badly,
which means not completing the future synchronously with an error.
Completing it synchronously with a value is perfectly fine because nobody can be
waiting on it yet.

> But if this means that "`await` may not always wait", that might be bug-prone
in itself, also for "reasonable" code. Presumably, that was exactly the reason
why Erik Meijer suggested in the first place that "`await` should always wait".

`await` always waits. That's not changed by this change because in the case
where the body completes synchronously, the future has not been returned and
awaited yet. Awaiting a completed future will wait (there is never a callback
immediately in response to calling future.then). If the body completes
synchronously with an error, we must delay the completion because otherwise the
error will appear unhandled, so again, any await will also wait for that.

> How do we know that this is actually an improvement? Erik Meijer would have
seen a lot of issues with exactly this type of bug, or otherwise I suppose he
wouldn't have suggested to make that change, relative to C#.

I don't actually know what these problems are, but if they exist in Dart, then
they already did prior to this change. Asynchronous functions (ones returning a
Future) have always been able to start immediately, it's only the "async" marked
ones that didn't. 
That, on the other hand, has meant that "async" functions can't be used in some
cases that require listening immediately (say one that does "await for" on a
broadcast stream, if you delay starting you might miss events), and there have
been bugs where users thought the function started immediately, and when
translating from a sync function to an async, an invariant was not preserved
across the invocation.

https://codereview.chromium.org/3000063002/diff/1/docs/language/dartLangSpec.tex
File docs/language/dartLangSpec.tex (right):

https://codereview.chromium.org/3000063002/diff/1/docs/language/dartLangSpec....
docs/language/dartLangSpec.tex:3794: The future $o$ is completed when execution
of the body of $f$ completes (\ref{completion}). If execution of the body
returns a value, $o$ is completed with that value, if it completes normally or
returns without a value, $o$ is completed with the \NULL{} value, and if it
throws an exception $e$ and stack trace $t$, $o$ is completed with the error $e$
and stack trace $t$, possibly at a later time to allow the caller to attach
error handlers to $o$.
On 2017/08/17 17:02:21, eernst wrote:
> Might as well say `exception $e$` twice rather than `exception $e$` first and
> then later `error $e$`.
> 
> I don't understand `, possibly..`, though. It does not seem to be normative
(for
> a normative part of the specification I wouldn't expect to see a word like
> 'possibly'). Doesn't the rest of the spec say enough to make that scenario
> possible such that it is already commentary, semantically? In that case I'd
> suggest that we put it into `\commentary{}` syntactically as well.

Acknowledged.

[eernst, 2017-08-18 07:21:07]

LGTM!

[Lasse Reichstein Nielsen, 2017-08-18 10:56:15]

PTAL

[eernst, 2017-08-18 10:58:38]

lgtm

[Lasse Reichstein Nielsen, 2017-08-18 11:01:07]

Description was changed from

==========
Specify that async functions start executing synchronously.
==========

to

==========
Specify that async functions start executing synchronously.

R=eernst@google.com

Committed:
https://github.com/dart-lang/sdk/commit/2170830a9e41fa5b4067fde7bd44b76f5128c502
==========

[Lasse Reichstein Nielsen, 2017-08-18 11:01:08]

Committed patchset #2 (id:20001) manually as
2170830a9e41fa5b4067fde7bd44b76f5128c502 (presubmit successful).