Repository navigation
Stack overflow on memory page in dart2js html app build #1610
Description
Activity
@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).
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 @sigmundch - it looks like
JSON.stringifyis 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 - I believe the
JSON.stringifyis debug-only code (added while trying to figure out this problem)Looking at the code in
color.cleanit seems to be a recursive object traversal by iterating over the entries returned byObject.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?
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 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 - 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_jsThe unminified js code is here:
https://raw.githubusercontent.com/flutter/devtools/master/third_party/packages/plotly_js/lib/plotly.jsThe function
color.cleanthat 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 callscleanis 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.dartI 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.
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-rtiflag 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 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.
Repro instructions (I used the flutter/examples/hello_world app and an android emulator):
I am not able to reproduce this in ddc builds of Devtools.
Flutter doctor: