separate SelectorThread into its own object

[minrk]

so it can be used on an existing asyncio loop without replacing the loop itself.

e.g.

_selector_threads = {}

def _get_selector():
    loop = asyncio.get_event_loop()
    if loop in _selector_threads:
        return _selector_threads[loop]
    if isinstance(loop, asyncio.ProactorEventLoop):
        loop = _selector_threads[loop] = SelectorThread(loop)
        return loop
    # not proactor (TODO: better detection?)
    return loop

I'm investigating using the same solution for proactor compatibility in pyzmq, and the hiccup I am encountering is that pyzmq methods don't get invoked until the loop is already running, which means I need to launch the selector thread without replacing the loop itself. Using AddThread seems to work for this, but I thought it seemed cleaner to separate those two stages (run reader/writer in a thread vs define reader/writer methods on the main loop) explicitly. I'm also not at all sure things are being cleaned up properly when I set it up that way.

I was also thinking of making just this functionality a standalone package (e.g. asyncio-selector-thread), what do you think about that? Since this was added for tornado's internal use, feel free to close as out-of-scope for this repo and I'll explore the standalone package myself, which would mostly be exactly the code in this PR.

[bdarnell]

I have no objections to this change (and I like that it shrinks the getattribute exception list). However, if you're using something like this _get_selector function, I'm not sure you need this change. That's more or less what Tornado already does, and it works whether the event loop has already started or not. (although you do need to shut down the selector thread somewhere)

I also have no objections to you copying this code into an independent project if that's what you decide to do.

[minrk]

Yeah, I saw your comment on the Python Issue and realized I misunderstood what was going on here because of the inheritance from AbstractEventLoop.

I had thought the result of this code was that asyncio.get_event_loop().add_reader would work because AddThread would be the event loop, but see now that it's only accessible as IOLoop.selector_loop, and that the pattern I had should work just fine with AddThread as it already is.

Thanks for the notes!

I'll hold off on the independent package for now, but keep it in mind if this proactor stuff keeps causing problems outside tornado, especially if folks think get_event_loop().add_reader should be an easier thing with proactor. Fingers crossed for not actually needing that by the time I get around to it 🤞 .

[minrk]

I added one more commit here because I noticed that AddThreadEventLoop.close() wasn't idempotent, which the docs say it should be.

[bdarnell]

What are your thoughts on this PR now, after time has passed and you have zeromq/pyzmq#1524? Are you interested in getting it updated and merged or should we just close it?

[minrk]

This isn't blocking me on anything, so I'm happy with whatever you'd prefer. 63ae59d at least fixes an actual (minor) bug in the expected idempotency of event_loop.close().

[bdarnell]

Alright, I still like this change so I've updated it to be mergeable. Thanks!