asyncio: support multiprocessing (support fork)

[danoreilly]

BPO	22087
Nosy	@gvanrossum, @pitrou, @1st1, @thehesiod, @miss-islington
PRs	bpo-22087: Fix Policy.get_event_loop() to detect fork #7208 [3.7] bpo-22087: Fix Policy.get_event_loop() to detect fork (GH-7208) #7215 [3.6] bpo-22087: Fix Policy.get_event_loop() to detect fork (GH-7208) #7218 bpo-22087: Restructure code to be more robust and safe #7226 Revert "bpo-22087: Fix Policy.get_event_loop() to detect fork (GH-7208)" #7232 Revert "bpo-22087: Fix Policy.get_event_loop() to detect fork (GH-7208)" #7233
Files	test_loop.py: Test script demonstrating the issue handle_mp_unix.diff: Patch that makes _UnixDefaultEventLoopPolicy create a new loop object if get_event_loop is called in a forked mp child process handle-mp_unix2.patch: Use os.getpid() instead of multiprocessing. Store pid state in Policy instance rather than the Loop instance. handle_mp_unix_with_test.diff: Adds a unit test to previous patch

^{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 2014-07-26.18:01:10.150>
labels = ['type-bug', 'expert-asyncio']
title = 'asyncio: support multiprocessing (support fork)'
updated_at = <Date 2018-05-30.00:56:36.541>
user = 'https://bugs.python.org/danoreilly'

bugs.python.org fields:

activity = <Date 2018-05-30.00:56:36.541>
actor = 'yselivanov'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2014-07-26.18:01:10.150>
creator = 'dan.oreilly'
dependencies = []
files = ['36117', '36118', '36119', '36134']
hgrepos = []
issue_num = 22087
keywords = ['patch']
message_count = 23.0
messages = ['224082', '224084', '224085', '224097', '224125', '224140', '224143', '224144', '224145', '226698', '235404', '235411', '288327', '297222', '297226', '297227', '297229', '318077', '318092', '318135', '318140', '318143', '318144']
nosy_count = 7.0
nosy_names = ['gvanrossum', 'pitrou', 'zmedico', 'yselivanov', 'thehesiod', 'dan.oreilly', 'miss-islington']
pr_nums = ['7208', '7215', '7218', '7226', '7232', '7233']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22087'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

Linked PRs

• GH-66285: fix forking in asyncio #99539
• GH-66285: skip asyncio fork tests for platforms without md5 hash #99745
• GH-66285: Revert "fix forking in asyncio" #99756
• GH-66285: fix forking in asyncio #99769
• GH-66285: remove redundant time.sleep from test_fork_signal_handling #99963

[danoreilly]

On non-Windows platforms, if a user attempts to use asyncio.get_event_loop() in a child process created by multiprocessing.Process using the fork context, and an asyncio event loop is also being used in the main process, the same _UnixSelectorEventLoop object will be used by both processes. This, of course, won't work properly; the child will raise a "RuntimeError: Event loop is running" exception as soon as it tries using the loop object.

However, this may or may not actually make it back to the parent: If the parent is expecting to get items from a queue from that child publishes to, rather than yielding from it immediately, the program will deadlock. Even if the child is yielded from, it may not be immediately obvious why "Event loop is running" was raised, and the behavior is inconsistent with the behavior if a method other than os.fork is used to create the child process, since the child will get a new event loop in that case.

So, it'd be better if _UnixDefaultEventLoopPolicy detected that get_event_loop was being called in a child process, and either

1. Created a new loop for the child (this would make the behavior appear consistent no matter what platform/method for launching children is used)
2. Raised an exception stating that no default event loop exists for this process, similar to the assert used for threads currently.

I've attached a test script that demonstrates the different between forked/spawned processes, and a patch that implements #1 above.

[gvanrossum]

Good point. Asyncio definitely should not share event loops across forked processes. However, I don't like the dependency on multiprocessing (even though it's in the stdlib) -- can't the policy just use os.getpid()?

Also, I've got a feeling that maybe the pid should be part of the policy state instead of the loop state? The policy could just reset self._local when the pid doesn't match.

[danoreilly]

Yep, agreed on both points. The latter suggestion also has the benefit of not requiring any test changes. Here's an updated patch.

[gvanrossum]

I think there should still be a new unittest -- we're adding a behavior we're promising, so we should test it.

[vstinner]

See aslo issue bpo-21998: "asyncio: a new self-pipe should be created in the child process after fork".

[danoreilly]

I've added a unit test that spawns a new forked process via multiprocessing, and verifies that the loop returned by get_event_loop is not the same as the one we have in the parent.

[danoreilly]

re: bpo-21998, perhaps it's time to revive bpo-16500? Without that, I'm not sure what can be done aside from documenting the need to call "loop = asyncio.get_event_loop()" in the child immediately after forking.

[vstinner]

A simple pid check in the policy should be enough.

[danoreilly]

Hmm, I'm not sure what you mean. What check in the policy would prevent this issue you described in bpo-21998?:

import asyncio, os
loop = asyncio.get_event_loop()
pid = os.fork()
if pid:
 print("parent", loop._csock.fileno(), loop._ssock.fileno())
else:
 print("child", loop._csock.fileno(), loop._ssock.fileno())

Output:
---
parent 6 5
child 6 5

[danoreilly]

Are any other changes needed here? I'm still not completely clear on what Victor meant with his last comment.

[vstinner]

This issue looks to be a duplicate of bpo-21998.

handle-mp_unix2.patch looks more to a workaround than a real issue. When I write asyncio code, I prefer to pass explicitly the loop, so get_event_loop() should never be called. IMO the methods of the event loop should detect the fork and handle the fork directly.