Add multiprocessing.queues.SimpleQueue.close()

[arigo]

BPO	30966
Nosy	@arigo, @pitrou, @vstinner, @applio, @zhangyangyu, @miss-islington, @iritkatriel
PRs	bpo-30966: Add multiprocessing.SimpleQueue.close() #2760 bpo-30966: Add multiprocessing.SimpleQueue.close() #2776 bpo-30966: Add multiprocessing.SimpleQueue.close() #19735 bpo-30966: concurrent.futures.Process.shutdown() closes queue #19738

^{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 2021-06-21.13:50:44.648>
created_at = <Date 2017-07-19.08:53:22.600>
labels = ['3.9', 'performance']
title = 'Add multiprocessing.queues.SimpleQueue.close()'
updated_at = <Date 2021-06-21.14:00:05.733>
user = 'https://github.com/arigo'

bugs.python.org fields:

activity = <Date 2021-06-21.14:00:05.733>
actor = 'iritkatriel'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-21.13:50:44.648>
closer = 'pitrou'
components = []
creation = <Date 2017-07-19.08:53:22.600>
creator = 'arigo'
dependencies = []
files = []
hgrepos = []
issue_num = 30966
keywords = ['patch']
message_count = 11.0
messages = ['298645', '298649', '298651', '298657', '298660', '298661', '298667', '367432', '367435', '367445', '396246']
nosy_count = 7.0
nosy_names = ['arigo', 'pitrou', 'vstinner', 'davin', 'xiang.zhang', 'miss-islington', 'iritkatriel']
pr_nums = ['2760', '2776', '19735', '19738']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue30966'
versions = ['Python 3.9']

[arigo]

multiprocessing.queues.SimpleQueue should have a close() method. This is needed to explicitly release the two file descriptors of the Pipe used internally. Without it, the file descriptors leak if a reference to the SimpleQueue object happens to stay around for longer than expected (e.g. in a reference cycle, or with PyPy).

I think the following would do:

diff -r 0b72fd1a7641 lib-python/2.7/multiprocessing/queues.py
--- a/lib-python/2.7/multiprocessing/queues.py Sun Jul 16 13:41:28 2017 +0200
+++ b/lib-python/2.7/multiprocessing/queues.py Wed Jul 19 10:45:03 2017 +0200
@@ -358,6 +358,11 @@
self._wlock = Lock()
self._make_methods()

+ def close(self):
+ # PyPy extension: CPython doesn't have this method!
+ self._reader.close()
+ self._writer.close()
+
def empty(self):
return not self._reader.poll()

[zhangyangyu]

Get this solved then we could also solve bpo-23267, which reports pipes leak in pool due to using SimpleQueue.

[vstinner]

On Python 3.6+, regrtest is able to detect file descriptor leaks when using --huntrleaks. Is someone interested to backport this feature to 3.5 and/or 2.7 which would mean fix all FD leaks in our test suite?

If someone wants to work on that, I would suggest to first fix all bugs, and when the test suite is fine: modify regrtest.

[vstinner]

See also my bpo-30171: "Emit ResourceWarning in multiprocessing Queue destructor".

[vstinner]

This issue is marked as also affecting Python 3.7. I don't see a SimpleQueue.close() method in master, but I don't remind any resource warning when running tests on master neither.

Does it mean that our test runner miss such file descriptor leak? Or worse, that SimpleQueue is not tested? I see a TestSimpleQueue test case in Lib/test/_test_multiprocessing.py.

haypo@selma$ ./python -m test test_multiprocessing_spawn -m TestSimpleQueue -R 3:3
(...)
Tests result: SUCCESS

This is needed to explicitly release the two file descriptors of the Pipe used internally. Without it, the file descriptors leak if a reference to the SimpleQueue object happens to stay around for longer than expected (e.g. in a reference cycle, or with PyPy).

Oh ok, the garbage collector is able to close the file descriptors. The bug is when a SimpleQueue is not collected.

So again, I consider that a ResourceWarning is needed here. The purpose of a ResourceWarning is to warn when a leak may be kept alive longer than expected if it's not closed explicitly.

[pitrou]

Anyone who thinks those objects stay alive too long can submit a PR for SimpleQueue and Pool to close them eagerly.

[vstinner]

Anyone who thinks those objects stay alive too long can submit a PR for SimpleQueue and Pool to close them eagerly.

I started with SimpleQueue for the master branch:
#2760

[vstinner]

bpo-23267 is marked as a duplicate of this issue.

[miss-islington]

New changeset 9adccc1 by Victor Stinner in branch 'master':
bpo-30966: Add multiprocessing.SimpleQueue.close() (GH-19735)
9adccc1

[vstinner]

New changeset 1a27501 by Victor Stinner in branch 'master':
bpo-30966: concurrent.futures.Process.shutdown() closes queue (GH-19738)
1a27501

[iritkatriel]

This seems complete, can it be closed?