Maximum call stack size exceeded #143

Closed
MSNexploder opened this Issue Sep 27, 2011 · 6 comments

Projects

None yet

3 participants

@MSNexploder

The following code dies using node v0.4.12.
I was able to track down the problem within the nowUtil.extend function, which gets recursively for deeply nested objects until stack size exceeds. More precisely it looks like it couldn't handle circular references properly.

But I couldn't come up with a not too hacky solution for this.

var io = require('socket.io');
var now = require('now');
var express = require('express');

var server = express.createServer();
var store = new io.RedisStore();

var everyone = now.initialize(server, {socketio: {store: store}});
// => RangeError: Maximum call stack size exceeded
@ericz
ericz commented Oct 3, 2011

Indeed the extend function doesn't currently handle circular references correctly causing the error. Not sure if we're going to fix this atm

@steveWang

Eric -- as a quick fix (and the least hacky method -- decycle / retrocycle is not really a good option), would you consider changing it such that nowjs.initialize modifies the input object, as opposed to the default settings object (how I believe things are currently done)?

I haven't looked at the code in some time, but this might be as simple as reversing the order of the arguments to nowUtil.extend.

edit: it's slightly more complicated. I think the fix would be to change nowUtil.extend(this.options, options); to this.options = nowUtil.extend(options, this.options);.

@ericz
ericz commented Oct 4, 2011

Yeahhh you want the defaults to be overwritten so it has to extend into this.options or extra memory will be used

@steveWang

Erm, all right, it's not as simple as this, but the main idea is there. You'd actually want to use a slight variant of extend that only adds fields if they didn't already exist in the target.

I think it might be something like the following:

function recurse(source, dest) {
  for (var i in source) {
    if (source[i] && typeof source[i] === 'object') {
      if (!dest[i] || typeof dest[i] !== 'object') {
        dest[i] = Array.isArray(source[i]) ? [] : {};
      }
      recurse(source[i], dest[i]);
    }
    else if (dest[i] === undefined) {
      dest[i] = source[i];
    }
  }
}

Also, sorry, I don't follow -- does it really use more memory if you extend into the other object, then overwrite this.options?

@ericz
ericz commented Oct 4, 2011

makes sense. i'll implement that

On Tue, Oct 4, 2011 at 8:33 AM, Steve Wang <
reply@reply.github.com>wrote:

Erm, all right, it's not as simple as this, but the main idea is there.
You'd actually want to use a slight variant of extend that only adds fields
if they didn't already exist in the target.

Reply to this email directly or view it on GitHub:
#143 (comment)

510-691-3951
EECS Student at UC Berkeley
http://ericzhang.com

@ericz ericz added a commit that referenced this issue Oct 4, 2011
@ericz ericz #143 extend optimization a043fb1
@ericz
ericz commented Oct 4, 2011

Fixed

@ericz ericz closed this Oct 4, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment