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

Autoreload with asyncio event loop doesn't work #1543

Closed
rudyryk opened this issue Oct 6, 2015 · 7 comments · Fixed by #1984
Closed

Autoreload with asyncio event loop doesn't work #1543

rudyryk opened this issue Oct 6, 2015 · 7 comments · Fixed by #1984

Comments

@rudyryk
Copy link

rudyryk commented Oct 6, 2015

When running Tornado server in debug mode with autoreload and asyncio event loop the following exception raises: RuntimeError: Cannot close a running event loop.

Seems like autoreload hook tries to close event loop, but asyncio loop can't be closed without calling stop(). Code to reproduce and traceback are below.

Steps to reproduce:

  1. run the app server
  2. modify script with app server code to trigger auto reload
# save this to server.py and run as "python server.py"
import os
import asyncio
import tornado.ioloop
import tornado.httpserver
import tornado.web

from tornado.ioloop import IOLoop
IOLoop.configure('tornado.platform.asyncio.AsyncIOLoop')


class HomePage(tornado.web.RequestHandler):
    def get(self):
        self.write("Hello!")

if __name__ == '__main__':
    port = 8000
    host = '127.0.0.1'
    app = tornado.web.Application(
        [
            (r'/', HomePage),
        ],
        debug=True)

    print("Running server at http://%s:%s" % (host, port))

    server = tornado.httpserver.HTTPServer(app)
    server.bind(port, host)
    server.start()
    tornado.ioloop.IOLoop.current().start()

The full traceback:

Traceback (most recent call last):
  File "/Users/user/.virtualenvs/project/src/tornado/tornado/ioloop.py", line 1039, in _run
    return self.callback()
  File "/Users/user/.virtualenvs/project/src/tornado/tornado/autoreload.py", line 190, in _reload_on_update
    _check_file(modify_times, path)
  File "/Users/user/.virtualenvs/project/src/tornado/tornado/autoreload.py", line 205, in _check_file
    _reload()
  File "/Users/user/.virtualenvs/project/src/tornado/tornado/autoreload.py", line 212, in _reload
    fn()
  File "/Users/user/.virtualenvs/project/src/tornado/tornado/platform/asyncio.py", line 63, in close
    self.asyncio_loop.close()
  File "/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/asyncio/unix_events.py", line 55, in close
    super().close()
  File "/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/asyncio/selector_events.py", line 94, in close
    raise RuntimeError("Cannot close a running event loop")
RuntimeError: Cannot close a running event loop 
@rudyryk
Copy link
Author

rudyryk commented Oct 6, 2015

And as I can see stop() method of asyncio loop doesn't stop immidiatelly, but places callback on event loop instead via call_soon():
https://github.com/python/cpython/blob/master/Lib/asyncio/base_events.py#L345

So we can't just add stop() call line before close().

@bdarnell
Copy link
Member

I think the right thing to do here is to just swallow any exceptions from IOLoop.close(). It's not really necessary; it's just there to guard against file descriptors that did not have the cloexec flag set. Whether the close succeeds or not, it's better to proceed to the execv than to abort the autoreload process.

@rudyryk
Copy link
Author

rudyryk commented Oct 27, 2015

Hi Ben! Thank you for the idea. Well, may be I'm trying to simplify things, but shouldn't we just swallow all exceptions on auto-reload? I've created a pull request implementing such simple logic, it really worked perfectly. Is it ok? :)

@jonathansp
Copy link

jonathansp commented Feb 14, 2017

Hi @rudyryk, @bdarnell. It still happening in tornado 4.4.2. Any workaround?

@dpereira
Copy link

dpereira commented Mar 14, 2017

The snippet bellow seems to work, however. (python 3.5.2, tornado 4.4.2)

The only difference is that instead of initializing the event loop using this form: http://www.tornadoweb.org/en/stable/asyncio.html#tornado.platform.asyncio.AsyncIOLoop

It uses this form:
http://www.tornadoweb.org/en/stable/asyncio.html#tornado.platform.asyncio.AsyncIOMainLoop

Anyway, it's kinda hard to tell only from the docs in which circumstances each form is usefull, as the examples seem to be trying to achieve the exact same result.

# save this to server.py and run as "python server.py"
import os
import asyncio
import tornado.ioloop
import tornado.httpserver
import tornado.platform.asyncio
import tornado.web

tornado.platform.asyncio.AsyncIOMainLoop().install()


class HomePage(tornado.web.RequestHandler):
    def get(self):
        self.write("Hello!")

if __name__ == '__main__':
    port = 8000
    host = '127.0.0.1'
    app = tornado.web.Application(
        [
            (r'/', HomePage),
        ],
        debug=True)

    print("Running server at http://%s:%s" % (host, port))

    server = tornado.httpserver.HTTPServer(app)
    server.bind(port, host)
    server.start()
    asyncio.get_event_loop().run_forever()

bdarnell added a commit to bdarnell/tornado that referenced this issue Mar 25, 2017
This was a last-ditch effort to close file descriptors that were not
marked as CLOEXEC. However, it was never complete (it didn't touch
file descriptors that were not registered on the IOLoop), and it can't
work with asyncio (which does not allow closing the IOLoop without
stopping it and unwinding the stack first). Since Tornado (and
hopefully all major libraries using the IOLoop) is careful about
setting CLOEXEC when needed, just get rid of the close.

Fixes tornadoweb#1543
@bdarnell
Copy link
Member

#1984 fixes this slightly differently, by skipping the close call completely instead of swallowing exceptions.

@EpicCodeWizard
Copy link

Versions 2.5.2+ have this issue. Downgrade to version 2.5.1 (pip install livereload==2.5.1).

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

Successfully merging a pull request may close this issue.

5 participants