gh-96471: Add threading queue shutdown #104750
Conversation
* Include docs
* Include raised exception in docstrings * Handle queue shutdown in task_done and join * Factor out queue-state checks and updates to methods * Logic fixes in qsize, get and shutdown * Don't set unfinished_tasks to 0 on immediate shutdown * Updated tests * Document feature added in 3.13
Misc/NEWS.d/next/Library/2023-05-06-04-57-10.gh-issue-96471.C9wAU7.rst
Outdated
Show resolved
Hide resolved
|
Yeah, the test is definitely hanging for me. Have you figured that out yet? |
|
I'd also recommend a merge. :-) |
…tdown-immediate-consume
@gvanrossum It's intermittently hanging in the test I think the solution is to simply ensure Perhaps it's simpler to set Edit: I just noticed that this pull request's description says that It's also intermittently hanging in the test Solution here (which fixes the assertion for Edit: and of course now it's failing only in Windows (CI only succeeds because it re-runs |
Also shut down before put-join in shutdown-put-join tests. Also remove indent
9d0a083 to
9072c6f
Compare
There was a problem hiding this comment.
LG, assuming you believe the test is stable. I have one nit, let me know how you feel about that.
Oh, one more thing. If you feel like writing documentation, could you update Doc/whatsnew/3.13.rst? There's a section about modified modules. A few lines there will go a long way.
Test is not stable, as |
|
@EpicWink Sorry, it looks like we have to revert this until the Windows free-threading hang has been resolved. (Although it's possible that you've uncovered a Windows-related bug in free-threading, which is a developing feature.) |
|
FWIW, having had only a quick look at the failing test, is it possible that it relies on timeouts too much? I see that there's a 0.1 msec delay that is used for sleeping, and |
Co-authored-by: Duprat <yduprat@gmail.com>
|
From #104228 (comment): it looks like |
|
Maybe I'm waking up a little too late, but despite having worked on the development 1 year ago, I have the impression that functionalities are missing in this merged PR. In the initial version of the feature (#104225), the basic shutdown just forbade The immediate shutdown prohibited all possible previous operations and had to release all threads blocked in Currently, in the following methods ( I don't want to hurt anyone's feelings, but I think this PR needs to be reworked (including documentation). PS: initial comment was done at the bad PR. Sorry |
@YvesDup see comment in the issue: #96471 (comment) Basically, the goal is to have waiters (callers of queue methods) not be blocked on a queue which has been shut-down. Making the shutdown immediate simply means gets won't take anything more from the queue (and instead except). There is a problem with this PR though, in that |
|
@EpicWink , thank you for this explanation. I ve missed that :-( |
|
FYI, I fixed the failling test in |
Where can we see this? |
|
|
Thanks, I'll wait for the pull request. |
PR is ready on this issue (#115940). Sorry if I didn't understand that you wait for a PR linked to this one. Please let me know if I have to change. |
|
@EpicWink: We're waiting for you! ^^ |
|
I'll review the new PR fixing the test today. My PR is another attempt, but unfinished. Perhaps in the future it can be added as another test. Edit: reviewed! |
Thank for the review. |
…ython#117532) (This is a small tweak of the original pythongh-104750 which added shutdown.)
Alternate implementation of #104225, where all queue items are consumed immediately in
Queue.shutdownwhenimmediate=Trueis passed (see the comparison for what's changed).task_doneandjoinwill now not raise after callingshutdown(immediate=True), rather they'll behave as if on a finished queue (joinreturns,task_doneraises the standardValueError)is_shutdownboolean attributeThis PR includes and modified changes from #104225.
📚 Documentation preview 📚: https://cpython-previews--104750.org.readthedocs.build/