Dart2js release build may make generic parameters dynamic

[rrousselGit]

From rrousselGit/provider#406

@mono0926 found a compilation issue with Dart2js when running:

flutter run -t chrome --release

where the build output behaves differently from debug builds and VM builds.

The issue happens with the following code:

typedef Locator = void Function<T>();

class Bar {
  Bar({this.read}) {
    read<int>();
  }

  final Locator read;
}

void main() {
  Bar(read: <T>() {
    print(T);
    return null;
  });
}

The expected behavior of this code is to print int in the console, which it does in debug builds and VM release builds.
But when running this code with flutter -t chrome --release, this will print dynamic

This causes InheritedWIdgets from Flutter to fail to be resolved

[rrousselGit]

This was tested on the up-to-date master channel of Flutter (1.18.0-5.0.pre-57), which uses Dart 2.8.0-dev.20.0

[vsmenon]

fyi - @rakudrama @fishythefish

[fishythefish]

Hmm, looks like we're discarding the type argument in the backend somewhere:

V.main_closure.prototype = {
  call$1$0: function($T) {
    H.printString(H.createRuntimeType($T).toString$0(0));
  },
  call$0: function() {
    return this.call$1$0(type$.dynamic);
  },
  $signature: 0
};

This is the case even if the Bar constructor is simply Bar(this.read); and main itself invokes .read<int>(), but the problem vanishes if the Locator is a local instead of a member.

[rakudrama]

At the very beginning the SSA code is missing the type argument.
With the command-line option --disable-rti-optimization, it prints int.
The type is probably being dropped on the basis of some 'rti-need' bug.

[sma]

This bug stopps us from release a web app. Is there any workaround other then removing Provider from the whole app?

[fishythefish]

Taking a deeper look at this now since this is causing NNBD tests to fail.

[whesse]

The fix for this is causing test failures on dartdevc firefox
https://dart-review.googlesource.com/c/sdk/+/151301
introduced failures on firefox:

dartdevk-checked-linux-release-firefox
Pass -> RuntimeError (expected Pass)
✔ language_2/regress/regress22443_test
Timeout -> RuntimeError (expected Pass)
✔ language_2/regress/regress23244_test

[fishythefish]

@whesse I'm a little confused by that, since this fix only touches pkg/compiler (the dart2js backend, not DDC), and the tests you mentioned don't seem to have anything to do with this issue.

The logs (22443, 23244) are inscrutable to me, so I don't know if this is just flakiness or not. @nshahan, do you know what's going on here?

[nshahan]

There are parts of the test infrastructure that get compiled by dart2js and then used to run tests for DDC so it is technically possible for a dart2js only change to break a DDC bot but that should appear to be an infra failure.

These failure logs show DDC stack traces so I'm not sure how your change could be the cause. @whesse Is it possible that they are flakes or did we update the version of firefox running on the bots around the same time this change landed?

[whesse]

I checked on it, by looking in the results.json for the build after the one they were reported on.
They were detected to be flaky in that next build, so they stopped being reported on the results feed, and the previous failure was not marked as "superceded" (inactive, replaced by a more recent change).

This failure has happened a number of times before, and confuses everyone and wastes their time.
The fix is to report a change in flaky status to the results feed, and mark earlier failures on now flaky tests as inactive.
I see now that fixing this is urgent, because it wastes so many users' time when it happens.
dart-lang/dart_ci#98

[matthewtsmith]

I'm not sure when this will be available in the beta channel but for those using StateNotifier/Provider/LocatorMixin who need a workaround I was able to solve it by using the following:

  Repository get repository {
    var reader = read;
    return reader();
  }

As this comment mentions, making read a local var seems to resolve the issue.

[rrousselGit]

I'm not sure why this is closed as the bug is still happening on the latest version of Dart/Flutter

Dart SDK version: 2.9.0-21.0.dev.flutter-06cb010247 (be) (Tue Jul 7 08:14:51 2020 +0000) on "macos_x64"

[fishythefish]

My apologies - I closed the issue when my fix landed and our tests started passing, but it looks like there's still another bug lurking around when --omit-implicit-checks is passed. I'll add another test and investigate.

[fishythefish]

The initial fix for this was pretty quick, but revealed a deeper issue causing other tests (mainly async ones) to fail. I have a more complete fix that I'm working on landing now.

[ochmist]

Hello guys, any news on this? This is blocking us from releasing a web app on:

Flutter (Channel master, 1.21.0-10.0.pre.18, on Mac OS X 10.15.6 19G73, locale en-US)

Thanks in advance.

[sigmundch]

HI @ochmist - the fix landed in 5671730, which is part of Dart's 2.10.0-1.0.dev release and newer releases.

I am not sure on which flutter release this version was rolled into. When I try flutter doctor -v, I see already a newer version of the Dart sdk in the master channel:

[✓] Flutter (Channel master, 1.21.0-10.0.pre.41, on Linux, locale en_US.UTF-8)
    • Flutter version 1.21.0-10.0.pre.41 at /usr/local/google/home/sigmund/dart/flutter/flutter
    • Framework revision 49fac9a885 (2 days ago), 2020-08-11 11:44:03 -0700
    • Engine revision 9b4ce4e3a0
    • Dart version 2.10.0 (build 2.10.0-11.0.dev)

So the fix should be there. Is the Dart version in your case further behind? Is this resolved after upgrading to the latest flutter?

[ochmist]

Hi @sigmundch, Confirmed that this works well after flutter upgrade. Thank you very much.

[venkatd]

@ochmist what flutter version is working for you (flutter doctor result)? We are also using StateNotifier and our web app is crashing due to this bug.Thanks!

@matthewtsmith could you please elaborate on the change you made? I wasn't able to understand the suggestion and how to apply it to our code.

[ochmist]

Actually, after the upgrade it ended up working for a while but breaking soon after. I noticed that once of my dependencies was still outdated and the crash was happening in there.

[fishythefish]

To clarify, do you have a repro that still fails with the latest SDK, or is this just a toolchain issue?

[takeru]

I tried flutter web + riverpod.

This code doesn't work.

final xxxxProvider = StateNotifierProvider.autoDispose.family<FooController, Bar>((ref, foo) {

Then, I built without minify. https://stackoverflow.com/questions/59710360

flutter build web --profile --dart-define=Dart2jsOptimization=O0

I can see this log in chrome console.

Uncaught TypeError: Instance of 'AutoDisposeProviderElement<StateNotifier<dynamic>, StateNotifier<dynamic>>': type 'AutoDisposeProviderElement<StateNotifier<dynamic>, StateNotifier<dynamic>>' is not a subtype of type 'ProviderElement<Object, FooController>'

But this workaround saved me.
rrousselGit/riverpod@5d6d1b1#diff-d98faf2ba46cc2fa0f6e56d1bf216afe6b380aabc34ba3b4f74ae7d83cecb52fR36

final $family = StateNotifierProvider.autoDispose.family;
final xxxxProvider = $family<FooController, Bar>((ref, foo) {

[creativecreatorormaybenot]

@fishythefish Can you confirm that this is fixed in the latest SDK?

Seems like it is not (from what @ochmist & @takeru reported). Also, we are still experiencing this issue as well.

[creativecreatorormaybenot]

(still happens in 2.12.0-133.2.beta)

[fishythefish]

@creativecreatorormaybenot Sorry, I missed this notification earlier. I can't reproduce this issue in the latest SDK when I compile the original example. Do you have a different minimal repro that continues to fail? (I don't work on flutter, so the less flutter-specific, the easier it is for me.)

[creativecreatorormaybenot]

@fishythefish Using riverpod (Dart library):

final providerFamily = StateNotifierProvider.autoDispose.family<Foo, Params>((ref, params) {
  return Foo();
});

class Foo extends StateNotifier<int> {
  Foo(): super(0);
}

class Params {}

void main() {
  final container = ProviderContainer();

  container.listen(providerFamily); // crashes
}

Fixed by:

final _family = StateNotifierProvider.autoDispose.family;
final providerFamily =
    _family<Foo, Params>((ref, params) {
  return Foo();
});

// ...

---

I do not have the time to test it right now. This is a simplified version of what we have in our app, so if the bug depends on some kind of edge case, it might not reproduce it, however, I think it should do so. Let me know if you have any problems reproducing the bug, then I will get back to you later with a fully tested one - I simply wanted to shoot this one at you in order to help asap.

[fishythefish]

container doesn't have a ref getter, so I can't compile this code.

[rrousselGit]

container.listen(providerFamily) should crash

Alternatively you can do:

final providerFamily = StateNotifierProvider.autoDispose.family<Foo, Params>((ref, params) {
  return Foo();
});

final refProvider = Provider((ref) => ref);

void main() {
  final container = ProviderContainer();

  final ref = container.read(refProvider);

 ref.watch(providerFamily);
}

which should crash too

[fishythefish]

Both your examples seem to be unrelated to this issue - I'm getting a compile-time type error in each case, not a runtime error or crash due to dart2js. (Both the common front-end to the compiler and dart analyzer report this).

[creativecreatorormaybenot]

@fishythefish Sorry for posting the code prior to testing it - I thought it would be helpful and that the concept would be enough. Here are the full steps for reproducing the bug:

• Dart SDK version: 2.12.0-133.2.beta (beta)
• dart create -t web-simple bug
• Add a riverpod: 0.12.1 dependency
• Edit web/main.dart to be the following:

import 'package:riverpod/riverpod.dart';

final providerFamily = StateNotifierProvider.autoDispose.family<Foo, Params>((ref, params) {
  return Foo();
});

class Foo extends StateNotifier<int> {
  Foo(): super(0);
}

class Params {}

void main() {
  final container = ProviderContainer();
  container.listen(providerFamily(Params())); // crashes in dart2js only
  print('console.log');
}

• webdev serve & run on localhost - succeeds ("console.log" in the JS console in dev tools)
• webdev build & cd build & http-server & run on localhost - fails (error in the JS console)

Uncaught Error: undefined
    a http://localhost:8080/main.dart.js:141
    dI http://localhost:8080/main.dart.js:533
    f_ http://localhost:8080/main.dart.js:532
    f1 http://localhost:8080/main.dart.js:507
    $2$1 http://localhost:8080/main.dart.js:2649
    $1 http://localhost:8080/main.dart.js:2650
    b9 http://localhost:8080/main.dart.js:2644
    <anonymous> http://localhost:8080/main.dart.js:3016
    c http://localhost:8080/main.dart.js:27
    dY http://localhost:8080/main.dart.js:1638
    <anonymous> http://localhost:8080/main.dart.js:3025
    <anonymous> http://localhost:8080/main.dart.js:3020
    dartProgram http://localhost:8080/main.dart.js:3023
    <anonymous> http://localhost:8080/main.dart.js:3025
main.dart.js:141:3

[fishythefish]

Thanks for your patience, @creativecreatorormaybenot. It does look like we're performing an incorrect RTI optimization somewhere and erasing needed type variables. I'm reopening the issue and will investigate when I get a chance.

[fishythefish]

Turns out it was the same issue as before with essentially the same fix, but dart2js needed to do some additional work to properly track call methods. The fix is landed in the SDK, although I'm not sure when it'll roll into Flutter. Please let me know if you continue to see this bug arise.

[enyo]

@fishythefish just upgraded from flutter 2.0.2 to 2.2.1 and am still getting the same error.

EDIT: Oops! I'm sorry. It was actually an annoying cloudfare cache that prevent the update from loading. 2.2.1 does not have this issue anymore 🥳

[venkatd]

@fishythefish thanks for resolving this. How do I get the fix? If I set my Dart version constraints to >= 2.13 would that resolve it?

[fishythefish]

@venkatd Yes, Dart 2.13 should have all the fixes in it. On the Flutter side, I believe this means 2.2 or higher should have the fix as well.