From 85d99830096a48b7d50cc3b0e5c3fba4172c2d02 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 18 Feb 2015 12:02:01 -0600 Subject: [PATCH] net: persist net.Socket options before connect Remembers net.Socket options called before connect and retroactively applies them after the handle has been created. This change makes the following function calls more user-friendly: - setKeepAlive() - setNoDelay() - ref() - unref() Related: https://github.com/joyent/node/issues/7077 and https://github.com/joyent/node/issues/8572 Fixes: https://github.com/joyent/node/issues/7077 Fixes: https://github.com/joyent/node/issues/8572 PR-URL: https://github.com/nodejs/io.js/pull/1518 Reviewed-By: Ben Noordhuis Reviewed-By: Roman Reiss --- lib/net.js | 31 ++++++++++++--- .../parallel/test-net-persistent-keepalive.js | 33 ++++++++++++++++ test/parallel/test-net-persistent-nodelay.js | 33 ++++++++++++++++ .../parallel/test-net-persistent-ref-unref.js | 39 +++++++++++++++++++ 4 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-net-persistent-keepalive.js create mode 100644 test/parallel/test-net-persistent-nodelay.js create mode 100644 test/parallel/test-net-persistent-ref-unref.js diff --git a/lib/net.js b/lib/net.js index f8ebb2c2d7b4f7..76c6f1d8891a90 100644 --- a/lib/net.js +++ b/lib/net.js @@ -321,14 +321,25 @@ Socket.prototype._onTimeout = function() { Socket.prototype.setNoDelay = function(enable) { + if (!this._handle) { + this.once('connect', + enable ? this.setNoDelay : this.setNoDelay.bind(this, enable)); + return; + } + // backwards compatibility: assume true when `enable` is omitted - if (this._handle && this._handle.setNoDelay) + if (this._handle.setNoDelay) this._handle.setNoDelay(enable === undefined ? true : !!enable); }; Socket.prototype.setKeepAlive = function(setting, msecs) { - if (this._handle && this._handle.setKeepAlive) + if (!this._handle) { + this.once('connect', this.setKeepAlive.bind(this, setting, msecs)); + return; + } + + if (this._handle.setKeepAlive) this._handle.setKeepAlive(setting, ~~(msecs / 1000)); }; @@ -971,14 +982,22 @@ function connectErrorNT(self, err, options) { Socket.prototype.ref = function() { - if (this._handle) - this._handle.ref(); + if (!this._handle) { + this.once('connect', this.ref); + return; + } + + this._handle.ref(); }; Socket.prototype.unref = function() { - if (this._handle) - this._handle.unref(); + if (!this._handle) { + this.once('connect', this.unref); + return; + } + + this._handle.unref(); }; diff --git a/test/parallel/test-net-persistent-keepalive.js b/test/parallel/test-net-persistent-keepalive.js new file mode 100644 index 00000000000000..adf7d7a4fa81d8 --- /dev/null +++ b/test/parallel/test-net-persistent-keepalive.js @@ -0,0 +1,33 @@ +'use strict'; +var common = require('../common'); +var assert = require('assert'); +var net = require('net'); + +var serverConnection; +var echoServer = net.createServer(function(connection) { + serverConnection = connection; + connection.setTimeout(0); + assert.equal(typeof connection.setKeepAlive, 'function'); + connection.on('end', function() { + connection.end(); + }); +}); +echoServer.listen(common.PORT); + +echoServer.on('listening', function() { + var clientConnection = new net.Socket(); + // send a keepalive packet after 1000 ms + // and make sure it persists + clientConnection.setKeepAlive(true, 400); + clientConnection.connect(common.PORT); + clientConnection.setTimeout(0); + + setTimeout(function() { + // make sure both connections are still open + assert.equal(serverConnection.readyState, 'open'); + assert.equal(clientConnection.readyState, 'open'); + serverConnection.end(); + clientConnection.end(); + echoServer.close(); + }, 600); +}); diff --git a/test/parallel/test-net-persistent-nodelay.js b/test/parallel/test-net-persistent-nodelay.js new file mode 100644 index 00000000000000..8ed5925f11a659 --- /dev/null +++ b/test/parallel/test-net-persistent-nodelay.js @@ -0,0 +1,33 @@ +'use strict'; +var common = require('../common'); +var assert = require('assert'); +var net = require('net'); +var TCPWrap = process.binding('tcp_wrap').TCP; + +var echoServer = net.createServer(function(connection) { + connection.end(); +}); +echoServer.listen(common.PORT); + +var callCount = 0; + +var Socket = net.Socket; +var setNoDelay = TCPWrap.prototype.setNoDelay; + +TCPWrap.prototype.setNoDelay = function(enable) { + setNoDelay.call(this, enable); + callCount++; +}; + +echoServer.on('listening', function() { + var sock1 = new Socket(); + // setNoDelay before the handle is created + // there is probably a better way to test this + + sock1.setNoDelay(); + sock1.connect(common.PORT); + sock1.on('end', function() { + assert.equal(callCount, 1); + echoServer.close(); + }); +}); diff --git a/test/parallel/test-net-persistent-ref-unref.js b/test/parallel/test-net-persistent-ref-unref.js new file mode 100644 index 00000000000000..b3ea0969f39961 --- /dev/null +++ b/test/parallel/test-net-persistent-ref-unref.js @@ -0,0 +1,39 @@ +'use strict'; +var common = require('../common'); +var assert = require('assert'); +var net = require('net'); +var TCPWrap = process.binding('tcp_wrap').TCP; + +var echoServer = net.createServer(function(conn) { + conn.end(); +}); + +var ref = TCPWrap.prototype.ref; +var unref = TCPWrap.prototype.unref; + +var refCount = 0; + +TCPWrap.prototype.ref = function() { + ref.call(this); + refCount++; + assert.equal(refCount, 0); +}; + +TCPWrap.prototype.unref = function() { + unref.call(this); + refCount--; + assert.equal(refCount, -1); +}; + +echoServer.listen(common.PORT); + +echoServer.on('listening', function() { + var sock = new net.Socket(); + sock.unref(); + sock.ref(); + sock.connect(common.PORT); + sock.on('end', function() { + assert.equal(refCount, 0); + echoServer.close(); + }); +});