Permalink
Browse files

http: handle aborts

  • Loading branch information...
1 parent c783aef commit 4733d0b1f08a644ecafc7a80e950fa3ac5814859 @ry ry committed Feb 4, 2011
Showing with 137 additions and 3 deletions.
  1. +4 −0 doc/api/http.markdown
  2. +63 −3 lib/http.js
  3. +3 −0 lib/net.js
  4. +59 −0 test/simple/test-http-client-abort.js
  5. +8 −0 test/simple/test-http-server.js
View
@@ -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.)
@mscdex

mscdex Feb 5, 2011

Should be v0.3.8?

+
## http.ClientResponse
View
@@ -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;
View
@@ -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() {
@@ -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
@mranney

mranney Feb 5, 2011

Probably should remove this clientError part since it was more "asking a question with code" than actually the right code to be in there.

+ 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);
+});
@@ -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;

0 comments on commit 4733d0b

Please sign in to comment.