Fix a bug with IOStream.read_until_close with a streaming_callback.

If the underlying socket was already closed the buffered data
would be passed to the final callback rather than the streaming callback.

diff --git a/tornado/iostream.py b/tornado/iostream.py
index 34fa9ed374dc9883a1187eec5d87fe0baab944d7..7f6ba7cac583f36d9633b04afd178155cac9c8ea 100644 (file)
--- a/tornado/iostream.py
+++ b/tornado/iostream.py
@@ -162,8 +162,14 @@ class BaseIOStream(object):
         a ``streaming_callback`` is not used.
         """
         self._set_read_callback(callback)
+        self._streaming_callback = stack_context.wrap(streaming_callback)
         if self.closed():
-            self._run_callback(callback, self._consume(self._read_buffer_size))
+            if self._streaming_callback is not None:
+                self._run_callback(self._streaming_callback,
+                                   self._consume(self._read_buffer_size))
+            self._run_callback(self._read_callback,
+                               self._consume(self._read_buffer_size))
+            self._streaming_callback = None
             self._read_callback = None
             return
         self._read_until_close = True

diff --git a/tornado/test/iostream_test.py b/tornado/test/iostream_test.py
index 875dddfca91f8b7858c61cd04a9df3bf928ce4a9..1177adbfcd23e8e1b42d27c7a5c9a88c67cc707b 100644 (file)
--- a/tornado/test/iostream_test.py
+++ b/tornado/test/iostream_test.py
@@ -301,6 +301,42 @@ class TestIOStreamMixin(object):
             server.close()
             client.close()
 
+    def test_read_until_close_after_close(self):
+        # Similar to test_delayed_close_callback, but read_until_close takes
+        # a separate code path so test it separately.
+        server, client = self.make_iostream_pair()
+        client.set_close_callback(self.stop)
+        try:
+            server.write(b("1234"))
+            server.close()
+            self.wait()
+            client.read_until_close(self.stop)
+            data = self.wait()
+            self.assertEqual(data, b("1234"))
+        finally:
+            server.close()
+            client.close()
+
+    def test_streaming_read_until_close_after_close(self):
+        # Same as the preceding test but with a streaming_callback.
+        # All data should go through the streaming callback,
+        # and the final read callback just gets an empty string.
+        server, client = self.make_iostream_pair()
+        client.set_close_callback(self.stop)
+        try:
+            server.write(b("1234"))
+            server.close()
+            self.wait()
+            streaming_data = []
+            client.read_until_close(self.stop,
+                                    streaming_callback=streaming_data.append)
+            data = self.wait()
+            self.assertEqual(b(''), data)
+            self.assertEqual(b('').join(streaming_data), b("1234"))
+        finally:
+            server.close()
+            client.close()
+
     def test_large_read_until(self):
         # Performance test: read_until used to have a quadratic component
         # so a read_until of 4MB would take 8 seconds; now it takes 0.25

diff --git a/website/sphinx/releases/next.rst b/website/sphinx/releases/next.rst
index 391c90aeb5569170a95eb4caa979b7a19a47f66c..b5043b395ca15f11cfbb33112a5b517daebb95ee 100644 (file)
--- a/website/sphinx/releases/next.rst
+++ b/website/sphinx/releases/next.rst
@@ -156,3 +156,6 @@ In progress
   response code the same as a 303.  This is contrary to the HTTP spec
   but consistent with all browsers and other major HTTP clients
   (including `CurlAsyncHTTPClient`).
+* Fixed a bug with `IOStream.read_until_close` with a ``streaming_callback``,
+  which would cause some data to be passed to the final callback instead
+  of the streaming callback.