bpo-30966: Add multiprocessing.SimpleQueue.close()

[vstinner]

Add a new close() method to multiprocessing.SimpleQueue to explicitly
close the queue.

https://bugs.python.org/issue30966

Automerge-Triggered-By: @pitrou

[vstinner]

This PR is my 3rd attempt to add the method!

• 1st attempt: PR bpo-30966: Add multiprocessing.SimpleQueue.close() #2760
• 2nd attempt: PR bpo-30966: Add multiprocessing.SimpleQueue.close() #2776

[vstinner]

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 __del__() method to emit a ResourceWarning if the queue is not closed explicitly. I didn't add the method in this PR. We can add it later in a separated PR.

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:

Ok, I think the current diff has gone too far :-) If raising ValueError cannot be provided without adding a lot of locking and complication in the implementation, then I'll happily lose the ValueError requirement so that the implementation remains simple and lightweight (it's called "SimpleQueue" for a reason :-)).

That's what I am proposing here!

Antoine also wrote (in my second PR):

Actually, I think we are mistaken here. SimpleQueue is not meant to be thread-safe. It's meant to be usable from multiple processes (the locks are multiprocessing Lock instances). Each process gets its own SimpleQueue instance pointing to the same underlying resource (e.g. an anonymous pipe).
Of course, it can be a bit surprising that a multiprocess-safe object is not thread-safe, so I'm still a bit uncomfortable about this.

[pitrou]

LGTM apart from the docs. Thank you @vstinner :-)

[pitrou]

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?)

[vstinner]

(I suppose that the underlying resources are released and the queue becomes unusable?)

Right. I completed the documentation. Does it look better?

[bedevere-bot]

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

[vstinner]

I have made the requested changes; please review again.

[bedevere-bot]

Thanks for making the requested changes!

@pitrou: please review the changes made to this pull request.

[miss-islington]

@vstinner: Status check is done, and it's a success ✅ .