From: Ben Darnell Date: Tue, 19 Jul 2011 06:52:27 +0000 (-0700) Subject: Delay IOStream close callback until all pending callbacks have been processed. X-Git-Tag: v2.1.0~73 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e360c72a1617ef8fd2fe4ce7274a7611c49484cf;p=thirdparty%2Ftornado.git Delay IOStream close callback until all pending callbacks have been processed. This fixes an issue with SimpleAsyncHTTPClient when the server closes the connection and the close is processed before all of the data is read. Closes #306. --- diff --git a/tornado/iostream.py b/tornado/iostream.py index f2af5375f..f6308d3fd 100644 --- a/tornado/iostream.py +++ b/tornado/iostream.py @@ -199,8 +199,12 @@ class IOStream(object): self.io_loop.remove_handler(self.socket.fileno()) self.socket.close() self.socket = None - if self._close_callback: - self._run_callback(self._close_callback) + if self._close_callback and self._pending_callbacks == 0: + # if there are pending callbacks, don't run the close callback + # until they're done (see _maybe_add_error_handler) + cb = self._close_callback + self._close_callback = None + self._run_callback(cb) def reading(self): """Returns true if we are currently reading from the stream.""" @@ -441,7 +445,13 @@ class IOStream(object): def _maybe_add_error_listener(self): if self._state is None and self._pending_callbacks == 0: - self._add_io_state(0) + if self.socket is None: + cb = self._close_callback + if cb is not None: + self._close_callback = None + self._run_callback(cb) + else: + self._add_io_state(0) def _add_io_state(self, state): """Adds `state` (IOLoop.{READ,WRITE} flags) to our event handler. diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index 279544e29..ed6f04066 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -7,7 +7,9 @@ import binascii from tornado.escape import utf8 from tornado.httpclient import AsyncHTTPClient -from tornado.testing import AsyncHTTPTestCase, LogTrapTestCase +from tornado.iostream import IOStream +from tornado import netutil +from tornado.testing import AsyncHTTPTestCase, LogTrapTestCase, get_unused_port from tornado.util import b, bytes_type from tornado.web import Application, RequestHandler, url @@ -101,6 +103,33 @@ class HTTPClientCommonTestCase(AsyncHTTPTestCase, LogTrapTestCase): self.assertEqual(chunks, [b("asdf"), b("qwer")]) self.assertFalse(response.body) + def test_chunked_close(self): + # test case in which chunks spread read-callback processing + # over several ioloop iterations, but the connection is already closed. + port = get_unused_port() + (sock,) = netutil.bind_sockets(port, address="127.0.0.1") + def accept_callback(conn, address): + # fake an HTTP server using chunked encoding where the final chunks + # and connection close all happen at once + stream = IOStream(conn, io_loop=self.io_loop) + stream.write(b("""\ +HTTP/1.1 200 OK +Transfer-Encoding: chunked + +1 +1 +1 +2 +0 + +""").replace(b("\n"), b("\r\n")), callback=stream.close) + netutil.add_accept_handler(sock, accept_callback, self.io_loop) + self.http_client.fetch("http://127.0.0.1:%d/" % port, self.stop) + resp = self.wait() + resp.rethrow() + self.assertEqual(resp.body, b("12")) + + def test_basic_auth(self): self.assertEqual(self.fetch("/auth", auth_username="Aladdin", auth_password="open sesame").body,