From a9c1909002ff3c1f6c03fff09fd525382835832f Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 26 Apr 2016 16:27:08 -0400 Subject: [PATCH] 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. PR-URL: https://github.com/nodejs/node/pull/6404 Reviewed-By: Evan Lucas Reviewed-By: Sakthipriyan Vairamani --- 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 a5590bdad1491b..466cacaece33f1 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -392,6 +392,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/lib/_tls_legacy.js b/lib/_tls_legacy.js index bf79d7c8678331..2b31a824d069ba 100644 --- a/lib/_tls_legacy.js +++ b/lib/_tls_legacy.js @@ -480,7 +480,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 5c5370e09c19e0..114a645a1b3f56 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -284,7 +284,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; @@ -472,9 +472,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'); }); } @@ -486,7 +486,7 @@ TLSSocket.prototype._init = function(socket, wrap) { }); } else { assert(!socket); - this._connecting = true; + this.connecting = true; } }; @@ -583,7 +583,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 c1de0384ea42a9..a9e87934358c81 100644 --- a/lib/net.js +++ b/lib/net.js @@ -118,7 +118,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; @@ -201,7 +201,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); } @@ -366,9 +366,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'; @@ -396,7 +403,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) { @@ -429,7 +436,7 @@ function maybeDestroy(socket) { if (!socket.readable && !socket.writable && !socket.destroyed && - !socket._connecting && + !socket.connecting && !socket._writableState.length) { socket.destroy(); } @@ -466,7 +473,7 @@ Socket.prototype._destroy = function(exception, cb) { return; } - self._connecting = false; + this.connecting = false; this.readable = this.writable = false; @@ -647,7 +654,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() { @@ -802,7 +809,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; @@ -912,7 +919,7 @@ Socket.prototype.connect = function(options, cb) { this._unrefTimer(); - this._connecting = true; + this.connecting = true; this.writable = true; if (pipe) { @@ -949,7 +956,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; @@ -984,7 +991,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 @@ -1052,8 +1059,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) { @@ -1069,7 +1076,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 304401f56ef50d..1ad3f78bc4c53f 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..fdd84874af3d6a --- /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.strictEqual(client.connecting, false); + + // Legacy getter + assert.strictEqual(client._connecting, false); + client.end(); + }); + assert.strictEqual(client.connecting, true); + + // Legacy getter + assert.strictEqual(client._connecting, true); +});