Skip to content

Commit 9b55e47

Browse files
indutnyMyles Borins
authored and
Myles Borins
committed
net: use _server for internal book-keeping
The role of `this.server` is now split between `this._server` and `this.server`. Where the first one is used for counting active connections of `net.Server`, and the latter one is just a public API for users' consumption. The reasoning for this is simple, `TLSSocket` instances wrap `net.Socket` instances, thus both refer to the `net.Server` through the `this.server` property. However, only one of them should be used for `net.Server` connection count book-keeping, otherwise double-decrement will happen on socket destruction. Fix: #5083 PR-URL: #5262 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 6a42204 commit 9b55e47

File tree

4 files changed

+44
-10
lines changed

4 files changed

+44
-10
lines changed

lib/_tls_wrap.js

-6
Original file line numberDiff line numberDiff line change
@@ -392,12 +392,6 @@ TLSSocket.prototype._init = function(socket, wrap) {
392392

393393
this.server = options.server;
394394

395-
// Move the server to TLSSocket, otherwise both `socket.destroy()` and
396-
// `TLSSocket.destroy()` will decrement number of connections of the TLS
397-
// server, leading to misfiring `server.close()` callback
398-
if (socket && socket.server === this.server)
399-
socket.server = null;
400-
401395
// For clients, we will always have either a given ca list or be using
402396
// default one
403397
const requestCert = !!options.requestCert || !options.isServer;

lib/net.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,10 @@ function Socket(options) {
171171
this.read(0);
172172
}
173173
}
174+
175+
// Reserve properties
176+
this.server = null;
177+
this._server = null;
174178
}
175179
util.inherits(Socket, stream.Duplex);
176180

@@ -481,12 +485,12 @@ Socket.prototype._destroy = function(exception, cb) {
481485
this.destroyed = true;
482486
fireErrorCallbacks();
483487

484-
if (this.server) {
488+
if (this._server) {
485489
COUNTER_NET_SERVER_CONNECTION_CLOSE(this);
486490
debug('has server');
487-
this.server._connections--;
488-
if (this.server._emitCloseIfDrained) {
489-
this.server._emitCloseIfDrained();
491+
this._server._connections--;
492+
if (this._server._emitCloseIfDrained) {
493+
this._server._emitCloseIfDrained();
490494
}
491495
}
492496
};
@@ -1416,6 +1420,7 @@ function onconnection(err, clientHandle) {
14161420

14171421
self._connections++;
14181422
socket.server = self;
1423+
socket._server = self;
14191424

14201425
DTRACE_NET_SERVER_CONNECTION(socket);
14211426
LTTNG_NET_SERVER_CONNECTION(socket);

test/parallel/test-net-stream.js

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ var s = new net.Stream();
1111

1212
s.server = new net.Server();
1313
s.server.connections = 10;
14+
s._server = s.server;
1415

1516
assert.equal(10, s.server.connections);
1617
s.destroy();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
if (!common.hasCrypto) {
5+
console.log('1..0 # Skipped: missing crypto');
6+
return;
7+
}
8+
9+
const assert = require('assert');
10+
const tls = require('tls');
11+
const fs = require('fs');
12+
13+
const options = {
14+
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
15+
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
16+
};
17+
18+
const server = tls.createServer(options, function(s) {
19+
s.end('hello');
20+
}).listen(common.PORT, function() {
21+
const opts = {
22+
port: common.PORT,
23+
rejectUnauthorized: false
24+
};
25+
26+
server.on('connection', common.mustCall(function(socket) {
27+
assert(socket.server === server);
28+
server.close();
29+
}));
30+
31+
const client = tls.connect(opts, function() {
32+
client.end();
33+
});
34+
});

0 commit comments

Comments
 (0)