Issue 34075: asyncio: We should prohibit setting a ProcessPoolExecutor in with set_default_executor (original) (raw)

Created on 2018-07-09 15:29 by yselivanov, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8533 merged Elvis.Pranskevichus,2018-07-28 16:13
Messages (13)
msg321324 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-07-09 15:29
I've had a few conversations with people who were confused that asyncio starts to behave weirdly when a ProcessPoolExecutor is set as the default one. We don't really test that asyncio's built-in functionality (like DNS resolving) works well with a process-pool, which leads to bug reports like [1]. Third-party libraries also always assume that the loop is always configured to use the ThreadPoolExecutor (as it is by default), and also don't even test against ProcessPool. My idea here would be to deprecate setting ProcessPoolExecutor as a default one in 3.8 and prohibit that in 3.9. Guido, Andrew, what do you think? [1] https://bugs.python.org/issue34073
msg321330 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-07-09 16:12
Yeah, that's a good idea. It was never meant for a ProcessPoolExecutor. We should warn against this in the docs right away (and backport the warning to all previous versions that have set_executor). I also support non-silent deprecation in 3.8.
msg321331 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-07-09 16:16
Great! Thanks for the quick reply, Guido.
msg321338 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-07-09 18:23
> We should warn against this in the docs right away (and backport the warning to all previous versions that have set_executor). I think we'll only allow instances of c.f.ThreadPoolExecutor (and its subclasses) to be passed to set_default_executor. That's a more robust check than just guarding against ProcessPoolExecutor.
msg321339 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-07-09 18:24
Of course, that's what I meant.
msg321459 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-11 14:34
> I think we'll only allow instances of c.f.ThreadPoolExecutor (and its subclasses) to be passed to set_default_executor. That's a more robust check than just guarding against ProcessPoolExecutor. I suggest to only reject ProcessPoolExecutor, instead of having a very specific test on ThreadPoolExecutor which might fail with funky but valid thread executor.
msg321469 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-07-11 14:53
I disagree. Other than subclasses of thread executor, what are you going to pass in? A mock? On Wed, Jul 11, 2018 at 7:34 AM STINNER Victor <report@bugs.python.org> wrote: > > STINNER Victor <vstinner@redhat.com> added the comment: > > > I think we'll only allow instances of c.f.ThreadPoolExecutor (and its > subclasses) to be passed to set_default_executor. That's a more robust > check than just guarding against ProcessPoolExecutor. > > I suggest to only reject ProcessPoolExecutor, instead of having a very > specific test on ThreadPoolExecutor which might fail with funky but valid > thread executor. > > ---------- > nosy: +vstinner > > _______________________________________ > Python tracker <report@bugs.python.org> > <https://bugs.python.org/issue34075> > _______________________________________ > -- --Guido (mobile)
msg321476 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-11 15:08
> Other than subclasses of thread executor, what are you going to pass in? I don't see why asyncio should prevent people to experiment their own custom executor. You can imagine a custom "green executor" which inherit from Executor but uses its own black magic like greenlet. People like to do crazy things to implement asynchronous programming :-)
msg321479 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-07-11 15:14
> I don't see why asyncio should prevent people to experiment their own custom executor. You can imagine a custom "green executor" which inherit from Executor but uses its own black magic like greenlet. Because asyncio and its ecosystem is built around the fact that the default executor is a ThreadPoolExecutor. asyncio users are free to: 1. Pass any kind of executor as a first argument to `loop.run_in_executor()` -- no restrictions there. 2. Pass a subclass of ThreadPoolExecutor to `loop.set_default_executor()` -- that's how people can experiment. Mind that (2) also covers exotic use cases like using greenlets under the hood (although I don't think it's possible), as long as they subclass ThreadPoolExecutor. My plan so far: * in 3.8 add a DeprecationWarning if an executor other than ThreadPoolExecutor is set as a default one. * during 3.8 see what kind of feedback we get. * if all goes great, in 3.9 we'll only allow subclasses of ThreadPoolExecutor.
msg321620 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2018-07-13 15:29
Agree, restricting to ThreadPoolExecutor sounds reasonable. A custom executor may have the same problem as ProcessPoolExecutor. Moreover it can work under same scenarios but crash with other third-party libs.
msg322665 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-30 10:42
New changeset 22d25085db2590932b3664ca32ab82c08f2eb2db by Victor Stinner (Elvis Pranskevichus) in branch 'master': bpo-34075: Deprecate non-ThreadPoolExecutor in loop.set_default_executor() (GH-8533) https://github.com/python/cpython/commit/22d25085db2590932b3664ca32ab82c08f2eb2db
msg322666 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-30 10:43
Elvis Pranskevichus: well done, your first PR was already good to be merged ;-)
msg338872 - (view) Author: Bas Nijholt (basnijholt) Date: 2019-03-26 11:40
I think this issue is related to the problem in https://bugs.python.org/issue36281 If it indeed is the case, then the fix proposed here and implemented in https://github.com/python/cpython/commit/22d25085db2590932b3664ca32ab82c08f2eb2db won't really help.
History
Date User Action Args
2022-04-11 14:59:02 admin set github: 78256
2019-03-26 11:40:40 basnijholt set nosy: + basnijholtmessages: +
2018-07-30 10:43:39 vstinner set status: open -> closedresolution: fixedmessages: + stage: patch review -> resolved
2018-07-30 10:42:48 vstinner set messages: +
2018-07-28 16:13:03 Elvis.Pranskevichus set keywords: + patchstage: patch reviewpull_requests: + <pull%5Frequest8050>
2018-07-13 15:29:21 asvetlov set messages: +
2018-07-11 15:14:35 yselivanov set messages: +
2018-07-11 15:08:56 vstinner set messages: +
2018-07-11 14:53:02 gvanrossum set messages: +
2018-07-11 14:34:44 vstinner set nosy: + vstinnermessages: +
2018-07-11 14:33:26 vstinner set title: We should prohibit setting a ProcessPoolExecutor in with set_default_executor -> asyncio: We should prohibit setting a ProcessPoolExecutor in with set_default_executor
2018-07-09 18:24:55 gvanrossum set messages: +
2018-07-09 18:23:53 yselivanov set messages: +
2018-07-09 16:16:08 yselivanov set messages: +
2018-07-09 16:12:47 gvanrossum set messages: +
2018-07-09 15:29:34 yselivanov create