dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.58k forks source link

Incorrect behavior of try/catch within finally within async* method #57046

Closed mraleph closed 6 days ago

mraleph commented 2 weeks ago

The following code shows different behavior on the Web and on the VM. It seems in dart2js case exception thrown from (*) is clobbering the exception which is currently in flight.

Future<void> something(int v) async {
  await Future.delayed(Duration(milliseconds: 10));
  throw StateError('error $v');
}

Stream<int> _readLoop() async* {
  try {
    while (true) {
      yield 1;
      await something(0);
    }
  } catch (e) {
    throw StateError('converted');
  } finally {
    try {
      await something(1);  // (*)
    } catch (e) {
      print('Ignored: $e');
    }
  }
}

void main() async {
  try {
    await for (var _ in _readLoop()) {
    }
  } catch (e) {
    print('Caught: $e');
  }
}
$ dart --version
Dart SDK version: 3.7.0-112.0.dev (dev) (Wed Nov 6 08:02:51 2024 -0800) on "macos_arm64"
$ dart test.dart
Ignored: Bad state: error 1
Caught: Bad state: converted
$ dart compile js test.dart
$ ~/.jsvu/v8 /Users/vegorov/src/flutter/flutter/bin/cache/dart-sdk/lib/_internal/js_runtime/lib/preambles/d8.js out.js
Ignored: Bad state: error 1
Caught: Bad state: error 1

DDC might also have a problem with desugarring but of a different nature because in DartPad I get unhandled StateError.

/cc @natebiggs

mnordine commented 2 weeks ago

I think your program maybe should have:

print('Caught: $e');

instead of:

print(e);

to produce the output you gave?

mraleph commented 2 weeks ago

@mnordine good catch :) fixed it.

biggs0125 commented 1 week ago

@mraleph Your initial assessment was right, dart2js's async control flow state machine is keeping a single 'error' variable per async function. If we nest error handlers then the inner handler will clobber the value already set by the outer one.

I have a fix that I'm verifying: https://dart-review.googlesource.com/c/sdk/+/395580 The fix keeps a stack of errors rather than a single error variable and pushes/pops as we enter/exit error handling scopes.