bpo-30966: Add multiprocessing.SimpleQueue.close()#19735
bpo-30966: Add multiprocessing.SimpleQueue.close()#19735miss-islington merged 2 commits intopython:masterfrom vstinner:mp_simplequeue_close
Conversation
Add a new close() method to multiprocessing.SimpleQueue to explicitly close the queue.
|
This PR is my 3rd attempt to add the method!
|
|
Can we please start with this dead simple implementaiton for "simple" queue, and enhance it later? https://bugs.python.org/issue30966 is open for 3 years because of bikesheding on how to write the "perfect" implementation. I need SimpleQueue.close() to fix concurrent.futures.Process.shutdown(): I would like to explicitly close the queue. cc @pitrou @serhiy-storchaka @cfbolz @arigo -- Previous issues on my two previous PRs. The first version of my first PR added a I have been asked to ensure that get() and put() methods fail with an error after the queue is closed... This point triggered most following issues... Can we please start with this simplistic PR, and enhance the code later? When I added an explicit _check_closed() call in get() and put(), @serhiy-storchaka asked to measure the performance overhead. Then a discussion on thread-safety was started with my changes. The problem is that it's no clear if right now (without my changes) SimpleQueue is thread-safe or nor. Can we please address this concern later? After many discussions and many versions of my PR, Antoine concluded with:
That's what I am proposing here! Antoine also wrote (in my second PR):
|
Doc/library/multiprocessing.rst
Outdated
|
|
||
| .. method:: close() | ||
|
|
||
| Close the queue. |
There was a problem hiding this comment.
From the reader's point of view, it isn't clear what the point of the method is. Can you add a sentence explaining what the consequences are? (I suppose that the underlying resources are released and the queue becomes unusable?)
There was a problem hiding this comment.
(I suppose that the underlying resources are released and the queue becomes unusable?)
Right. I completed the documentation. Does it look better?
|
When you're done making the requested changes, leave the comment: |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @pitrou: please review the changes made to this pull request. |
|
@vstinner: Status check is done, and it's a success ✅ . |
Add a new close() method to multiprocessing.SimpleQueue to explicitly
close the queue.
https://bugs.python.org/issue30966
Automerge-Triggered-By: @pitrou