bpo-37137, asyncio: add BaseEventLoop.wait_executor_on_close

[vstinner]

Add BaseEventLoop.wait_executor_on_close attribute: true by default.

loop.close() now waits for the default executor to finish by default.
Set loop.wait_executor_on_close attribute to False to not wait for
the executor.

https://bugs.python.org/issue37137

[vstinner]

See https://bugs.python.org/issue34037

[vstinner]

About the overall idea, Yury Selivanov wrote:

Yury, Andrew: Do you know if the executor doesn't wait on purpose? Would it be possible to change that in Python 3.8?

Maybe. At least we need to add a "timeout" argument to asyncio.run() to let it wait for executor jobs.

I'm also thinking about making OS threads cancellable/interruptable in Python 3.8 (if they run pure Python code).

https://bugs.python.org/issue34037#msg326052

[vstinner]

I used git commit --amend to fix the bpo number: https://bugs.python.org/issue34037 is the right issue with a better history.

[asvetlov]

I support the idea but have a question about implementation.
asyncio provides an interface class for loop called AbstractEventLoop.
I believe that the new property should be reflected there.
The next question is: should it be a property or get_wait_executor_on_close() / set_wait_executor_on_close() pair of methods? Asyncio loop is designed to don't use properties but getters / setters. I think we should keep this approach for sake of uniformemess.

@vstinner if you agree I can prepare a PR.

[vstinner]

First of all, I wrote this PR to attempt to fix https://bugs.python.org/issue37137 Sadly and surprisingly (for me), it didn't fix https://bugs.python.org/issue37137 We might simply revert this change later if needed. I'm just trying to get green buildbots to be able to release beta1.

I support the idea but have a question about implementation. asyncio provides an interface class for loop called AbstractEventLoop. I believe that the new property should be reflected there.

For me, it's more an implementation detail. I don't think that it should belong to the abstract class. A completely different implementation can decide to not implement this attribute.

The next question is: should it be a property or get_wait_executor_on_close() / set_wait_executor_on_close() pair of methods? Asyncio loop is designed to don't use properties but getters / setters. I think we should keep this approach for sake of uniformemess.

This attribute is a dummy boolean, I don't see which kind of extra code you should like to execute when the attribute is set or get. But it's more up to you :-)

[asvetlov]

asyncio explicitly supports concurrent.futures executors by loop.run_in_executor().

Technically there is a possibility to write default (None) executor without concurrent.futures.ThreadPoolExecutor but it is very unlikely. Thus in loop.close() we always should to decide whether call executor.shutdown(wait) with True or False.

That's why I think this is a part of the public API.

[1st1]

I don't like this last minute api decision. Please don't merge this.

[vstinner]

I don't like this last minute api decision. Please don't merge this.

See my previous comment. https://bugs.python.org/issue37137 is blocking Python 3.8 beta1 release. I tried 2 fixes, but it sounds like Andrew found the root issue: https://bugs.python.org/issue37137#msg344488

Once beta1 will be released, we can revert other attempts to fix https://bugs.python.org/issue37137