Repository navigation
multiprocessing: Add a stop() method to ForkServer #81688
Description
| BPO | 37507 |
|---|---|
| Nosy | @pitrou, @vstinner, @applio, @pablogsal |
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 2019-10-01.10:33:10.128>
created_at = <Date 2019-07-05.14:22:18.080>
labels = ['library', '3.9']
title = 'multiprocessing: Add a stop() method to ForkServer'
updated_at = <Date 2019-10-01.10:33:10.127>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2019-10-01.10:33:10.127>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-10-01.10:33:10.128>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2019-07-05.14:22:18.080>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 37507
keywords = []
message_count = 6.0
messages = ['347351', '347441', '347488', '347489', '347491', '353662']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'davin', 'pablogsal']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue37507'
versions = ['Python 3.9']
Activity
vstinner commentedon Jul 5, 2019
While working on bpo-37421, I fixed multiprocessing tests to explicitly call _run_finaliers(), so temporary directories are removed:
New changeset 039fb49 by Victor Stinner in branch 'master':
bpo-37421: multiprocessing tests call _run_finalizers() (GH-14527)
039fb49
Problem: some tests started to fail when run more than once, which happens using ./python -m test -R 3:3 (hunt reference leaks). So I made a first change:
New changeset 9d40554 by Victor Stinner in branch 'master':
bpo-37421: Fix multiprocessing get_temp_dir() finalizer (GH-14572)
9d40554
But I also had to explicitly stop the ForkServer:
New changeset 9d40554 by Victor Stinner in branch 'master':
bpo-37421: Fix multiprocessing get_temp_dir() finalizer (GH-14572)
9d40554
I propose to add a public ForkServer.stop() method to stop the server. The method would block until the server stops. The server doesn't track its children, so if there are running child processes, they will continue to run after stop() completes. Child processes are tracked by APIs like multiprocessing Pool.
I dislike relying on implicit resource management. I prefer to handle them explicitly, to ensure that errors are properly reported if something goes wrong. Implicit resource management rely on Python destructor which may be called very late during Python finalization, while other modules are already cleaned up, and so some function may fail silently (the machineny to report issues is broken as well).
In short, I propose to rename the new _stop() method as stop() and document it properly :-)
pitrou commentedon Jul 6, 2019
There is no reason to expose "stop the forkserver" to users. How the forkserver process works, what tasks it handles, is an implementation detail. If users start "stopping the forkserver" in their applications for no good to reason, they might get bugs now or later that they wouldn't get otherwise.
Side note: if you're modifying multiprocessing, asking for a review doesn't hurt ;-)
vstinner commentedon Jul 8, 2019
Well, we already had this conversation before. I really dislike APIs which hold resources but disallow to "release resources", especially when a resource is not a few bytes of memory, but a whole process.
Since there is a method to start the server (ensure_running), I would like to control when it's stopped "to ensure that errors are properly reported if something goes wrong."
That's why I added Process.close() in Python 3.7 and ResourceWarning in Pool in Python 3.8.
Here I don't propose to add a ResourceWarning if the server is not stopped explicitly. I only propose to add a method if the developer wants to control when it's stopped.
vstinner commentedon Jul 8, 2019
I only modified tests (I only added a *private* _stop() method which is only used by tests). When I touch the multiprocessing module directly, I wait for a review.
Right now, I'm embarrassed since my "multiprocessing tests call _run_finalizers()" change which was supposed to be simple and safe broke Refleak buildbots. I had to fix test_concurrent_futures to also explicitly closes the forkserver there:
#14643
I was quite surprised to discover that a test leaves a child process in background. IMHO tests should have zero side effects on following tests (at least a test file must clear all its resources when it completes).
Python finalization is *very* fragile (not reliable at all), so I prefer to release resources as soon as possible, to get errors reported. Multiprocessing tests failures are still common, and very likely caused by these small details like a server which isn't stopped properly (IMHO). Tests should get a fresh and reliable environment.
Maybe each test should ensure that the forkserver is stopped, but I chose the easy fix: only stop it once at (test file) exit. Start/stop the server in each test might make these tests even slower, whereas multiprocessing tests are almost part of the slowest tests.
vstinner commentedon Jul 8, 2019
Previous related discussion on python-dev:
https://mail.python.org/archives/list/python-dev@python.org/thread/NJCLS7IOKKO6W7LBGPQI2VFAO57EJEPW/#DIOMN256E24NUBSPKWFQA556F43IOWXR
vstinner commentedon Oct 1, 2019
_test_multiprocessing.py calls:
It seems like multiprocessing tests are the main consumer of this method. So let's keep it private for now.
grayjk commentedon Nov 25, 2024
stop() would be useful in order to change the forkserver list of preload modules during runtime:
vstinner commentedon Nov 26, 2024
This issue is closed. Please open a new issue if you would like to get a new public stop() method.