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

Only add defined callbacks to the stack #447

Merged
merged 1 commit into from
Nov 21, 2016

Conversation

darrachequesne
Copy link
Member

The kind of change this PR does introduce

  • a bug fix

Current behaviour

Before that commit, undefined callbacks were also added to the array,
which could lead to 'Maximum call stack size exceeded' error when
flush() method was called.

New behaviour

Other information (e.g. related issues)

Fixes #399
Fixes socketio/socket.io#2589

Before that commit, undefined callbacks were also added to the array,
which could lead to 'Maximum call stack size exceeded' error when
flush() method was called.

Fixes socketio#399
@3rd-Eden
Copy link
Contributor

Looks to me that you're fixing symptoms here instead of the actual bug, you probably want to prevent undefined's from ending up in that array in the first place.

@darrachequesne
Copy link
Member Author

@3rd-Eden my reasoning is that when using socket.send('a'), callback arg is undefined, right?

Socket.prototype.send =
Socket.prototype.write = function (data, options, callback) {
  this.sendPacket('message', data, options, callback);
  return this;
};

@darrachequesne
Copy link
Member Author

@3rd-Eden or have I misunderstood your comment?

@darrachequesne
Copy link
Member Author

Steps to reproduce:

var opts = { allowUpgrades: false, transports: ['websocket'] };
var engine = listen(opts, function (port) {
  var socket = new eioc.Socket('ws://localhost:%d'.s(port), { transports: ['websocket'] });
  engine.on('connection', function (conn) {
    for (var i = 0; i < 200000; i++) {
      conn.send('a');
    }
  });
});

@darrachequesne darrachequesne merged commit cd2ff46 into socketio:master Nov 21, 2016
@darrachequesne darrachequesne deleted the fix-callstacksize branch November 21, 2016 21:41
@darrachequesne darrachequesne added this to the 1.8.1 milestone Nov 27, 2016
darrachequesne pushed a commit that referenced this pull request May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants