Skip to content

net: introduce Socket#connecting property#6404

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.

Sign up for GitHub

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

Closed
indutny wants to merge 3 commits into from
Closed

net: introduce Socket#connecting property #6404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9db9528
net: introduce `Socket#connecting` property
indutny Apr 26, 2016
f906cb3
...
indutny Apr 27, 2016
1ed27a3
fix
indutny Apr 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/api/net.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -400,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
Expand Down
2 changes: 1 addition & 1 deletion lib/_tls_legacy.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';
}elseif(this.readable&&this.writable){
return'open';
Expand Down
10 changes: 5 additions & 5 deletions lib/_tls_wrap.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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');
});
}
Expand All@@ -480,7 +480,7 @@ TLSSocket.prototype._init = function(socket, wrap){
});
} else{
assert(!socket);
this._connecting = true;
this.connecting = true;
}
};

Expand DownExpand Up@@ -581,7 +581,7 @@ TLSSocket.prototype._finishInit = function(){
};

TLSSocket.prototype._start = function(){
if (this._connecting){
if (this.connecting){
this.once('connect', function(){
this._start();
});
Expand Down
35 changes: 21 additions & 14 deletions lib/net.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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);
}
Expand DownExpand Up@@ -367,9 +367,16 @@ Socket.prototype.address = function(){
};


Object.defineProperty(Socket.prototype, '_connecting',{
get: function(){
Copy link
Contributor

@evanlucasevanlucasApr 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a deprecation message here?

Copy link
Contributor

@mscdexmscdexApr 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChALkeR Can you check how many people are using ._connecting in the wild?

Copy link
MemberAuthor

@indutnyindutnyApr 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evanlucas it should be, will do it in a follow-up PR, because it won't be backported.

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'
Expand DownExpand Up@@ -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){
Expand DownExpand Up@@ -430,7 +437,7 @@ function maybeDestroy(socket){
if (!socket.readable &&
!socket.writable &&
!socket.destroyed &&
!socket._connecting &&
!socket.connecting &&
!socket._writableState.length){
socket.destroy();
}
Expand DownExpand Up@@ -465,7 +472,7 @@ Socket.prototype._destroy = function(exception, cb){
return;
}

this._connecting = false;
this.connecting = false;

this.readable = this.writable = false;

Expand DownExpand Up@@ -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(){
Expand DownExpand Up@@ -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;

Expand DownExpand Up@@ -913,7 +920,7 @@ Socket.prototype.connect = function(options, cb){

this._unrefTimer();

this._connecting = true;
this.connecting = true;
this.writable = true;

if (pipe){
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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){
Expand All@@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-connect-buffer.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);

Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-net-socket-connecting.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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);
});