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

Out of stack space error in IE #502

Closed
satazor opened this issue Jul 8, 2012 · 23 comments
Closed

Out of stack space error in IE #502

satazor opened this issue Jul 8, 2012 · 23 comments
Labels
type: bug a defect, confirmed by a maintainer

Comments

@satazor
Copy link

satazor commented Jul 8, 2012

Large amounts of tests often cause "out of stack space" errors in Internet Explorer (IE <= 9).
After analyzing the call stack when the error is thrown, it is huge!

If I let the tests "breath":

  process.nextTick(function(){
    setTimeout(function () {
      next(0);
    }, 5);
  });

.. all is fine.

The project in which this is happening has 258 asserts and a total of 102 tests.

@liammclennan
Copy link

I get this error too. The specific error is: " Unable to get value of the property 'appendChild': object is null or undefined" thrown from runner.on('test end':

stack[0].appendChild(el);

for some reason stack is []

@satazor
Copy link
Author

satazor commented Jul 16, 2012

@liammclennan that also happens to me sometimes after I get the stack space error on IE and refresh

@mxriverlynn
Copy link

we're getting this error in IE10 as well.

does anyone have any clue what's going on? is this a design problem in mocha using recursion to run tests, under a limited stack frame environment? or ???

any help is appreciated, as we are running in to what looks like a limit of around 20 it blocks.

the problem we're seeing is that the tests complete (pass or fail), and then trying to report the problem fails at line 1753:

stack[0].appendChild(el);

stack on this line is an empty array.

@OscarGodson
Copy link
Contributor

Uh oh. EpicEditor is nearing over 100 tests as well and @johnmdonahue and I were going to switch over to Mocha. I'll be keeping an eye on this ticket.

@mxriverlynn
Copy link

i've spent the better part of the day trying to track this problem down, and here's what i've come up with: Mocha uses recursion to run the specs.

I'm seeing the Runner.runSuite called over and over and over again. I have a test suite with 8 file, 13 "describe" blocks, 22 "it" blocks. By the time it gets down to the last of the it blocks in the last of the describe blocks, there are 535 items on the JavaScript call stack (that is, 535 stack frames).

It looks like the stack is reset at the beginning of each file, some times... but at other times it looks like it is a cumulative stack for all files. It's quite confusing and I'm having a hard time understanding what circumstances would cause such a deep stack trace, especially when the file that finally runs in to this problem only had 3 "describe" and 6 "it" blocks in it.

trying to dig in to this further, but any help from @visionmedia would be greatly appreciated. i don't want to have to go back to jasmine with it's awful "waitsFor" and "runs" methods for async support.

@tj
Copy link
Contributor

tj commented Jul 27, 2012

that's why we have some next ticks in there to kill the stack, we just need a few more I guess

@tj
Copy link
Contributor

tj commented Jul 27, 2012

I haven't seen this myself personally though, maybe im not writing enough tests haha

@mxriverlynn
Copy link

next update...

i figured out that the scenario in which i thought recursion was due to a new file, was due to a suite of async tests and them not completing before some synchronous tests.

that made me thing: a deep stack trace can be cut by using any async call to push something to the back of the javascript timer/queue (whatever that's called). a simple setTimeout should do that...

The Workaround

At the beginning of every .spec file, just inside of your wrapping describe block, add an async beforeEach and do a setTimeout(done, 0).

describe("the contents of this file", function(){

  // blow away the stack trace here
  beforeEach(function(done){
    setTimeout(done, 0);
  });

  // do my tests here
  describe("whatever", function(){
    // ...
  });

  // do my tests here
  describe("whatever", function(){
    // ...
  });

  // do my tests here
  describe("whatever", function(){
    // ...
  });


});

The potential problem with this workaround, is that if you have a large spec file with many "describe" and "it" blocks in it, or a deep stack trace due to your own code, you can still run in to this problem. The continued workaround for that, though, is to call setTimeout(done, 0) multiple times - either within the same file, or splitting multiple specs in to multiple files.

At least this is a workaround for now, as ugly as it is.

Can we get a legit fix to this, though? The specs shouldn't be called recursively, IMO. Is there a reason that it was designed this way?

@mxriverlynn
Copy link

an even better work around:

create a helper.js file (or open the one you already have) and just put the beforeEach block with the setTimeout(done,0) in that directly, with no describe around it.

this way you only have to do it once, not in every file. it will get run before all of your specs and you'll never have to worry about this again.

@tj
Copy link
Contributor

tj commented Jul 27, 2012

nothing is wrong with recursion at all, we just need to drop the stack every so often, recursion simplifies many things internally

@mxriverlynn
Copy link

nothing is wrong with recursion at all, we just need to drop the stack every so often, recursion simplifies many things internally

while i agree with the first part of that on it's own, when the qualification of the second part is a necessity, then recursion is not an appropriate solution for the situation.

... but... yeah :) a fix in Mocha would be awesome. the setTimeout trick works for now.

@tj
Copy link
Contributor

tj commented Jul 27, 2012

when you can nextTick in a tiny fraction of a second it's not an issue, implementing actions against a tree like we have here would get very messy when combined with async callback crap all over, if we used coroutines then yeah maintaining our own stack would be fine

@satazor
Copy link
Author

satazor commented Jul 28, 2012

@derickbailey thanks for the quick fix, gonna apply that until this issue is resolved

@molily
Copy link

molily commented Sep 21, 2012

Any plans to fix this soon? This seems to kill our test suite in IE8 completely. The IE process takes 1.5 GB of memory and then fails with an out of memory error. The setTimeout workaround didn’t help. I know no-one here cares about old IEs, but any real-world client-side JavaScript has to.

@tj
Copy link
Contributor

tj commented Sep 21, 2012

I dont have any windows vms going so I can't test it unfortunately, not any time soon at least. When I get my old MBP fixed I'll chuck ugly old windows on there

@arian
Copy link
Contributor

arian commented Jan 22, 2013

The workaround fixed the tests in IE (kamicane/prime@762be12)

@vmeurisse
Copy link

visionmedia commented:
that's why we have some next ticks in there to kill the stack, we just need a few more I guess

The problem is that process.nextTick doesn't kill the stack in IE. Here is an implementation which does:

process.nextTick = (function(){
  var timeouts = []
  // postMessage behaves badly on IE8
  if (window.ActiveXObject || !window.postMessage) {
    return function(fn){
      timeouts.push(fn);
      setTimeout(function(){
        if (timeouts.length) timeouts.shift()();
      }, 0);
    }
  }

  // based on setZeroTimeout by David Baron
  // - http://dbaron.org/log/20100309-faster-timeouts
  var name = 'mocha-zero-timeout'

  window.addEventListener('message', function(e){
    if (e.source == window && e.data == name) {
      if (e.stopPropagation) e.stopPropagation();
      if (timeouts.length) timeouts.shift()();
    }
  }, true);

  return function(fn){
    timeouts.push(fn);
    window.postMessage(name, '*');
  }
})();

It also fix issue #737.

@jvilk
Copy link

jvilk commented Feb 27, 2013

I found this while Googling for an appropriate mechanism for resetting IE8's stack for one of my projects.

While setTimeout works, it has a minimum wait time (4ms in the spec, but apparently older browsers had something like 10ms of delay). This is probably fine for tests, but less than ideal for actual applications that have to call this method often.

If anyone figures out a way to clear the stack in IE8 and trigger the callback immediately, let me know. Conversely, if I find anything, I'll update here.

@jvilk
Copy link

jvilk commented Feb 27, 2013

I found this project:
https://github.com/NobleJS/setImmediate

They have a solution for IE6-8, which is quite the hack:
https://github.com/NobleJS/setImmediate/blob/master/setImmediate.js#L161

It inserts a script into the page with a onreadystatechange callback which triggers the task. Totally hacky, but... I'll see if it ends up working for me.

@machineghost
Copy link

+1

@OscarGodson
Copy link
Contributor

BALLS. As I called it, this is now happening to me in IE9 as well as 10. :(

@ghost
Copy link

ghost commented Sep 8, 2013

Code Bounty

@boneskull boneskull added type: bug a defect, confirmed by a maintainer Browser - IE labels Nov 7, 2014
@jbnicolai
Copy link

I believe this is no longer an issue, with the current version of mocha.

If I'm mistaken, please comment and I'll re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests