Consider adding support to --lazy-async-stacks for `Future.timeout()`

[mkustermann]

Some g3 users use Future.timeout() in code, e.g.:

import 'dart:async';

main() async {
  await bar();
}
Future bar() async {
  await (foo().timeout(Duration(seconds: 1)));
}
Future foo() async {
  await 0;
  throw 'a';
}

Though the --lazy-async-stacks implemementation looses track of the awaiter of the Future returned by .timeout():

% out/ReleaseX64/dart --lazy-async-stacks --no-causal-async-stacks async.dart
Unhandled exception:
a
#0      foo (file:///.../test.dart:13:3)
<asynchronous suspension>
#1      Future.timeout.<anonymous closure> (dart:async/future_impl.dart)
<asynchronous suspension>

The implementation looks like this (see future_impl.dart

  Future<T> timeout(Duration timeLimit, {FutureOr<T> onTimeout()}) {
    _Future<T> result = new _Future<T>();
    ...
    this.then((T v) {
      if (timer.isActive) {
        timer.cancel();
        result._completeWithValue(v);
      }
      ...
    });
    return result;
  }
}

We can consider adding special support for unwinding in this case by recognizing the inner closure in timeout() and knowing that the result variable will be available in the context.

b/297400933

/cc @cskau-g @mraleph

[mraleph]

I wonder if we could do this in a way that does not require custom C++ code (I am thinking towards future places like this). Can we add some helpers/annotations that would guide stack unwinding?

[mkustermann]

I wonder if we could do this in a way that does not require custom C++ code
(I am thinking towards future places like this). Can we add some helpers/annotations that would guide stack unwinding?

We have discussed this a bit offline. Summarization:

• We could scan all context entries and parent contexts to see if we find Future/StreamController objects and automatically continue unwinding there: This could result in phantom stacks that are incorrect (unknown if this would actually be an issue in practice)
• We could add an annotation in Dart to specifically guide the runtime unwinder. Though that would require the VM's compiler to store the unwinding instructions in a way to make it accessible from the closure function (could extend pc descriptors, ...). Whether this complexity is justified is another question.

We agreed that for now we will add the two special cases - Future.timeout and Future.wait - which we know are important.

If, after turning lazy async stack on-by-default, we get feedback that there are more cases (possibly outside the core libraries) it might justify the added complexity to generalize this.

[mkustermann]

Even though we should not encourage more usage of our old Future based-api, we should make dealing with legacy code work reasonably well. Support for Future.timeout() and Future.whenComplete() was added now.

@cskau-g Could you investigate adding the remaining support for Future.then() and Future.catchError() as well?

Support for Future.timeout() was added in 219935a, and support for Future.then() and Future.catchError() was added in 8f2c47f.