Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad interaction with fake-zones. - static futures must be created in the root zone. #32556

Closed
grouma opened this issue Mar 16, 2018 · 2 comments
Closed

Comments

@grouma
Copy link
Member

@grouma grouma commented Mar 16, 2018

Creating an external tracking issue so that it can be referenced from package:test.

From an internal investigation:

This is a bug in the SDK and a bad interaction with the fake-zones.
Basically what happens is that the async-SDK caches some common futures to avoid allocating them all the time. Specifically, there is:

  /// A `Future<Null>` completed with `null`.
  static final _Future<Null> _nullFuture = new _Future<Null>.value(null);

  /// A `Future<bool>` completed with `false`.
  static final _Future<bool> _falseFuture = new _Future<bool>.value(false);

These are returned from internal classes whenever needed.
Furthermore, there are some optimizations in our Stream implementation. For example (but there are others):

      if (!identical(cancelFuture, Future._nullFuture)) {
        cancelFuture.whenComplete(() {
          result._completeError(error, stackTrace);
        });
      } else {
        result._completeError(error, stackTrace);
      }
    };

What happens now is that the first time the _nullFuture is used in the test, the current Zone is set to a custom zone that overwrites the scheduleMicrotask to redirect it to testing._FakeAsync.

However, scheduleMicrotask is used for every .then call, and with the fake-async scheduleMicrotask (which needs to be triggered by hand?) the listeners are never executed. This can be easily seen by adding one more scenario to Nate's document:

  tearDown(() async {
    var f = setUpSubscription.cancel();
    f.then((_) { print("null triggered"); });
    return f;
  });

This will never execute the .then.

The only reason it is sometimes working, is that it hits our optimizations in the stream class. There we don't even call .then but shortcut the execution since we recognize the instance and know that it will just return null.

This also explains most of Nate's scenarios: as soon as you actually call .then on the future you will get a timeout. This can be explicitly or implicitly with await. It only works, if the cancel() future is passed on directly (as an optimization as is the case in my example here), and it then hits our optimization.

For some reason the whole cancel() call must be delayed too, but I haven't tried to figure out why yet. It's probably related to some timing guarantees we give, thus calling .then on the future.

The solution is as simple as creating these static futures in the root-zone. Inside async/future.dart:

static final _Future<Null> _nullFuture = Zone.ROOT.run(() => new
      Future<Null>.value(null));

  /// A `Future<bool>` completed with `false`.
  static final _Future<bool> _falseFuture = Zone.ROOT.run(() => new
      Future<bool>.value(false));
@natebosch
Copy link
Member

@natebosch natebosch commented Mar 28, 2018

cc @lrhn - Is this something we can get fixed soon?

@lrhn
Copy link
Member

@lrhn lrhn commented Nov 1, 2019

This was finally fixed a while ago.

@lrhn lrhn closed this Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants