Repository: tornadoweb/tornado
Pull request #2449 (2018-07-20 08:54:35)

fix issue #2448: RequestHandler.finish() future not resolving
This bug exists in Tornado 5.1 and 6.0.


1. In RequestHandler.finish(), RequestHandler.flush() method will be called， the `future` returned by flush() will be finish()'s returned value.
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/web.py#L1040
2. RequestHandler.flush() then call HTTP1Connection.write_headers() .
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/web.py#L981-L982
3. In HTTP1Connection.write_headers(), we can know the `future` returned by finish() is exactly the `http1connection`'s ` _write_future`. **And the `_pending_write` future is already finished, but the callback `_on_write_complete` will be handled in the next iteration of `ioloop`.**
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/http1connection.py#L397-L403
4. Then `http1connection`'s finish() will be called.
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/web.py#L1040-L1041
5. In `http1connection`'s finish(), because the `_pending_write` has been finished, its `_finish_request` will be called immediately.
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/http1connection.py#L462-L465
6. `http1connection`.`_finish_request` will clear its callbacks as follow. Unfortunately, the `_write_future ` will be cleared. 
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/http1connection.py#L496-L497
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/http1connection.py#L262-L269
7. So, while the next iteration of `ioloop`, http1connection._on_write_complete() will be called. But its `_write_future` is None, so the RequestHandler.finish() future will never be resolved.
https://github.com/tornadoweb/tornado/blob/d4eb8eb4eb5cc9a6677e9116ef84ded8efba8859/tornado/http1connection.py#L467-L479


Patch:
diff --git a/tornado/http1connection.py b/tornado/http1connection.py
index 0002f857..e2b4c787 100644
--- a/tornado/http1connection.py
+++ b/tornado/http1connection.py
@@ -399,7 +399,7 @@ class HTTP1Connection(httputil.HTTPConnection):
             if chunk:
                 data += self._format_chunk(chunk)
             self._pending_write = self.stream.write(data)
-            self._pending_write.add_done_callback(self._on_write_complete)
+            future_add_done_callback(self._pending_write, self._on_write_complete)
         return future
 
     def _format_chunk(self, chunk):


Test patch:
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index cb1ab353..0916d9ae 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -185,6 +185,7 @@ class FinalReturnTest(WebTestCase):
             @gen.coroutine
             def get(self):
                 test.final_return = self.finish()
+                yield test.final_return
 
         class RenderHandler(RequestHandler):
             def create_template_loader(self, path):
@@ -204,6 +205,7 @@ class FinalReturnTest(WebTestCase):
         response = self.fetch(self.get_url('/finish'))
         self.assertEqual(response.code, 200)
         self.assertIsInstance(self.final_return, Future)
+        self.assertTrue(self.final_return.done())
 
     def test_render_method_return_future(self):
         response = self.fetch(self.get_url('/render'))