Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asyncio: will shutdown_default_executor work in single step (stop, run_forever) mode? #84537

Closed
cmeyer mannequin opened this issue Apr 21, 2020 · 3 comments
Closed

asyncio: will shutdown_default_executor work in single step (stop, run_forever) mode? #84537

cmeyer mannequin opened this issue Apr 21, 2020 · 3 comments

Comments

@cmeyer
Copy link
Mannequin

cmeyer mannequin commented Apr 21, 2020

BPO 40357
Nosy @cmeyer, @aeros

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 = None
created_at = <Date 2020-04-21.22:00:47.811>
labels = ['3.9']
title = 'asyncio: will shutdown_default_executor work in single step (stop, run_forever) mode?'
updated_at = <Date 2020-05-06.04:43:16.445>
user = 'https://github.com/cmeyer'

bugs.python.org fields:

activity = <Date 2020-05-06.04:43:16.445>
actor = 'aeros'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2020-04-21.22:00:47.811>
creator = 'cmeyer'
dependencies = []
files = []
hgrepos = []
issue_num = 40357
keywords = []
message_count = 2.0
messages = ['366945', '368224']
nosy_count = 2.0
nosy_names = ['cmeyer', 'aeros']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue40357'
versions = ['Python 3.9']

@cmeyer
Copy link
Mannequin Author

cmeyer mannequin commented Apr 21, 2020

Is the new asyncio.loop.shutdown_default_executor() suitable for event loops that are run in single-step mode?

event_loop.create_task(event_loop.shutdown_default_executor())
event_loop.stop()
event_loop.run_forever()

I don't see how it will work since shutdown_default_executor() may not be finished during a single 'stopped' run_forever() call.

Also, what happens to pending executor futures? Previously reported bug bpo-28464.

Here is my currently working code for shutting down the event loop.

# give event loop one chance to finish up
event_loop.stop()
event_loop.run_forever()
# wait for everything to finish, including tasks running in executors
# this assumes that all outstanding tasks finish in a reasonable time (i.e. no infinite loops).
all_tasks_fn = getattr(asyncio, "all_tasks", None)
if not all_tasks_fn:
    all_tasks_fn = asyncio.Task.all_tasks
tasks = all_tasks_fn(loop=event_loop)
if tasks:
    gather_future = asyncio.gather(*tasks, return_exceptions=True)
else:
    # work around fact that gather always uses global event loop in Python 3.8
    gather_future = event_loop.create_future()
    gather_future.set_result([])
event_loop.run_until_complete(gather_future)
# due to a seeming bug in Python libraries, the default executor needs to be shutdown explicitly before the event loop
# see http://bugs.python.org/issue28464 .
_default_executor = getattr(event_loop, "_default_executor", None)
if _default_executor:
    _default_executor.shutdown()
event_loop.close()

@cmeyer cmeyer mannequin added the 3.9 only security fixes label Apr 21, 2020
@aeros
Copy link
Contributor

aeros commented May 6, 2020

Is the new asyncio.loop.shutdown_default_executor() suitable for event loops that are run in single-step mode?

I honestly can't say for certain; the primary intended use case for shutdown_default_executor() was to provide a means of properly finalizing the resources associated with the default executor without blocking the event loop, notably the threads in the ThreadPoolExecutor (which were intermittently left dangling). So, when working on the implementation w/ Yury and Andrew, I did not strongly consider single-step event loops. I was more concerned with how it fit in with asyncio.run() and safe finalization of executor resources. See https://bugs.python.org/issue34037 for context.

If you have a recommendation for a change to the current version shutdown_default_executor() that would help provide compatibility with single-step event loops without hindering the primary goals, I'm sure it would be considered.

Also, what happens to pending executor futures?

When using loop.shutdown_default_executor(), it calls executor.shutdown(wait=True), which waits for submitted futures to the executor to complete before joining the executor's workers (regardless of whether they're threads or processes). So, the executor should not be terminated prior to the pending futures being completed.

From a glance at the example code posted above, it seems like it would be incompatible with asyncio.run(), which is a requirement for shutdown_default_executor(). See

def run(main, *, debug=False):
.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@kumaraditya303
Copy link
Contributor

This isn't a supported use-case. You are supposed to await loop.shutdown_default_executor not create a task for it.

@kumaraditya303 kumaraditya303 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 13, 2022
@kumaraditya303 kumaraditya303 removed the 3.9 only security fixes label Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants