Avoid setting IOStream.error to wrong exception, close #651

tornado/iostream.py

@@ -209,10 +209,14 @@ def set_close_callback(self, callback):
         """Call the given callback when the stream is closed."""
         self._close_callback = stack_context.wrap(callback)
 
-    def close(self):
-        """Close this stream."""
+    def close(self, exc_info=False):
+        """Close this stream.
+
+        If `exc_info` is true, set the `error` attribute to the current
+        exception from ``sys.exc_info()``.
+        """
         if not self.closed():
-            if any(sys.exc_info()):
+            if exc_info and any(sys.exc_info()):
                 self.error = sys.exc_info()[1]
             if self._read_until_close:
                 callback = self._read_callback
@@ -285,7 +289,7 @@ def _handle_events(self, fd, events):
         except Exception:
             gen_log.error("Uncaught exception, closing connection.",
                           exc_info=True)
-            self.close()
+            self.close(exc_info=True)
             raise
 
     def _run_callback(self, callback, *args):
@@ -300,7 +304,7 @@ def wrapper():
                 # (It would eventually get closed when the socket object is
                 # gc'd, but we don't want to rely on gc happening before we
                 # run out of file descriptors)
-                self.close()
+                self.close(exc_info=True)
                 # Re-raise the exception so that IOLoop.handle_callback_exception
                 # can see it and log the error
                 raise
@@ -348,7 +352,7 @@ def _handle_read(self):
                 self._pending_callbacks -= 1
         except Exception:
             gen_log.warning("error on read", exc_info=True)
-            self.close()
+            self.close(exc_info=True)
             return
         if self._read_from_buffer():
             return
@@ -397,9 +401,9 @@ def _read_to_buffer(self):
                 # Treat ECONNRESET as a connection close rather than
                 # an error to minimize log spam  (the exception will
                 # be available on self.error for apps that care).
-                self.close()
+                self.close(exc_info=True)
                 return
-            self.close()
+            self.close(exc_info=True)
             raise
         if chunk is None:
             return 0
@@ -503,7 +507,7 @@ def _handle_write(self):
                 else:
                     gen_log.warning("Write error on %d: %s",
                                     self.fileno(), e)
-                    self.close()
+                    self.close(exc_info=True)
                     return
         if not self._write_buffer and self._write_callback:
             callback = self._write_callback
@@ -664,7 +668,7 @@ def connect(self, address, callback=None):
             if e.args[0] not in (errno.EINPROGRESS, errno.EWOULDBLOCK):
                 gen_log.warning("Connect error on fd %d: %s",
                                 self.socket.fileno(), e)
-                self.close()
+                self.close(exc_info=True)
                 return
         self._connect_callback = stack_context.wrap(callback)
         self._add_io_state(self.io_loop.WRITE)
@@ -733,19 +737,19 @@ def _do_ssl_handshake(self):
                 return
             elif err.args[0] in (ssl.SSL_ERROR_EOF,
                                  ssl.SSL_ERROR_ZERO_RETURN):
-                return self.close()
+                return self.close(exc_info=True)
             elif err.args[0] == ssl.SSL_ERROR_SSL:
                 try:
                     peer = self.socket.getpeername()
                 except:
                     peer = '(not connected)'
                 gen_log.warning("SSL Error on %d %s: %s",
                                 self.socket.fileno(), peer, err)
-                return self.close()
+                return self.close(exc_info=True)
             raise
         except socket.error, err:
             if err.args[0] in (errno.ECONNABORTED, errno.ECONNRESET):
-                return self.close()
+                return self.close(exc_info=True)
         else:
             self._ssl_accepting = False
             if self._ssl_connect_callback is not None:
@@ -842,7 +846,7 @@ def read_from_fd(self):
             elif e.args[0] == errno.EBADF:
                 # If the writing half of a pipe is closed, select will
                 # report it as readable but reads will fail with EBADF.
-                self.close()
+                self.close(exc_info=True)
                 return None
             else:
                 raise

tornado/test/iostream_test.py

@@ -2,7 +2,8 @@
 from tornado import netutil
 from tornado.ioloop import IOLoop
 from tornado.iostream import IOStream, SSLIOStream, PipeIOStream
-from tornado.log import gen_log
+from tornado.log import gen_log, app_log
+from tornado.stack_context import NullContext
 from tornado.testing import AsyncHTTPTestCase, AsyncHTTPSTestCase, AsyncTestCase, bind_unused_port, ExpectLog
 from tornado.test.util import unittest, skipIfNonUnix
 from tornado.util import b
@@ -197,6 +198,24 @@ def test_gaierror(self):
             stream.connect(('an invalid domain', 54321))
             self.assertTrue(isinstance(stream.error, socket.gaierror), stream.error)
 
+    def test_read_callback_error(self):
+        # Test that IOStream sets its exc_info when a read callback throws
+        server, client = self.make_iostream_pair()
+        try:
+            server.set_close_callback(self.stop)
+            with ExpectLog(
+                app_log, "(Uncaught exception|Exception in callback)"
+            ):
+                # Clear ExceptionStackContext so IOStream catches error
+                with NullContext():
+                    server.read_bytes(1, callback=lambda data: 1 / 0)
+                client.write(b("1"))
+                self.wait()
+            self.assertTrue(isinstance(server.error, ZeroDivisionError))
+        finally:
+            server.close()
+            client.close()
+
     def test_streaming_callback(self):
         server, client = self.make_iostream_pair()
         try: