Skip to content

Commit

Permalink
net: omit superfluous 'connect' event
Browse files Browse the repository at this point in the history
Don't emit a 'connect' event on sockets that are handed off to
net.Server 'connection' event listeners.

1. It's superfluous because the connection has already been established
   at that point.

2. The implementation is arguably wrong because the event is emitted on
   the same tick of the event loop while the rule of thumb is to always
   emit it on the next one.

This has been tried before in commit f0a440d but was reverted again in
ede1acc because the change was incomplete (at least one test hadn't
been updated).

Fixes #1047 (again).
  • Loading branch information
bnoordhuis committed Mar 1, 2013
1 parent bb43153 commit c116120
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 14 deletions.
1 change: 0 additions & 1 deletion lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,6 @@ function onconnection(clientHandle) {
DTRACE_NET_SERVER_CONNECTION(socket);
COUNTER_NET_SERVER_CONNECTION(socket);
self.emit('connection', socket);
socket.emit('connect');
}


Expand Down
15 changes: 6 additions & 9 deletions test/pummel/test-net-throttle.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,12 @@ for (var i = 0; i < N; i++) {
console.log('start server on port ' + common.PORT);

var server = net.createServer(function(connection) {
connection.on('connect', function() {
connection.write(body.slice(0, part_N));
connection.write(body.slice(part_N, 2 * part_N));
assert.equal(false, connection.write(body.slice(2 * part_N, N)));
console.log('bufferSize: ' + connection.bufferSize);
assert.ok(0 <= connection.bufferSize &&
connection.bufferSize <= N);
connection.end();
});
connection.write(body.slice(0, part_N));
connection.write(body.slice(part_N, 2 * part_N));
assert.equal(false, connection.write(body.slice(2 * part_N, N)));
console.log('bufferSize: ' + connection.bufferSize);
assert.ok(0 <= connection.bufferSize && connection.bufferSize <= N);
connection.end();
});

server.listen(common.PORT, function() {
Expand Down
6 changes: 2 additions & 4 deletions test/simple/test-net-reconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ var server = net.createServer(function(socket) {
console.error('SERVER: got socket connection');
socket.resume();

socket.on('connect', function() {
console.error('SERVER connect, writing');
socket.write('hello\r\n');
});
console.error('SERVER connect, writing');
socket.write('hello\r\n');

socket.on('end', function() {
console.error('SERVER socket end, calling end()');
Expand Down

0 comments on commit c116120

Please sign in to comment.