Support for coroutine callback in autoreload.add_reload_hook.

[bhch]

Supporting coroutine callbacks in autoreload.add_reload_hook would be really helpful for closing database connections when the app auto reloads during development.

I'm currently using aiopg and the function to close the connections is a coroutine.

I tried using IOLoop.add_callback to run the coroutine from the reload hook callback, but it seems that the event loop is terminated before it can run the callback.

[mivade]

What if you tried something like this as your reload hook:

import asyncio
from threading import Event

def close_connection():
    closed = Event()
    task = asyncio.create_task(aiopg.sa.wait_closed())
    task.add_done_callback(lambda: closed.set())
    closed.wait()

[bdarnell]

@mivade I think it'll deadlock to use a threading.Event.wait like that on the event loop thread.

This sounds like a pretty reasonable request to me (although are you sure you need to close your database connections in this case? We try to close all file descriptors which should prevent leaks. It's not the most graceful way to shut down but that rarely matters)

[bhch]

@bdarnell Ah, you're right. I didn't know that all the fds are closed during autoreload. I even monitored the db processes to double check; connections are indeed closed.

It seems an ok solution. So, that solves my problem. Feel free to close the issue.

Thank you.

[bdarnell]

I took a look at this and found that it's more subtle than it first appears. Sometimes there are two different event loops in the process: the main one and a secondary event loop created by autoreload itself when run via python -m tornado.autoreload. This can be a problem because you can't necessarily close a connection used in one loop via another one. (sometimes it works, but I'm not sure how well we can rely on it).

I think the problems here are solvable, but since your problems are solved by the default close of all file descriptors I'm going to close this issue as "not planned". I'd be willing to reconsider if there were more interest in this issue.