From 9db95288c3c4673edf6397e695b09b3cee02ab2f Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 26 Apr 2016 16:27:08 -0400 Subject: [PATCH 1/3] net: introduce `Socket#connecting` property There is no official way to figure out if the socket that you have on hand is still connecting to the remote host. Introduce `Socket#connecting`, which is essentially an unprefixed `_connecting` property that we already had. --- doc/api/net.md | 6 ++++ lib/_tls_legacy.js | 2 +- lib/_tls_wrap.js | 10 +++--- lib/net.js | 35 ++++++++++++--------- test/parallel/test-net-connect-buffer.js | 2 +- test/parallel/test-net-socket-connecting.js | 21 +++++++++++++ 6 files changed, 55 insertions(+), 21 deletions(-) create mode 100644 test/parallel/test-net-socket-connecting.js diff --git a/doc/api/net.md b/doc/api/net.md index 7a58bf23108710..0cc23c05bceeb0 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -333,6 +333,12 @@ socket as reported by the operating system. Returns an object with three properties, e.g. `{ port: 12346, family: 'IPv4', address: '127.0.0.1' }` +### socket.connecting + +If `true` - [`socket.connect(options\[, connectListener\])`][] was called and +haven't yet finished. Will be set to `false` before emitting `connect` event +and/or calling [`socket.connect(options\[, connectListener\])`][]'s callback. + ### socket.bufferSize `net.Socket` has the property that `socket.write()` always works. This is to diff --git a/lib/_tls_legacy.js b/lib/_tls_legacy.js index 456679ff0d2aa4..3043c031121f37 100644 --- a/lib/_tls_legacy.js +++ b/lib/_tls_legacy.js @@ -477,7 +477,7 @@ CryptoStream.prototype._done = function() { // readyState is deprecated. Don't use it. Object.defineProperty(CryptoStream.prototype, 'readyState', { get: function() { - if (this._connecting) { + if (this.connecting) { return 'opening'; } else if (this.readable && this.writable) { return 'open'; diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index f4c8d54e3dfb29..029d9ac26fe6bd 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -272,7 +272,7 @@ function TLSSocket(socket, options) { this._init(socket, wrap); - // Make sure to setup all required properties like: `_connecting` before + // Make sure to setup all required properties like: `connecting` before // starting the flow of the data this.readable = true; this.writable = true; @@ -466,9 +466,9 @@ TLSSocket.prototype._init = function(socket, wrap) { this._parent = socket; // To prevent assertion in afterConnect() and properly kick off readStart - this._connecting = socket._connecting || !socket._handle; + this.connecting = socket.connecting || !socket._handle; socket.once('connect', function() { - self._connecting = false; + self.connecting = false; self.emit('connect'); }); } @@ -480,7 +480,7 @@ TLSSocket.prototype._init = function(socket, wrap) { }); } else { assert(!socket); - this._connecting = true; + this.connecting = true; } }; @@ -581,7 +581,7 @@ TLSSocket.prototype._finishInit = function() { }; TLSSocket.prototype._start = function() { - if (this._connecting) { + if (this.connecting) { this.once('connect', function() { this._start(); }); diff --git a/lib/net.js b/lib/net.js index c2b3f5500a3ccb..509112f58f2077 100644 --- a/lib/net.js +++ b/lib/net.js @@ -119,7 +119,7 @@ const BYTES_READ = Symbol('bytesRead'); function Socket(options) { if (!(this instanceof Socket)) return new Socket(options); - this._connecting = false; + this.connecting = false; this._hadError = false; this._handle = null; this._parent = null; @@ -202,7 +202,7 @@ Socket.prototype._unrefTimer = function unrefTimer() { // so that only the writable side will be cleaned up. function onSocketFinish() { // If still connecting - defer handling 'finish' until 'connect' will happen - if (this._connecting) { + if (this.connecting) { debug('osF: not yet connected'); return this.once('connect', onSocketFinish); } @@ -367,9 +367,16 @@ Socket.prototype.address = function() { }; +Object.defineProperty(Socket.prototype, '_connecting', { + get: function() { + return this.connecting; + } +}); + + Object.defineProperty(Socket.prototype, 'readyState', { get: function() { - if (this._connecting) { + if (this.connecting) { return 'opening'; } else if (this.readable && this.writable) { return 'open'; @@ -397,7 +404,7 @@ Object.defineProperty(Socket.prototype, 'bufferSize', { Socket.prototype._read = function(n) { debug('_read'); - if (this._connecting || !this._handle) { + if (this.connecting || !this._handle) { debug('_read wait for connection'); this.once('connect', () => this._read(n)); } else if (!this._handle.reading) { @@ -430,7 +437,7 @@ function maybeDestroy(socket) { if (!socket.readable && !socket.writable && !socket.destroyed && - !socket._connecting && + !socket.connecting && !socket._writableState.length) { socket.destroy(); } @@ -465,7 +472,7 @@ Socket.prototype._destroy = function(exception, cb) { return; } - this._connecting = false; + this.connecting = false; this.readable = this.writable = false; @@ -648,7 +655,7 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) { // If we are still connecting, then buffer this for later. // The Writable logic will buffer up any more writes while // waiting for this one to be done. - if (this._connecting) { + if (this.connecting) { this._pendingData = data; this._pendingEncoding = encoding; this.once('connect', function() { @@ -803,7 +810,7 @@ function connect(self, address, port, addressType, localAddress, localPort) { // TODO return promise from Socket.prototype.connect which // wraps _connectReq. - assert.ok(self._connecting); + assert.ok(self.connecting); var err; @@ -913,7 +920,7 @@ Socket.prototype.connect = function(options, cb) { this._unrefTimer(); - this._connecting = true; + this.connecting = true; this.writable = true; if (pipe) { @@ -952,7 +959,7 @@ function lookupAndConnect(self, options) { var addressType = exports.isIP(host); if (addressType) { process.nextTick(function() { - if (self._connecting) + if (self.connecting) connect(self, host, port, addressType, localAddress, localPort); }); return; @@ -980,7 +987,7 @@ function lookupAndConnect(self, options) { // It's possible we were destroyed while looking this up. // XXX it would be great if we could cancel the promise returned by // the look up. - if (!self._connecting) return; + if (!self.connecting) return; if (err) { // net.createConnection() creates a net.Socket object and @@ -1048,8 +1055,8 @@ function afterConnect(status, handle, req, readable, writable) { debug('afterConnect'); - assert.ok(self._connecting); - self._connecting = false; + assert.ok(self.connecting); + self.connecting = false; self._sockname = null; if (status == 0) { @@ -1065,7 +1072,7 @@ function afterConnect(status, handle, req, readable, writable) { self.read(0); } else { - self._connecting = false; + self.connecting = false; var details; if (req.localAddress && req.localPort) { details = req.localAddress + ':' + req.localPort; diff --git a/test/parallel/test-net-connect-buffer.js b/test/parallel/test-net-connect-buffer.js index bd8efe1643441f..2bcd144fcdc82f 100644 --- a/test/parallel/test-net-connect-buffer.js +++ b/test/parallel/test-net-connect-buffer.js @@ -40,7 +40,7 @@ tcp.listen(common.PORT, function() { connectHappened = true; }); - console.log('_connecting = ' + socket._connecting); + console.log('_connecting = ' + socket.connecting); assert.equal('opening', socket.readyState); diff --git a/test/parallel/test-net-socket-connecting.js b/test/parallel/test-net-socket-connecting.js new file mode 100644 index 00000000000000..00b5200aacb329 --- /dev/null +++ b/test/parallel/test-net-socket-connecting.js @@ -0,0 +1,21 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const server = net.createServer((conn) => { + conn.end(); + server.close(); +}).listen(common.PORT, () => { + const client = net.connect(common.PORT, () => { + assert.equal(client.connecting, false); + + // Legacy getter + assert.equal(client._connecting, false); + client.end(); + }) + assert.equal(client.connecting, true); + + // Legacy getter + assert.equal(client._connecting, true); +}); From f906cb3cd63156e3a74ce6a01073fdbcefbfa05e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 26 Apr 2016 22:06:00 -0400 Subject: [PATCH 2/3] ... --- doc/api/net.md | 12 ++++++------ test/parallel/test-net-connect-buffer.js | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/api/net.md b/doc/api/net.md index 0cc23c05bceeb0..054069f0de5b01 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -333,12 +333,6 @@ socket as reported by the operating system. Returns an object with three properties, e.g. `{ port: 12346, family: 'IPv4', address: '127.0.0.1' }` -### socket.connecting - -If `true` - [`socket.connect(options\[, connectListener\])`][] was called and -haven't yet finished. Will be set to `false` before emitting `connect` event -and/or calling [`socket.connect(options\[, connectListener\])`][]'s callback. - ### socket.bufferSize `net.Socket` has the property that `socket.write()` always works. This is to @@ -406,6 +400,12 @@ The `connectListener` parameter will be added as a listener for the As [`socket.connect(options\[, connectListener\])`][`socket.connect(options, connectListener)`], with options either as either `{port: port, host: host}` or `{path: path}`. +### socket.connecting + +If `true` - [`socket.connect(options\[, connectListener\])`][] was called and +haven't yet finished. Will be set to `false` before emitting `connect` event +and/or calling [`socket.connect(options\[, connectListener\])`][]'s callback. + ### socket.destroy() Ensures that no more I/O activity happens on this socket. Only necessary in diff --git a/test/parallel/test-net-connect-buffer.js b/test/parallel/test-net-connect-buffer.js index 2bcd144fcdc82f..2c8acf331d2a1d 100644 --- a/test/parallel/test-net-connect-buffer.js +++ b/test/parallel/test-net-connect-buffer.js @@ -40,7 +40,7 @@ tcp.listen(common.PORT, function() { connectHappened = true; }); - console.log('_connecting = ' + socket.connecting); + console.log('connecting = ' + socket.connecting); assert.equal('opening', socket.readyState); From 1ed27a33d2fe48cfe628796bf70ce7d0fdb5e36e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 26 Apr 2016 22:53:19 -0400 Subject: [PATCH 3/3] fix --- test/parallel/test-net-socket-connecting.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-net-socket-connecting.js b/test/parallel/test-net-socket-connecting.js index 00b5200aacb329..fdd84874af3d6a 100644 --- a/test/parallel/test-net-socket-connecting.js +++ b/test/parallel/test-net-socket-connecting.js @@ -8,14 +8,14 @@ const server = net.createServer((conn) => { server.close(); }).listen(common.PORT, () => { const client = net.connect(common.PORT, () => { - assert.equal(client.connecting, false); + assert.strictEqual(client.connecting, false); // Legacy getter - assert.equal(client._connecting, false); + assert.strictEqual(client._connecting, false); client.end(); - }) - assert.equal(client.connecting, true); + }); + assert.strictEqual(client.connecting, true); // Legacy getter - assert.equal(client._connecting, true); + assert.strictEqual(client._connecting, true); });