From 2470d2ee92b162ebcc4eda68958769715c3d17fb Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 25 Oct 2010 22:04:39 -0700 Subject: [PATCH] allowHalfOpen disabled by default Users too often would forget to add socket.on('end', function () { socket.end(); }); Which is a mistake. Therefore we default to this behavior and only optionally let people handle the 'end' case themselves. --- doc/api.markdown | 13 +++++-- doc/index.html | 4 -- lib/http.js | 6 +-- lib/net.js | 51 ++++++++++++++++++++------ test/pummel/test-net-pingpong-delay.js | 2 +- test/pummel/test-net-pingpong.js | 2 +- test/simple/test-net-pingpong.js | 4 +- 7 files changed, 56 insertions(+), 26 deletions(-) diff --git a/doc/api.markdown b/doc/api.markdown index 5f7f141933..08fbf0af6a 100644 --- a/doc/api.markdown +++ b/doc/api.markdown @@ -2311,9 +2311,16 @@ Emitted when data is received. The argument `data` will be a `Buffer` or `function () { }` -Emitted when the other end of the stream sends a FIN packet. After this is -emitted the `readyState` will be `'writeOnly'`. One should probably just -call `stream.end()` when this event is emitted. +Emitted when the other end of the stream sends a FIN packet. + +By default (`allowHalfOpen == false`) the stream will destroy its file +descriptor once it has written out its pending write queue. However, by +setting `allowHalfOpen == true` the stream will not automatically `end()` +its side allowing the user to write arbitrary amounts of data, with the +caviot that the user is required to `end()` thier side now. In the +`allowHalfOpen == true` case after `'end'` is emitted the `readyState` will +be `'writeOnly'`. + ### Event: 'timeout' diff --git a/doc/index.html b/doc/index.html index 7a6c2d78a8..4630f90cde 100644 --- a/doc/index.html +++ b/doc/index.html @@ -67,14 +67,10 @@ Server running at http://127.0.0.1:8124/ 
 var net = require('net');
 net.createServer(function (socket) {
-  socket.setEncoding("utf8");
   socket.write("Echo server\r\n");
   socket.on("data", function (data) {
     socket.write(data);
   });
-  socket.on("end", function () {
-    socket.end();
-  });
 }).listen(8124, "127.0.0.1");
diff --git a/lib/http.js b/lib/http.js index 50bc6f16d0..d561058b79 100755 --- a/lib/http.js +++ b/lib/http.js @@ -743,9 +743,9 @@ function httpSocketSetup (socket) { function Server (requestListener) { if (!(this instanceof Server)) return new Server(requestListener); - net.Server.call(this); + net.Server.call(this, { allowHalfOpen: true }); - if(requestListener){ + if (requestListener) { this.addListener("request", requestListener); } @@ -877,7 +877,7 @@ function connectionListener (socket) { function Client ( ) { if (!(this instanceof Client)) return new Client(); - net.Stream.call(this); + net.Stream.call(this, { allowHalfOpen: true }); var self = this; httpSocketSetup(self); diff --git a/lib/net.js b/lib/net.js index b3f0643ef2..28f8660b85 100644 --- a/lib/net.js +++ b/lib/net.js @@ -475,6 +475,7 @@ function initStream (self) { if (!self.writable) self.destroy(); // Note: 'close' not emitted until nextTick. + if (!self.allowHalfOpen) self.end(); if (self._events && self._events['end']) self.emit('end'); if (self.onend) self.onend(); } else if (bytesRead > 0) { @@ -519,16 +520,29 @@ function initStream (self) { self.writable = false; } -function Stream (fd, type) { - if (!(this instanceof Stream)) return new Stream(fd, type); +// Deprecated API: Stream(fd, type) +// New API: Stream({ fd: 10, type: "unix", allowHalfOpen: true }) +function Stream (options) { + if (!(this instanceof Stream)) return new Stream(arguments[0], arguments[1]); stream.Stream.call(this); this.fd = null; this.type = null; this.secure = false; + this.allowHalfOpen = false; + + if (typeof options == "object") { + this.fd = options.fd !== undefined ? parseInt(options.fd, 10) : null; + this.type = options.type || null; + this.secure = options.secure || false; + this.allowHalfOpen = options.allowHalfOpen || false; + } else if (typeof options == "number") { + this.fd = arguments[0]; + this.type = arguments[1]; + } - if (parseInt(fd, 10) >= 0) { - this.open(fd, type); + if (parseInt(this.fd, 10) >= 0) { + this.open(this.fd, this.type); } else { setImplmentationMethods(this); } @@ -1030,8 +1044,8 @@ Stream.prototype._shutdown = function () { Stream.prototype.end = function (data, encoding) { if (this.writable) { - if (data) this.write(data, encoding); if (this._writeQueueLast() !== END_OF_FILE) { + if (data) this.write(data, encoding); this._writeQueue.push(END_OF_FILE); if (!this._connecting) { this.flush(); @@ -1041,13 +1055,22 @@ Stream.prototype.end = function (data, encoding) { }; -function Server (listener) { - if (!(this instanceof Server)) return new Server(listener); +function Server (/* [ options, ] listener */) { + if (!(this instanceof Server)) return new Server(arguments[0], arguments[1]); events.EventEmitter.call(this); var self = this; - if (listener) { - self.addListener('connection', listener); + var options = {}; + if (typeof arguments[0] == "object") { + options = arguments[0]; + } + + // listener: find the last argument that is a function + for (var l = arguments.length - 1; l >= 0; l--) { + if (typeof arguments[l] == "function") { + self.addListener('connection', arguments[l]); + } + if (arguments[l] !== undefined) break; } self.connections = 0; @@ -1055,6 +1078,8 @@ function Server (listener) { self.paused = false; self.pauseTimeout = 1000; + self.allowHalfOpen = options.allowHalfOpen || false; + function pause () { // We've hit the maximum file limit. What to do? // Let's try again in 1 second. @@ -1094,7 +1119,9 @@ function Server (listener) { self.connections++; - var s = new Stream(peerInfo.fd, self.type); + var s = new Stream({ fd: peerInfo.fd, + type: self.type, + allowHalfOpen: self.allowHalfOpen }); s.remoteAddress = peerInfo.address; s.remotePort = peerInfo.port; s.type = self.type; @@ -1118,8 +1145,8 @@ util.inherits(Server, events.EventEmitter); exports.Server = Server; -exports.createServer = function (listener) { - return new Server(listener); +exports.createServer = function () { + return new Server(arguments[0], arguments[1]); }; diff --git a/test/pummel/test-net-pingpong-delay.js b/test/pummel/test-net-pingpong-delay.js index 39e746b1d6..f1e3dace43 100644 --- a/test/pummel/test-net-pingpong-delay.js +++ b/test/pummel/test-net-pingpong-delay.js @@ -11,7 +11,7 @@ function pingPongTest (port, host, on_complete) { var count = 0; var client_ended = false; - var server = net.createServer(function (socket) { + var server = net.createServer({ allowHalfOpen: true }, function (socket) { socket.setEncoding("utf8"); socket.addListener("data", function (data) { diff --git a/test/pummel/test-net-pingpong.js b/test/pummel/test-net-pingpong.js index 48704ef048..e58b4734a4 100644 --- a/test/pummel/test-net-pingpong.js +++ b/test/pummel/test-net-pingpong.js @@ -9,7 +9,7 @@ function pingPongTest (port, host, on_complete) { var count = 0; var sent_final_ping = false; - var server = net.createServer(function (socket) { + var server = net.createServer({ allowHalfOpen: true }, function (socket) { assert.equal(true, socket.remoteAddress !== null); assert.equal(true, socket.remoteAddress !== undefined); if (host === "127.0.0.1" || host === "localhost" || !host) { diff --git a/test/simple/test-net-pingpong.js b/test/simple/test-net-pingpong.js index 5db5e127e3..6596d0030c 100644 --- a/test/simple/test-net-pingpong.js +++ b/test/simple/test-net-pingpong.js @@ -10,7 +10,7 @@ function pingPongTest (port, host) { var count = 0; var sent_final_ping = false; - var server = net.createServer(function (socket) { + var server = net.createServer({ allowHalfOpen: true }, function (socket) { console.log("connection: " + socket.remoteAddress); assert.equal(server, socket.server); assert.equal(1, server.connections); @@ -30,7 +30,7 @@ function pingPongTest (port, host) { }); socket.addListener("end", function () { - assert.equal(true, socket.writable); + assert.equal(true, socket.writable); // because allowHalfOpen assert.equal(false, socket.readable); socket.end(); });