Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random browser hang/crash using Area chart (on Chrome) #191

Closed
kidplug opened this issue Mar 6, 2013 · 24 comments
Closed

Random browser hang/crash using Area chart (on Chrome) #191

kidplug opened this issue Mar 6, 2013 · 24 comments

Comments

@kidplug
Copy link

kidplug commented Mar 6, 2013

I am experiencing random (intermittent) freezes and crashing when rendering an area chart in my app.
Sometimes it is on the initial creation - other times I "empty" the div then create a new area chart.

One time I did get evidence of an infinite loop in raphael -
Error parsing d H1810 << my best memory of the repeating error.

Unfortunately I'm not getting the error msg anymore in console - chrome is just hanging.

Here is my code to create the area chart:

var chart = this.line =  Morris.Area({
                    padding: padding,       // eg 10
                    element: $el,              // a div
                    data: data,                  //   [] length = 72 - see sample below
                    xkey: 'tm',
                    ykeys: ['wk','av','rst','ex'],
                    labels: ["Working","Available","Rest","Exception"],
                    lineColors: ["blue","gray","red","green"],
                    // lineWidth: null,
                    // pointSize: null,
                    pointFillColors: ["blue","gray","red","green"],
                    // pointStrokeColors: null,
                    ymax: "auto "+(sdc.ymax+2),        // eg 20
                    ymin: 0,
                    smooth: false,
                    hideHover: true,
                    //parseTime: null,
                    //postUnits,
                    //preUnits,
                    dateFormat: function(t) {
                        return util.dts(t);
                    },
                    //xLabels:
                    xLabelFormat: function(t) {
                        return "";
                    },
                    xLabelMargin: 0

                });
example data element:

av: 2
ex: 0
nc: 1
nm: 2
rst: 0
tm: 1362551400000
wk: 0

My div is styled like this:

<div class="wall-chart" style="left: -26px; width: 1828px;">

and is also position: absolute and height: 100% inside another div:

<div style="height: 78px">

That parent div has padding.

Any ideas?
Most of the time (8/10 times) the chart renders properly.

@kidplug
Copy link
Author

kidplug commented Mar 6, 2013

Also - i was previously using v 0.3.3 but I just upgraded to 0.4.1 to see if the issue would go away.
Using raphael v 2.1.0

@oesmith
Copy link
Contributor

oesmith commented Mar 6, 2013

Hmmmmm.. very interesting. Can you reproduce the problem in a jsbin (or similar)?

@kidplug
Copy link
Author

kidplug commented Mar 6, 2013

I thought you'd ask that!

I just tried to and WAS able to get chrome to hang.
http://jsbin.com/ugavuz/3/

I haven't used jsbin before- but it seems that if you edit anything it re-renders the iframe.
I was able to get chrome to hang very easily by modifying the "width" of the "chart" div in the html pane.

@kidplug
Copy link
Author

kidplug commented Mar 6, 2013

Try this link - you need to be in edit mode;
http://jsbin.com/ugavuz/4/edit

Watch the console while it renders the iframe. It should print "finished" after drawing the chart.
Eventually for me, it hangs and never reaches that point. Then I have to kill the whole tab in chrome.

@oesmith
Copy link
Contributor

oesmith commented Mar 6, 2013

Whoah! That's the first time I've seen Chrome get stuck like that. I'd guess that's something upsetting the Chrome renderer itself -- usually if a bit of javascript gets stuck in an infinite loop then the tab can usually detect that and kill itself.

I'll see if I can ask the Chrome dev team at work tomorrow if they have any ideas..

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Hm - like I said in earlier comment - sometimes during the "freeze" the console was actually spewing errors from within the Raphael js code - stuck in an infinite loop of some kind...

I think the line in the morris code was this:

line 384 i believe
          _results.push(this.drawGridLine("M" + this.left + "," + y + "H" + (this.left + this.width)));

I think thats where the H1810 came from in the "Error parsing" exception / stacktrace.

If this were a Chrome rendering issue, I don't think I would have gotten the "looping" symptom.
Also in my experience Chrome usually DOES kill the tab with "Aw, Snap".

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Doing some debugging:
http://jsbin.com/ugavuz/18/edit

Got it to hang after "debug 11" - seems to be triggered by reducing the width of the chart div (eg: 33px).
So the freeze is occuring in:

this.drawXAxis();  (Line.prototype)

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Found the stuck loop!
try this: http://jsbin.com/ugavuz/24/edit
Reduce the width to 33px (instead of 331px).
My console is spewing: debug dx-loop:
In normal / successful completion, this loop should only be 17 iterations.

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Looks like labelSeries is returning 1.5 million labels instead of 17. Must be triggered by a width too small.
---debug dx6b: 1533601

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

So in labelSeries:

console.log("==ls: "+dmin+" "+dmax+" "+pxwidth);
output:
==ls: 1366434000000 1367967600000 -1 

Looks like it is getting a negative value for pxwidth - probably due to the narrow width (33px) and my declared padding of 10px.

This results in the endless loop in labelSeries.
Now I'm not sure this is the cause of the issue in my app, since I am NOT using a narrow width - my width is more like 1800px. So I will play around with some debugging there and report back.

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Also here is the latest jsbin showing the 1.5 million labels returned:
http://jsbin.com/ugavuz/25/edit

This one actually wont hang the browser because I added a safety break out in the next loop.

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

The hang in my app is occurring in labelSeries but NOT due to negative pxwidth!
Analyzing now...

@oesmith
Copy link
Contributor

oesmith commented Mar 7, 2013

Nice one! Top debugging effort there!

On 7 March 2013 10:27, kidplug notifications@github.com wrote:

The hang in my app is occurring in labelSeries but NOT due to negative
pxwidth!
Analyzing now...


Reply to this email directly or view it on GitHubhttps://github.com/oesmith/morris.js/issues/191#issuecomment-14552446
.

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Thanks - so here is output from my app in the labelSeries loop, building the ret array.
First value is "t" and second value is size of ret[].
The length SHOULD be 61 on successful loop.
Notice that on iteration #59, t has NOT increased! So I guess some issue in the "incrementer" ?

===ls loop: 1362870000000 51 
===ls loop: 1362873600000 52
===ls loop: 1362877200000 53 
===ls loop: 1362880800000 54 
===ls loop: 1362884400000 55 
===ls loop: 1362888000000 56 
===ls loop: 1362891600000 57 
===ls loop: 1362895200000 58 
===ls loop: 1362895200000 59 
===ls loop: 1362895200000 60 
===ls loop: 1362895200000 61 
===ls loop: 1362895200000 62 
===ls loop: 1362895200000 63 

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

OK guess what...
the incrementer breaks down when crossing day light savings time!
d.setMinutes(d.getMinutes()+60) - winds up with SAME value

msh.incr: Sun Mar 10 2013 00:00:00 GMT-0500 (Eastern Standard Time) min:0 interval:60 morris-0.4.1.js:1006
===ls loop: 1362895200000 55 morris-0.4.1.js:985
msh.incr: Sun Mar 10 2013 01:00:00 GMT-0500 (Eastern Standard Time) min:0 interval:60 morris-0.4.1.js:1006
===ls loop: 1362895200000 56 morris-0.4.1.js:985
msh.incr: Sun Mar 10 2013 01:00:00 GMT-0500 (Eastern Standard Time) min:0 interval:60 morris-0.4.1.js:1006
===ls loop: 1362895200000 57 morris-0.4.1.js:985
msh.incr: Sun Mar 10 2013 01:00:00 GMT-0500 (Eastern Standard Time) min:0 interval:60 morris-0.4.1.js:1006
===ls loop: 1362895200000 58 morris-0.4.1.js:985
msh.incr: Sun Mar 10 2013 01:00:00 GMT-0500 (Eastern Standard Time) min:0 interval:60 morris-0.4.1.js:1006
===ls loop: 1362895200000 59 morris-0.4.1.js:985
msh.incr: Sun Mar 10 2013 01:00:00 GMT-0500 (Eastern Standard Time) min:0 interval:60 morris-0.4.1.js:1006
===ls loop: 1362895200000 60 

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Should probably change to getTime() / setTime() using actual milliseconds for interval.

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

So to summarize -

  1. endless loop due to "negative" pxwidth
  2. endless loop due to incrementer failing to cross into DST

(Both issues occur in labelSeries fn)

@oesmith
Copy link
Contributor

oesmith commented Mar 7, 2013

Excellent work. Best bug report ever :)

On 7 March 2013 10:50, kidplug notifications@github.com wrote:

So to summarize -

  1. endless loop due to "negative" pxwidth
  2. endless loop due to incrementer failing to cross into DST


Reply to this email directly or view it on GitHubhttps://github.com/oesmith/morris.js/issues/191#issuecomment-14553481
.

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

Thanks! and thanks for all your excellent work on this project!

@kidplug
Copy link
Author

kidplug commented Mar 7, 2013

So here is my temporary fix / diff

diff -r1.1 morris-0.4.1.js
987a988
>     var span = interval * 60 * 1000;        // millis
989c990
<       span: interval * 60 * 1000,
---
>       span: span,
997c998,999
<         return d.setMinutes(d.getMinutes() + interval);
---
>         //return d.setMinutes(d.getMinutes() + interval);
>           return d.setTime(d.getTime()+span);
1002a1005
>     var span = interval * 1000;
1004c1007
<       span: interval * 1000,
---
>       span: span,
1012c1015,1016
<         return d.setSeconds(d.getSeconds() + interval);
---
>         //return d.setSeconds(d.getSeconds() + interval);
>           return d.setTime(d.getTime()+span);

@oesmith
Copy link
Contributor

oesmith commented Mar 14, 2013

I've committed fixes for both of these to master now.

Thanks again for the bug report!

@oesmith oesmith closed this as completed Mar 14, 2013
@AJamesPhillips
Copy link

kidplug's return d.setTime(d.getTime()+span); doesn't appear in master yet. This causes a crash with the following code:

date = new Date("Sun Mar 31 2013 00:00:00 GMT+0000 (GMT)")
dmin = date.getTime()-(1000*60*60*24)
dmax = date.getTime()+(1000*60*60*24)
// ddensity needs to be >= s.span of 60 * 60 * 1000 so that we get "hour" as x label
ddensity = 60*60*1000
pxwidth = 200 * (dmax - dmin) / ddensity

 // The following line will loop infinitely
Morris.labelSeries(date.getTime()-60, date.getTime()+60, pxwidth, "hour")

It happens because when interval == 60:

d = new Date("Sun Mar 31 2013 00:00:00 GMT+0000 (GMT)")
d.setSeconds(d.getSeconds() + interval)
//d is still equal to Sun Mar 31 2013 00:00:00 GMT+0000 (GMT)

An alternative fix is: return d.setUTCMinutes(d.getMinutes() + interval);

@oesmith
Copy link
Contributor

oesmith commented Apr 2, 2013

The fix is already in master (note how 01:00 doesn't appear in the output below).

var d1 = new Date(2013, 2, 30, 12); 
var d2 = new Date(2013, 2, 31, 12);

var labels = Morris.labelSeries(d1, d2, 1000, "hours");
console.log($.map(labels, function (x) { return x[0] }));

=> ["12:00", "13:00", "14:00", "15:00", "16:00", "17:00", "18:00", "19:00", "20:00", "21:00", "22:00", "23:00", "00:00", "02:00", "03:00", "04:00", "05:00", "06:00", "07:00", "08:00", "09:00", "10:00", "11:00", "12:00"]

@AJamesPhillips
Copy link

Apologies, I don't know what went wrong there but I obviously didn't use the latest code. (minor point, just in case any one's confused later by this, labelSeries needs "hour" rather than "hours"). Thanks for the excellent and continued work, much appreciated @oesmith.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants