Sdk: DDC "hot restart" not clearing timers

Created on 21 Oct 2019  路  4Comments  路  Source: dart-lang/sdk

DDC doesn't seem to be clearing timers and microtasks on hot restart. This is causing problems in flutter web (e.g. https://github.com/flutter/flutter/issues/41284).

Repro

  1. Run the flutter app below.
  2. Click the text field and type "aaa".
  3. Immediately go back to the terminal and see "Timer started" printed.
  4. Trigger a hot restart by clicking "R" in the terminal.
  5. Wait for a few seconds and see "Timer triggered" printed in the terminal.
import 'dart:async';

import 'package:flutter/material.dart';

void main() {
  runApp(App());
}

class App extends StatefulWidget {
  @override
  _AppState createState() => _AppState();
}

class _AppState extends State<App> {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: TextField(
          onChanged: (String text) {
            if (text == 'aaa') {
              print('Timer started');
              Timer(const Duration(seconds: 10), () {
                print('Timer triggered');
              });
            }
          },
        ),
      ),
    );
  }
}

Behavior on Android

In step 5, "Time triggered" won't be printed because the timer has been canceled (or as I was told, the whole Dart isolate was torn down, so all timers are gone with it).

Behavior on web

(using flutter run -d chrome)

In step 5, "Time triggered" is printed because timers aren't being cleared by DDC.

You'll also notice errors like NoSuchMethodError: invalid member on null: 'findRenderObject'. These are happening because timers aren't cleared.

cc @vsmenon @jakemac53 @jonahwilliams

area-web dev-compiler-sdk web-dev-compiler

Most helpful comment

The fix landed in master at cccb9ffb2ab3268389a08d20261861115864156c - @vsmenon verified with a flutter client that this addresses the issues in the sample above.

All 4 comments

Hmm, this might be fixable by adding a check here:

https://github.com/dart-lang/sdk/blob/master/sdk/lib/_internal/js_dev_runtime/private/isolate_helper.dart#L43

and, similarly, below that the isolate is still alive. We may have other DOM entry points too though (e.g., addEventListener, Dart callbacks passed to JS via allowInterop). We may also want to consider a general way to wrap these.

@sigmundch @natebosch

@vsmenon we handle addEventListener clean up in the flutter web engine right now:

developer.registerExtension('ext.flutter.disassemble', (_, __) {
  // Remove DOM elements and event listeners.
  return Future<developer.ServiceExtensionResponse>.value(developer.ServiceExtensionResponse.result('OK'));
});

But this means we have to remember to do this in many places. So it would be super helpful if DDC can wrap event listeners and remove them on hot restart.

As for DOM elements, I think we will still have to remove them manually but there are fewer callsites that create/insert DOM elements.

the event listeners are a bit trickier to handle because of how dart:html is shared with dart2js, but here is a cl that addresses the immediate issue with timers: https://dart-review.googlesource.com/c/sdk/+/122522

The fix landed in master at cccb9ffb2ab3268389a08d20261861115864156c - @vsmenon verified with a flutter client that this addresses the issues in the sample above.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

DartBot picture DartBot  路  3Comments

55555Mohit55555 picture 55555Mohit55555  路  3Comments

xster picture xster  路  3Comments

jmesserly picture jmesserly  路  3Comments

gspencergoog picture gspencergoog  路  3Comments