Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequestHandler.finish() future not resolving #2448

Closed
sbrandtb opened this issue Jul 20, 2018 · 5 comments
Closed

RequestHandler.finish() future not resolving #2448

sbrandtb opened this issue Jul 20, 2018 · 5 comments

Comments

@sbrandtb
Copy link

Just tried Tornado 5.1 for the first time and am wondering if I'm doing something wrong when waiting for the future of the finish method:

import tornado.ioloop
import tornado.web

class MainHandler(tornado.web.RequestHandler):
    async def get(self):
        print('before')
        await self.finish("Hello, world")
        print('after')

def make_app():
    return tornado.web.Application([
        (r"/", MainHandler),
    ])

if __name__ == "__main__":
    app = make_app()
    app.listen(8888)
    tornado.ioloop.IOLoop.current().start()

It seems like the future is never resolved. after is not printed and interrupting the application outputs:

KeyboardInterrupt
ERROR:asyncio:Task was destroyed but it is pending!
task: <Task pending coro=<MainHandler.get() running at main.py:7> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7f0cb0b25e28>()]> cb=[IOLoop.add_future.<locals>.<lambda>() at /home/sebastian/.virtualenvs/tempenv-2fc3210213e1f/lib/python3.6/site-packages/tornado/ioloop.py:719]>

Python 3.6, Ubuntu

garenchan added a commit to garenchan/tornado that referenced this issue Jul 20, 2018
@garenchan
Copy link
Contributor

garenchan commented Jul 20, 2018

I give a PR to solve this problem in #2449.

@sbrandtb
Copy link
Author

@garenchan Thanks!

bdarnell added a commit that referenced this issue Jul 29, 2018
fix issue #2448: RequestHandler.finish() future not resolving
@bdarnell
Copy link
Member

Fixed by #2449

bdarnell pushed a commit to bdarnell/tornado that referenced this issue Sep 16, 2018
@cramosc
Copy link

cramosc commented Dec 10, 2018

@garenchan This bug hasn't been completely fixed in #2449
If you try with the following modification of the code above, the future RequestHandler.finish() is still not resolving

import tornado.ioloop
import tornado.web


class MainHandler(tornado.web.RequestHandler):
    async def get(self):
        self.write('first')
        await self.flush('first')
        print('before finish')
        await self.finish('second')
        print('finished')


def make_app():
    return tornado.web.Application([
        (r"/", MainHandler),
    ])


if __name__ == "__main__":
    app = make_app()
    app.listen(8888)
    tornado.ioloop.IOLoop.current().start()

The fix on HTTP1Connection.write_headers() should also be applied to HTTP1Connection.write() since the latter is the one called in the second call to RequestHandler.flush() (after the headers have been written).

@garenchan
Copy link
Contributor

@cramosc, Thank you very much! You are right and I left out the changes to the other branch.

garenchan added a commit to garenchan/tornado that referenced this issue Dec 10, 2018
The PR tornadoweb#2449 was incomplete, it omited consideration of another
branch of the 'flush' method. In short, the future returned by
'RequestHandler.finish()' may also be the return value of
'HTTP1Connection.write()'. So we should make the same change to
'HTTP1Connection.write()'.

Fix tornadoweb#2448.
garenchan added a commit to garenchan/tornado that referenced this issue Dec 10, 2018
The PR tornadoweb#2449 was incomplete, it omited consideration of another
branch of the 'flush' method. In short, the future returned by
'RequestHandler.finish()' may also be the return value of
'HTTP1Connection.write()'. So we should make the same change to
'HTTP1Connection.write()'.

Fix tornadoweb#2448.
garenchan added a commit to garenchan/tornado that referenced this issue Dec 10, 2018
The PR tornadoweb#2449 was incomplete, it omited consideration of another
branch of the 'flush' method. In short, the future returned by
'RequestHandler.finish()' may also be the return value of
'HTTP1Connection.write()'. So we should make the same change to
'HTTP1Connection.write()'.

Fix tornadoweb#2448.
bdarnell referenced this issue Dec 15, 2018
web: Use 'future_add_done_callback' to add callback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants