sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Crash in html_dart2js.dart

Open annagrin opened this issue 3 years ago • 1 comments

Original issue in Flutter: https://github.com/flutter/flutter/issues/101647

Rare issue seen in production on Flutter web when calling myStreamSubscription.cancel() in StatefulWidget dispose(). Appears to be a race condition and in release mode only. Not sure how to reproduce.

Suggestion: is it possible to prevent those errors by checking for null in _StreamController._recordCancel in the top of the stack below? Or is the error non-recoverable and is supposed to crash?

NoSuchMethodError: method not found: 'toString' on null
  at _StreamController._recordCancel(org-dartlang-sdk:///lib/html/dart2js/html_dart2js.dart:16962:34)
  at _ControllerSubscription._onCancel(org-dartlang-sdk:///lib/async/stream_controller.dart:722:30)
  at _BufferingStreamSubscription._cancel(org-dartlang-sdk:///lib/async/stream_controller.dart:850:12)
  at _BufferingStreamSubscription.cancel(org-dartlang-sdk:///lib/async/stream_impl.dart:252:5)
  at _ForwardingStreamSubscription._onCancel(org-dartlang-sdk:///lib/async/stream_impl.dart:198:7)
  at _BufferingStreamSubscription._cancel(org-dartlang-sdk:///lib/async/stream_pipe.dart:145:7)
  at _BufferingStreamSubscription.cancel(org-dartlang-sdk:///lib/async/stream_impl.dart:252:5)

annagrin avatar Sep 15 '22 17:09 annagrin

Note: toString on null, is how null asserts errors show up in dart2js compiled apps.

It appears this error likely comes form this logic:

16956     int? watchId;
16957     StreamController<Geoposition> controller =
16958         new StreamController<Geoposition>(
16959             sync: true,
16960             onCancel: () {
16961               assert(watchId != null);
16962               _clearWatch(watchId!);  /// << this null assert
16963             });
16964     controller.onListen = () {
16965       assert(watchId == null);
16966       watchId = _watchPosition((position) {
16967         controller.add(_ensurePosition(position));
16968       }, (error) {
16969         controller.addError(error);
16970       }, options);
16971     };

Either the logic listening is getting cancelled before the watch position has been set, or some other understanding of nullability is incorrect about the DOM API. So as you suggest, yes, we could instead call _clearWatch conditionally or make _clearWatch accept nullable values (if the browser accepts it).

sigmundch avatar Sep 15 '22 20:09 sigmundch

Seeing many cases of this, especially when accessing env variables.

Opened issue https://github.com/invertase/dart_edge/issues/33 originally but am experiencing this with flutter web as well now which led me here.

MichealReed avatar May 12 '23 16:05 MichealReed

Any update on this?

wer-mathurin avatar Feb 05 '24 11:02 wer-mathurin

Note: toString on null, is how null asserts errors show up in dart2js compiled apps.

It appears this error likely comes form this logic:

16956     int? watchId;
16957     StreamController<Geoposition> controller =
16958         new StreamController<Geoposition>(
16959             sync: true,
16960             onCancel: () {
16961               assert(watchId != null);
16962               _clearWatch(watchId!);  /// << this null assert
16963             });
16964     controller.onListen = () {
16965       assert(watchId == null);
16966       watchId = _watchPosition((position) {
16967         controller.add(_ensurePosition(position));
16968       }, (error) {
16969         controller.addError(error);
16970       }, options);
16971     };

Either the logic listening is getting cancelled before the watch position has been set, or some other understanding of nullability is incorrect about the DOM API. So as you suggest, yes, we could instead call _clearWatch conditionally or make _clearWatch accept nullable values (if the browser accepts it).

Maybe we could also keep the same logic by changing the assert by throwing an exception?

wer-mathurin avatar Feb 05 '24 11:02 wer-mathurin

Given the fix was very localized and small I've sent https://dart-review.googlesource.com/c/sdk/+/350982 to address this issue.

That said, please note that, while we still support fixing critical issues in dart:html, we are not investing much in it. Instead, our plan is to move towards package:web. If package:web provides the API you need already, consider giving it a try. We're in the middle of writing some documentation to share more details about this soon.

sigmundch avatar Feb 08 '24 21:02 sigmundch