asyncio: We should prohibit setting a ProcessPoolExecutor in with set_default_executor

[1st1]

BPO	34075
Nosy	@gvanrossum, @vstinner, @asvetlov, @1st1, @basnijholt
PRs	bpo-34075: Deprecate non-ThreadPoolExecutor in loop.set_default_executor() #8533

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-07-30.10:43:39.503>
created_at = <Date 2018-07-09.15:29:34.966>
labels = ['type-bug', '3.8', 'expert-asyncio']
title = 'asyncio: We should prohibit setting a ProcessPoolExecutor in with set_default_executor'
updated_at = <Date 2019-03-26.11:40:40.218>
user = 'https://github.com/1st1'

bugs.python.org fields:

activity = <Date 2019-03-26.11:40:40.218>
actor = 'basnijholt'
assignee = 'none'
closed = True
closed_date = <Date 2018-07-30.10:43:39.503>
closer = 'vstinner'
components = ['asyncio']
creation = <Date 2018-07-09.15:29:34.966>
creator = 'yselivanov'
dependencies = []
files = []
hgrepos = []
issue_num = 34075
keywords = ['patch']
message_count = 13.0
messages = ['321324', '321330', '321331', '321338', '321339', '321459', '321469', '321476', '321479', '321620', '322665', '322666', '338872']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'vstinner', 'asvetlov', 'yselivanov', 'basnijholt']
pr_nums = ['8533']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue34075'
versions = ['Python 3.8']

[1st1]

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

[gvanrossum]

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.

[1st1]

Great! Thanks for the quick reply, Guido.

[1st1]

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.

[gvanrossum]

Of course, that's what I meant.

[vstinner]

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.

[gvanrossum]

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)

[vstinner]

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 :-)

[1st1]

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.

[asvetlov]

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.

[vstinner]

New changeset 22d2508 by Victor Stinner (Elvis Pranskevichus) in branch 'master':
bpo-34075: Deprecate non-ThreadPoolExecutor in loop.set_default_executor() (GH-8533)
22d2508

[vstinner]

Elvis Pranskevichus: well done, your first PR was already good to be merged ;-)

[basnijholt]

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 22d2508 won't really help.