From 4733d0b1f08a644ecafc7a80e950fa3ac5814859 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 4 Feb 2011 15:14:58 -0800 Subject: [PATCH] http: handle aborts --- doc/api/http.markdown | 4 ++ lib/http.js | 66 +++++++++++++++++++++++++-- lib/net.js | 3 ++ test/simple/test-http-client-abort.js | 59 ++++++++++++++++++++++++ test/simple/test-http-server.js | 8 ++++ 5 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 test/simple/test-http-client-abort.js diff --git a/doc/api/http.markdown b/doc/api/http.markdown index b21314b038..3e75bf36c0 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -503,6 +503,10 @@ chunked, this will send the terminating `'0\r\n\r\n'`. If `data` is specified, it is equivalent to calling `request.write(data, encoding)` followed by `request.end()`. +### request.abort() + +Aborts a request. (New since v0.3.80.) + ## http.ClientResponse diff --git a/lib/http.js b/lib/http.js index 3b38aa1ee2..efeb9d2415 100644 --- a/lib/http.js +++ b/lib/http.js @@ -763,6 +763,25 @@ util.inherits(ClientRequest, OutgoingMessage); exports.ClientRequest = ClientRequest; +ClientRequest.prototype.abort = function() { + if (this._queue) { + // queued for dispatch + assert(!this.connection); + var i = this._queue.indexOf(this); + this._queue.splice(i, 1); + + } else if (this.connection) { + // in-progress + var c = this.connection; + this.detachSocket(c); + c.destroy(); + + } else { + // already complete. + } +}; + + function httpSocketSetup(socket) { // NOTE: be sure not to use ondrain elsewhere in this file! socket.ondrain = function() { @@ -781,6 +800,11 @@ function Server(requestListener) { this.addListener('request', requestListener); } + // Similar option to this. Too lazy to write my own docs. + // http://www.squid-cache.org/Doc/config/half_closed_clients/ + // http://wiki.squid-cache.org/SquidFaq/InnerWorkings#What_is_a_half-closed_filedescriptor.3F + this.httpAllowHalfOpen = false; + this.addListener('connection', connectionListener); } util.inherits(Server, net.Server); @@ -797,6 +821,15 @@ exports.createServer = function(requestListener) { function connectionListener(socket) { var self = this; var outgoing = []; + var incoming = []; + + function abortIncoming() { + while (incoming.length) { + var req = incoming.shift(); + req.emit('aborted'); + } + // abort socket._httpMessage ? + } debug('SERVER new http connection'); @@ -842,9 +875,18 @@ function connectionListener(socket) { }; socket.onend = function() { - parser.finish(); + var ret = parser.finish(); - if (outgoing.length) { + if (ret instanceof Error) { + debug('parse error'); + socket.destroy(ret); + return; + } + + if (!self.httpAllowHalfOpen) { + abortIncoming(); + socket.end(); + } else if (outgoing.length) { outgoing[outgoing.length - 1]._last = true; } else if (socket._httpMessage) { socket._httpMessage._last = true; @@ -854,14 +896,19 @@ function connectionListener(socket) { }; socket.addListener('close', function() { + debug('server socket close'); // unref the parser for easy gc parsers.free(parser); + + abortIncoming(); }); // The following callback is issued after the headers have been read on a // new message. In this callback we setup the response object and pass it // to the user. parser.onIncoming = function(req, shouldKeepAlive) { + incoming.push(req); + var res = new ServerResponse(req); debug('server response shouldKeepAlive: ' + shouldKeepAlive); res.shouldKeepAlive = shouldKeepAlive; @@ -877,6 +924,9 @@ function connectionListener(socket) { // When we're finished writing the response, check if this is the last // respose, if so destroy the socket. res.on('finish', function() { + assert(incoming[0] === req); + incoming.shift(); + res.detachSocket(socket); if (res._last) { @@ -909,23 +959,28 @@ function connectionListener(socket) { exports._connectionListener = connectionListener; + function Agent(host, port) { this.host = host; this.port = port; this.queue = []; this.sockets = []; - this.maxSockets = 5; + this.maxSockets = Agent.defaultMaxSockets; } util.inherits(Agent, EventEmitter); exports.Agent = Agent; +Agent.defaultMaxSockets = 5; + + Agent.prototype.appendMessage = function(options) { var self = this; var req = new ClientRequest(options); this.queue.push(req); + req._queue = this.queue; /* req.on('finish', function () { @@ -978,6 +1033,8 @@ Agent.prototype._establishNewConnection = function() { req = socket._httpMessage; } else if (self.queue.length) { req = self.queue.shift(); + assert(req._queue === self.queue); + req._queue = null; } else { // No requests on queue? Where is the request assert(0); @@ -1135,6 +1192,9 @@ Agent.prototype._cycle = function() { debug('Agent found socket, shift'); // We found an available connection! this.queue.shift(); // remove first from queue. + assert(first._queue === this.queue); + first._queue = null; + first.assignSocket(socket); self._cycle(); // try to dispatch another return; diff --git a/lib/net.js b/lib/net.js index 8c387df0e4..965d485207 100644 --- a/lib/net.js +++ b/lib/net.js @@ -754,6 +754,8 @@ Socket.prototype.destroy = function(exception) { // pool is shared between sockets, so don't need to free it here. var self = this; + debug('destroy ' + this.fd); + // TODO would like to set _writeQueue to null to avoid extra object alloc, // but lots of code assumes this._writeQueue is always an array. assert(this.bufferSize >= 0); @@ -787,6 +789,7 @@ Socket.prototype.destroy = function(exception) { // FIXME Bug when this.fd == 0 if (typeof this.fd == 'number') { + debug('close ' + this.fd); close(this.fd); this.fd = null; process.nextTick(function() { diff --git a/test/simple/test-http-client-abort.js b/test/simple/test-http-client-abort.js new file mode 100644 index 0000000000..3e0199b327 --- /dev/null +++ b/test/simple/test-http-client-abort.js @@ -0,0 +1,59 @@ +var common = require('../common'); +var assert = require('assert'); +var http = require('http'); + +var clientAborts = 0; + +var server = http.Server(function(req, res) { + console.log("Got connection"); + res.writeHead(200); + res.write("Working on it..."); + + // I would expect an error event from req or res that the client aborted + // before completing the HTTP request / response cycle, or maybe a new + // event like "aborted" or something. + req.on("aborted", function () { + clientAborts++; + console.log("Got abort " + clientAborts); + if (clientAborts === N) { + console.log("All aborts detected, you win."); + server.close(); + } + }); + + // since there is already clientError, maybe that would be appropriate, + // since "error" is magical + req.on("clientError", function () { + console.log("Got clientError"); + }); +}); + +var responses = 0; +var N = http.Agent.defaultMaxSockets - 1; +var requests = []; + +server.listen(common.PORT, function() { + console.log("Server listening."); + + for (var i = 0; i < N; i++) { + console.log("Making client " + i); + var options = { port: common.PORT, path: '/?id=' + i }; + var req = http.get(options, function(res) { + console.log("Client response code " + res.statusCode); + + if (++responses == N) { + console.log("All clients connected, destroying."); + requests.forEach(function (outReq) { + console.log("abort"); + outReq.abort(); + }); + } + }); + + requests.push(req); + } +}); + +process.on('exit', function() { + assert.equal(N, clientAborts); +}); diff --git a/test/simple/test-http-server.js b/test/simple/test-http-server.js index 969b6d35d6..3784439f0b 100644 --- a/test/simple/test-http-server.js +++ b/test/simple/test-http-server.js @@ -48,6 +48,8 @@ var server = http.createServer(function(req, res) { }); server.listen(common.PORT); +server.httpAllowHalfOpen = true; + server.addListener('listening', function() { var c = net.createConnection(common.PORT); @@ -69,6 +71,12 @@ server.addListener('listening', function() { if (requests_sent == 2) { c.write('GET / HTTP/1.1\r\nX-X: foo\r\n\r\n' + 'GET / HTTP/1.1\r\nX-X: bar\r\n\r\n'); + // Note: we are making the connection half-closed here + // before we've gotten the response from the server. This + // is a pretty bad thing to do and not really supported + // by many http servers. Node supports it optionally if + // you set server.httpAllowHalfOpen=true, which we've done + // above. c.end(); assert.equal(c.readyState, 'readOnly'); requests_sent += 2;