Issue37373
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2019-06-22 18:33 by Ben.Darnell, last changed 2022-04-11 14:59 by admin.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
selector_thread.py | vstinner, 2019-06-24 10:50 |
Messages (24) | |||
---|---|---|---|
msg346289 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2019-06-22 18:33 | |
On Windows there are two event loop implementions with different interfaces: The proactor event loop is missing the file descriptor family of methods (add_reader()), while the selector event loop has other limitations including missing support for pipes and subprocesses and generally lower scalability. (The default has changed from selector to proactor in Python 3.8). If an application requires the selector event loop, it can set the global event loop policy when it starts up. But what if a library requires the selector event loop? It wouldn't be appropriate for a library to set the event loop policy. The best I can do is document that "This library requires an event loop which supports the add_reader() method; to use this library on Windows (directly or indirectly) you must install the WindowsSelectorEventLoopPolicy." This places a burden on application developers that target Windows to examine all their transitive dependencies to see if any require selectorevent loops (and if any have conflicting requirements for proactor event loops, which are even less likely to be documented since this is now the default). Concretely, this is a concern for Tornado (which requires add_reader()) and applications in the scientific python community (including Jupyter) which depend on it. I know it's probably too late to do anything about this for 3.8, but it would be great if there were some better process to negotiate the right event loop. Some ideas (none of which are very satisfying): - A declarative marker in setup.cfg or similar indicating the required feature set for an event loop - A dummy package that could be depended on to indicate the same thing (and maybe this package could even use .pth hacks to change the default simply by being installed, but that seems like a hack too far) - Some sort of runtime registry: at import time, Tornado could call `asyncio.add_required_features(asyncio.ADD_READER)` and this would influence the default policy - Flags to `get_event_loop()` indicating the required features (passed through to the policy). |
|||
msg346293 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2019-06-22 19:34 | |
Victor, Andrew, I'm not an expert in IOCP, but is it possible to implement add_reader/writer in ProactorEventLoop? If there's no native API for that, I guess we can spawn a thread with a 'select()' call to emulate this API? Lukasz, Another question: if we fix this, would you allow this to go in beta2/3? Strictly speaking it's going to be a new functionality. |
|||
msg346295 - (view) | Author: Big Stone (Big Stone) | Date: 2019-06-22 19:54 | |
Windows users would certainly prefer to have an emulated non-performant emulation of the old API, rather than a breakage of their beloved Jupyter/Tornado ecosystem. |
|||
msg346299 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2019-06-22 21:00 | |
> Is it possible to implement add_reader/writer in ProactorEventLoop? Sorry, no. There is no way IIUC. Regarding a request for selecting event loop by a library. 1. You can call asyncio.set_event_loop_policy() already. 2. From my understanding, there is no issue for Tornado itself. If Jupiter Notebook needs Tornado, Tornado needs selector event loop on Windows -- Jupiter can install the proper loop. |
|||
msg346302 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2019-06-22 22:05 | |
> From my understanding, there is no issue for Tornado itself. If Jupiter Notebook needs Tornado, Tornado needs selector event loop on Windows -- Jupiter can install the proper loop. Right. I'm just advocating for something that would make the transition smoother than each project independently stumbling across this issue and adding their own patch (and then fielding support issues from users who have ended up with a combination of versions that doesn't quite work). This of course depends on how many affected projects there are; I know Jupyter is the biggest but they're not the only one. There's no more direct way, but a thread that does select() should work. In fact, ProactorEventLoop could simply spin up a SelectorEventLoop in another thread when one of the FD methods is called, and proxy the callbacks back and forth. def add_reader(self, fd, callback, *args): if not self.selector_started: self.start_selector() self.selector.call_soon(self.selector.add_reader, fd, lambda: self.call_soon(callback, *args)) def start_selector(self): self.selector = SelectorEventLoop() def target(): asyncio.set_event_loop(self.selector) self.selector.run_forever() thread = threading.Thread(target=target) thread.start() # Clean shutdown is left as an exercise for the reader. Unifying the two interfaces like this would be preferable to adding more complexity for configuration IMHO. |
|||
msg346311 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2019-06-23 01:19 | |
Andrew, > Sorry, no. There is no way IIUC. Well, in this case we should do that via a thread+select approach as I and Ben suggested. I can write some PoC code; will you have some time to polish it and commit it? (I'm OOO and don't have a windows machine/VM at the moment). |
|||
msg346315 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2019-06-23 10:31 | |
I doubt if we should split a proactor loop into two objects in two threads. I see no strong objections but have a feeling that we'll get other problems in this way. asyncio is just not designed for this mode. The proposal looks like a very dirty hack and smells as it is. There is no very strong reason to apply it. Jupiter Notebook can fix the problem on its own side in two-three lines of clear code. |
|||
msg346347 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2019-06-23 23:14 | |
Yeah, it's definitely a hack. The argument for it, at best, is "practicality beats purity". The solution is two simple lines, but those lines need to be repeated in every project that depends on Tornado and cares about Windows, now or in the future. How many projects have to be affected to justify a hack like this? I'm not sure, but without this hack the add_reader methods, and by extension Tornado, will remain a compatibility trap. |
|||
msg346348 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2019-06-23 23:32 | |
+1 to what Ben said. Andrew, > The proposal looks like a very dirty hack and smells as it is. I wonder why are you feeling like that about this idea. I don't think this is a hack at all. There's no native API, so we're forced to use another approach. > There is no very strong reason to apply it. I think there's such a reason. asyncio has a well defined API that people use and create programs dependent on it. And then people learn that the API isn't really supported on Windows with one loop, or when it is supported, other APIs aren't. They are then forced to either not support Windows or to rewrute their programs. The situation further complicates if they are using third-party code. This is a very unfortunate and weird position we put our users into. So this indeed is a perfect example where "practicality beats purity" principle should be invoked. |
|||
msg346360 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2019-06-24 09:42 | |
I don't like to have not required parts like thread pool, time handlers, and the exception handler executed in the auxiliary thread. What's about an alternative proposal: embed into ProactorEventLoop not entire SelectorEventLoop but selectors.SelectSelector only? We will have a little more code but the code is very clear and self-contained. |
|||
msg346369 - (view) | Author: STINNER Victor (vstinner) * | Date: 2019-06-24 10:50 | |
Ben: > Concretely, this is a concern for Tornado (which requires add_reader()) and applications in the scientific python community (including Jupyter) which depend on it. If you need add_reader/add_writer in Python 3.8, you can switch the default event loop to selector at the beginning of your application: if sys.platform == 'win32': asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) Yury: > If there's no native API for that, I guess we can spawn a thread with a 'select()' call to emulate this API? There is no native API because IOCP design is to run an asynchronous read/write and then wait for its completion. Unix select() has the opposite design: check if a file descriptor is read for read/write. Reimplementing add_reader/add_writer using a single select() call in a thread sounds like a good idea, but we have to make sure that it can be stopped whenever using a dedicated "self-pipe" (to awake the blocked select(), so loop.close() can stop this thread. See attached proof-of-concept: selector_thread.py: run a selector in a separated thread which pass pack events to the loop using call_soon(). I would prefer to use a single selector to better scale with the number of FD. Note: On Windows, select() only supports sockets. Note: select.select() may be extended to support more than 512 sockets on Windows, see bpo-28708 :-) Yury: > Another question: if we fix this, would you allow this to go in beta2/3? Strictly speaking it's going to be a new functionality. It's a new feature, so it can wait for Python 3.9 :-) I don't see any regression here, as soon as you can opt-in for SelectorEventLoop. |
|||
msg353823 - (view) | Author: Nathaniel Smith (njs) * | Date: 2019-10-03 07:16 | |
It's also possible to implement 'select' on top of IOCP using undocumented private APIs. That's what libuv does (and how 'select' works internally). There's a bunch of information on this collected here: https://github.com/python-trio/trio/issues/52 |
|||
msg355891 - (view) | Author: Big Stone (Big Stone) | Date: 2019-11-03 08:42 | |
Is it be possible to backport this inside the standard ProactorEventLoop of Python-3.8.1 ? As things are currently broken, no kitten would be armed https://github.com/python-trio/trio/pull/1269 |
|||
msg355893 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2019-11-03 11:37 | |
IOCTL_AFD_POLL looks interesting. I wonder if it is 100% reliable; we can miss some edge case easily. |
|||
msg355906 - (view) | Author: Nathaniel Smith (njs) * | Date: 2019-11-03 17:53 | |
Yeah, that's the question. I've dug into the AFD_POLL stuff more now, and... There's a lot of edge cases to think about. It certainly *can* be done, because this is the primitive that 'select' and friends are built on, so obviously it can do anything they can do. But it also has some very sharp corners that 'select' knows how to work around, and we might not, since the folks writing 'select' could look at the internals and we can't. The good news is that libuv has been shipping a version of this code for many years, and trio started shipping a version yesterday, so we can treat those as field experiments to gather data for asyncio. (I think that rust's mio library is also moving this way, but I'm not as familiar with the details. And wepoll is a good reference too, but I don't know how widely-deployed it is.) The libuv implementation is very conservative, and falls back to calling 'select' in a thread if anything looks the slightest bit weird. The author of that code told me that he now thinks that's too conservative, esp. since some if the issues they were worrying about in the win xp era are no longer relevant. So Trio's version is more aggressive. I'm very curious to see how it goes. I do think the incompatibilities between the different aio event loops are really a problem and the ultimate goal should be to support the full feature set everywhere. The question is how to make that viable. Getting more experience with AFD_POLL will help make it possible for aio to implement its own version, if that's the direction we want to go. Maybe it would also be helpful to try to find the right folks inside Microsoft to get more information about this? My understanding is that their official position on AFD_POLL is "don't do that", but you can't put the genie back into the bottle... Alternatively: it seems like this is really highlighting the downsides of aio maintaining its own written-from-scratch event loops. Would it make sense to promote uvloop to the One True Event Loop? I get that there are significant complications to doing that, but there are also significant benefits: we'd get a more mature event loop core that we don't have to maintain alone, and it would fix tornado. |
|||
msg355991 - (view) | Author: STINNER Victor (vstinner) * | Date: 2019-11-05 01:23 | |
> Is it be possible to backport this inside the standard ProactorEventLoop of Python-3.8.1 ? As things are currently broken, no kitten would be armed https://github.com/python-trio/trio/pull/1269 No, we don't add features to minor releases. If you need add_reader(), your code works with Python 3.7 but fails with 3.8, you can use the following code to ensure that you use the SelectorEventLoop: if sys.platform == 'win32': asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) |
|||
msg362483 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2020-02-22 23:43 | |
I have an implementation of the selector-in-another-thread solution in https://github.com/tornadoweb/tornado/pull/2815. Is something like this worth considering for Python 3.9, or was Tornado the only project experiencing this pain and a tornado-specific solution is enough? |
|||
msg362587 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2020-02-24 14:15 | |
From my understanding, the issue is still desirable but we have a lack of resources. Regarding the patch for tornado, using selectors.SelectSelector for implementing add_reader()/remove_reader() and add_writer()/remove_writer() in ProactorEventLoop looks is more robust solution IMHO. |
|||
msg362643 - (view) | Author: Nathaniel Smith (njs) * | Date: 2020-02-25 14:59 | |
> was Tornado the only project experiencing this pain At least twisted and anyio are still broken on 3.8+Windows because of this change: https://twistedmatrix.com/trac/ticket/9766 https://github.com/agronholm/anyio/issues/77 |
|||
msg362658 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2020-02-25 18:58 | |
I considered using the `selectors` module directly, but it's not as simple as it sounds. Using the low-level interface means you need to also use a self-waker-pipe (or socket on windows) and manage a queue analogous to that used by `call_soon_threadsafe`. We already have two implementations of this pattern in asyncio with subtle differences between them (such as this one i just found: https://bugs.python.org/issue39651). In the end you'd have to duplicate a non-trivial portion of SelectorEventLoop. While there might be some efficiency gains to be had by working directly with the lower-level interface (and avoiding some redundancies between the two threads' event loops), I think the most robust/safest option is to use the well-tested SelectorEventLoop so that the only new code is what's needed to pass things back and forth between threads. |
|||
msg366792 - (view) | Author: Łukasz Langa (lukasz.langa) * | Date: 2020-04-19 17:13 | |
I'd be +1 to bringing uvloop into the stdlib, it would solve many things while introducing an acceptable dependency in the form of libuv. However, uvloop itself is written in Cython which makes it impossible for us to directly merge it. So that option is pretty much off the table as rewriting the library is not something we have resources for right now. Ben, Nathaniel is onto something though. Would it be acceptable for you to *require* use of uvloop when Tornado is used with AsyncIO? That indeed solves the Windows problems, and more. I use uvloop in all asyncio applications I maintain (*except for Black which is tested to work on Windows with the Proactor loop). For Python 3.9, it is a bit late, but not *too late* yet, to make the Proactor loop support AFD_POLL. Maybe counter-intuitively I would feel better about *that* rather than have a background thread with a SelectorEventLoop. |
|||
msg366795 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2020-04-19 18:21 | |
> Would it be acceptable for you to *require* use of uvloop when Tornado is used with AsyncIO? How, exactly? Adding the dependency is no problem, but AFAIK I'd still be stuck with an import-time side effect to set the event loop policy (or a .pth file hack, I guess). Maybe an import-time side effect that chooses uvloop is better since it's a functional superset of the two default windows event loops, but setting the policy at import time still has its problems (What if another module has initialized the event loop before tornado is imported? What if someone also wants to set a custom policy to work around asyncio's strict "only on the main thread" defaults?) That's why I started this thread not asking for a proactor+selector hybrid event loop, but a better way to *configure* the event loop because policies aren't really doing the job in this case. > I considered using the `selectors` module directly, but it's not as simple as it sounds. FWIW, I've reconsidered this. Treating SelectorEventLoop as a black box means you don't have enough control over synchronization and it's hard to avoid spurious wakeups and busy loops on the selector thread. I have a (broken) prototype using SelectorEventLoop that I plan to rewrite to call select.select directly. |
|||
msg393368 - (view) | Author: Min RK (minrk) * | Date: 2021-05-10 09:04 | |
A hiccup to using uvloop is that it doesn't support Windows yet (https://github.com/MagicStack/uvloop/issues/14), so it can't be used in the affected environment. I'm exploring this again for pyzmq / Jupyter, and currently investigating relying on tornado's AddThread loop functionality. It's even slightly easier for tornado, which can reasonably set the proactor-wrapper policy at IOLoop start time, which means `asyncio.get_event_loop()` returns a loop with add_reader. But pyzmq doesn't get invoked until an event loop is already running. That means the selector thread needs to work not as a wrapper of the loop itself, as in tornado's AddThreadSelector, but attached after-the-fact. Using tornado's AddThread seems to work for this, but I'm not sure that should be assumed. |
|||
msg393726 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2021-05-15 19:32 | |
> It's even slightly easier for tornado, which can reasonably set the proactor-wrapper policy at IOLoop start time, which means `asyncio.get_event_loop()` returns a loop with add_reader. But pyzmq doesn't get invoked until an event loop is already running. That's not what I'm doing in Tornado; I don't change the policy or the result of get_event_loop. Instead, I call get_event_loop (only once) and wrap its result in AddThreadSelectorEventLoop. This works even while the event loop is already running (which is not an uncommon case; there is no expectation that you use tornado.ioloop.IOLoop.start instead of asyncio.EventLoop.run_forever). This relies on the fact that I already have my own thread-local lookup function to retrieve the wrapped event loop; an application that used the more typical asyncio patterns and relied on get_event_loop would indeed have difficulty with this pattern. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:59:17 | admin | set | github: 81554 |
2021-05-15 19:32:35 | Ben.Darnell | set | messages: + msg393726 |
2021-05-10 09:04:30 | minrk | set | nosy:
+ minrk messages: + msg393368 |
2020-07-07 22:47:12 | jack1142 | set | nosy:
+ jack1142 |
2020-04-20 15:07:40 | cmeyer | set | nosy:
+ cmeyer |
2020-04-19 18:21:47 | Ben.Darnell | set | messages: + msg366795 |
2020-04-19 17:13:40 | lukasz.langa | set | messages: + msg366792 |
2020-02-25 18:58:15 | Ben.Darnell | set | messages: + msg362658 |
2020-02-25 14:59:24 | njs | set | messages: + msg362643 |
2020-02-24 14:15:00 | asvetlov | set | messages: + msg362587 |
2020-02-22 23:43:06 | Ben.Darnell | set | messages: + msg362483 |
2020-02-18 13:56:40 | vstinner | set | nosy:
- vstinner |
2020-02-18 04:41:47 | mikeshardmind | set | nosy:
+ mikeshardmind |
2020-02-12 08:52:07 | steve.dower | set | nosy:
+ steve.dower |
2020-01-15 19:16:33 | carltongibson | set | nosy:
+ carltongibson |
2019-11-05 01:23:24 | vstinner | set | messages: + msg355991 |
2019-11-03 17:53:58 | njs | set | messages: + msg355906 |
2019-11-03 11:37:04 | asvetlov | set | messages: + msg355893 |
2019-11-03 08:42:16 | Big Stone | set | messages: + msg355891 |
2019-10-03 07:16:24 | njs | set | nosy:
+ njs messages: + msg353823 |
2019-06-24 10:50:53 | vstinner | set | files:
+ selector_thread.py messages: + msg346369 |
2019-06-24 09:42:43 | asvetlov | set | messages: + msg346360 |
2019-06-23 23:32:52 | yselivanov | set | messages: + msg346348 |
2019-06-23 23:14:59 | Ben.Darnell | set | messages: + msg346347 |
2019-06-23 10:31:57 | asvetlov | set | messages: + msg346315 |
2019-06-23 01:19:01 | yselivanov | set | messages: + msg346311 |
2019-06-22 22:05:04 | Ben.Darnell | set | messages: + msg346302 |
2019-06-22 21:00:49 | asvetlov | set | messages: + msg346299 |
2019-06-22 19:54:05 | Big Stone | set | nosy:
+ Big Stone messages: + msg346295 |
2019-06-22 19:34:50 | yselivanov | set | nosy:
+ vstinner, lukasz.langa messages: + msg346293 |
2019-06-22 18:33:25 | Ben.Darnell | create |