Stack overflow on memory page in dart2js html app build

Repro instructions (I used the flutter/examples/hello_world app and an android emulator):

flutter$ pub global activate devtools
flutter$ pub global run devtools
flutter$ cd examples/hello_world
hello_world$ flutter run
<open devtools & connect>
<select the memory page>

I am not able to reproduce this in ddc builds of Devtools.

Flutter doctor:

$ flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel master, v1.15.1-pre.42, on Linux, locale en_US.UTF-8)
[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
[✓] Chrome - develop for the web
[✓] Linux toolchain - develop for Linux desktop
[✓] Android Studio (version 3.5)
[✓] IntelliJ IDEA Ultimate Edition (version 2018.3)
[✓] IntelliJ IDEA Community Edition (version 2019.1)
[✓] IntelliJ IDEA Community Edition (version 2019.2)
[✓] IntelliJ IDEA Community Edition (version 2018.3)
[✓] VS Code (version 1.41.1)
[✓] Connected device (4 available)

@terrylucas

@terrylucas bisected the error to first occurring in

Flutter 1.13.8 • channel dev • https://github.com/flutter/flutter.git
Framework • revision 1c79347ef6 (4 weeks ago) • 2020-01-08 07:34:36 +0100
Engine • revision 3f52888b3b
Tools • Dart 2.8.0 (build 2.8.0-dev.0.0 bebc7d3af5)

Can you console.log(val)

I would not expect a @JS() interop type to have this property... This looks more like a Dart object.

This likely happened with the new RTI for dart2js

Nate what do you need from us to get this fixed? Its a bad break as the latest beta channel of Flutter (v1.14.6 from 2/5/2020) breaks DevTools as well as DevTools on dev channel is broken too (since 1.13.8 1/8/2020) for release builds of DevTools. Debug builds of DevTools work fine (they use ddc).

@bkonyi

@bkonyi

Nate our use of plotly JS library (is exposed as a package in third_party - plotly dart JS interop file). Do we need to re-publish this package with an updated pubspec? Does the new RTI require this?

Nate I pointed the plotly_js package to the source (instead of the package version) same problem (with a flutter pub get).

  plotly_js:
#    ^0.0.2
    path: ../../third_party/packages/plotly_js

@fishythefish

@fishythefish @sigmundch - it looks like JSON.stringify is not skipping rti related fields, perhaps?

Using Flutter 1.13.8 (which fails) console.log(val) returns this:

Using Flutter 1.13.7 (which works) below output:

1.13.7 scope (works):

1.13.8 scope (fails):

For reference, oldRTI gives this:

building with --use-old-rti fixes this issue.

We need to push a patch to the release that went out on Monday.

@vsmenon

@vsmenon - I believe the JSON.stringify is debug-only code (added while trying to figure out this problem)

Looking at the code in color.clean it seems to be a recursive object traversal by iterating over the entries returned by Object.keys(container). Our new RTI caches type data as an extra field property on objects and arrays. My guess is that they are being picked up by this code.

@fishythefish - could we address it if we use non-enumerable properties of some sort?

@natebosch

I just pushed out #1616, which fixes this issue for us.

@natebosch can you open an issue in dart2js to fix the underlying problem?

Tracking at dart-lang/sdk#40535

Thanks, Nate! Closing this Devtools-level issue as fixed.

@DaveShuckerow

@DaveShuckerow I want to re-open this issue because it is infeasible to 'prevent' reference cycles in the dart2js implementation of various Dart features.
The old rti is less efficient and will be removed soon so it is not a viable option beyond a few weeks.

We need more information.

Please set up a meeting to demonstrate the problem. I'd like to hand-edit the generated JavaScript to see if a modification to the reified type management would fix the problem.

/cc @natebosch

@rakudrama

@rakudrama - note the logic comes from a js library wrapped via js-interop under a package named plotly_js. You can find it here: https://github.com/flutter/devtools/tree/master/third_party/packages/plotly_js

The unminified js code is here:
https://raw.githubusercontent.com/flutter/devtools/master/third_party/packages/plotly_js/lib/plotly.js

The function color.clean that shows up in the stack trace is:

color.clean = function(container) {
    if(!container || typeof container !== 'object') return;

    var keys = Object.keys(container);
    var i, j, key, val;

    for(i = 0; i < keys.length; i++) {
        key = keys[i];
        val = container[key];

        // only sanitize keys that end in "color" or "colorscale"
        if(key.substr(key.length - 5) === 'color') {
            if(Array.isArray(val)) {
                for(j = 0; j < val.length; j++) val[j] = cleanOne(val[j]);
            }
            else container[key] = cleanOne(val);
        }
        else if(key.substr(key.length - 10) === 'colorscale' && Array.isArray(val)) {
            // colorscales have the format [[0, color1], [frac, color2], ... [1, colorN]]
            for(j = 0; j < val.length; j++) {
                if(Array.isArray(val[j])) val[j][1] = cleanOne(val[j][1]);
            }
        }
        // recurse into arrays of objects, and plain objects
        else if(Array.isArray(val)) {
            var el0 = val[0];
            if(!Array.isArray(el0) && el0 && typeof el0 === 'object') {
                for(j = 0; j < val.length; j++) color.clean(val[j]);
            }
        }
        else if(val && typeof val === 'object') color.clean(val);
    }
};

There is a stack trace on the top issue that might give you some hints into how this is being invoked from Dart. I believe the Dart program calls newPlot with a List<Data> which indirectly calls clean is on an element of the list. Data is an anonymous js-interop type, defined here: https://github.com/flutter/devtools/blob/master/third_party/packages/plotly_js/lib/plotly.dart

I don't believe we have reified .$ti on js-interop types, so my guess is that this is likely coming from some of the nested lists that are stored inside the Data objects.

@DaveShuckerow

The fix for this issue landed in dart-lang/sdk@8ae984c a few week ago and has been released in 2.8.0-dev.13.0

@DaveShuckerow - could you verify and remove the --use-old-rti flag on your build?

Let us know if this works out smoothly. If so, we should be all clear to delete the old rti soon afterwards.

@rakudrama

@rakudrama Based on our discussion, it looks like the fix doesn't work for the case of js interop with plotly.js. We were able to produce this issue in DevTools with the new fixed RTI.

The plotly.js package is traversing symbols as well as object.keys.

I'm going to create a new issue to track moving DevTools to the new RTI.

Closing this issue in favor of #1789.

Navigation Menu

Repository navigation

Stack overflow on memory page in dart2js html app build #1610

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Stack overflow on memory page in dart2js html app build #1610

Description

Activity

DaveShuckerow commented on Feb 7, 2020

natebosch commented on Feb 7, 2020

natebosch commented on Feb 7, 2020

terrylucas commented on Feb 7, 2020

terrylucas commented on Feb 7, 2020

terrylucas commented on Feb 7, 2020

terrylucas commented on Feb 7, 2020

vsmenon commented on Feb 7, 2020

terrylucas commented on Feb 7, 2020

terrylucas commented on Feb 7, 2020

terrylucas commented on Feb 7, 2020

DaveShuckerow commented on Feb 7, 2020

DaveShuckerow commented on Feb 7, 2020

sigmundch commented on Feb 7, 2020

DaveShuckerow commented on Feb 7, 2020

natebosch commented on Feb 7, 2020

DaveShuckerow commented on Feb 7, 2020

rakudrama commented on Feb 19, 2020

sigmundch commented on Feb 19, 2020

sigmundch commented on Feb 19, 2020

sigmundch commented on Mar 27, 2020

DaveShuckerow commented on Apr 3, 2020

DaveShuckerow commented on Apr 3, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions