Yeah, the test is definitely hanging for me. Have you figured that out yet?

I'd also recommend a merge. :-)

Yeah, the test is definitely hanging for me. Have you figured that out yet?

@gvanrossum It's intermittently hanging in the testtest_shutdown_immediate_put_join.q.join is blocking, after shutting down the queue, becauseunfinished_tasks becomes-1 (when zero is required). This is because the test callstask_done before taking items from the queue, but the implementation ofshutdown() reducesunfinished_tasks by the number of items in the queue.

I think the solution is to simply ensureunfinished_tasks doesn't go negative. I don't set it straight to zero because there may still be queue consumer who will successfully complete their task and calltask_done, but they will get aValueError ifunfinished_tasks was immediately set to zero. There is a problem here however if some consumers still calltask_done without getting items from the queue.

Perhaps it's simpler to setunfinished_tasks to zero, then skip theValueError raise if the queue is shut down.

Edit: I just noticed that this pull request's description says thattask_done should raiseValueError on a shut down queue. Hmmm.

It's also intermittently hanging in the testtest_shutdown_put_join, simply because of a logic error: 2 items are put on the queue (sometimes), then it's shut down, then one task is marked as done, then the queue is waited for (aka joined on). If only one item was put on the queue, then the test simply fails during assertion.

Solution here (which fixes the assertion fortest_shutdown_immediate_put_join as well) is to shut down the queue before running the put-then-join.

Edit: and of course now it's failing only in Windows (CI only succeeds because it re-runstest_queue)

LG, assuming you believe the test is stable.

Test is not stable, astest_shutdown_all_methods_in_many_threads (and the correspondingimmediate version) hangs intermittently in Windows. It also fails consistently when adding debugging print-statements on my machine. I'm investigating

@EpicWink Sorry, it looks like we have to revert this until the Windows free-threading hang has been resolved. (Although it'spossible 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_read_msg_thread() makes some non-blockingget() calls, ignoringEmpty exceptions. Could there be a scenario (especially when running without GIL) where different threads run in a different order than you anticipated, and it simply doesn't read enough queue items, so the join() hangs forever?

From#104228 (comment): it looks likeQueue.join() never raisesShutDown, contradicting the docs.

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 forbadeput operations, also released all threads blocked input. On the other hand, it leftget,join andtask_done operations possible.

The immediate shutdown prohibited all possible previous operations and had to release all threads blocked inget, andjoin. And the queue was purged.

Currently, in the following methods (put,get,join andtask_done), there is no specific behavior depending on the type of the shutdown. It's not even noted in an attribute.

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

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 forbadeput operations, also released all threads blocked input. On the other hand, it leftget,join andtask_done operations possible.

The immediate shutdown prohibited all possible previous operations and had to release all threads blocked inget, andjoin. And the queue was purged.

Currently, in the following methods (put,get,join andtask_done), there is no specific behavior depending on the type of the shutdown. It's not even noted in an attribute.

@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 thatq.task_done() andq.join() are documented as raisingShutDown when they don't; see#115838 for the fix for that.

@EpicWink , thank you for this explanation. I ve missed that :-(
Features really change from the initial version. Now it seems more efficient and simpler.
As we wrote all tests for the initial version, I 'd like to review this part because features are changed. May be we will have to remove some of tests, to modify others, or add new ones ?
Do you agree ?

FYI, I fixed thefailling test in_read_msg_thread() method. Loop was infinite, I insert abreak onqueue.Shutdow exception. .... (and I removed alltime.sleep())

FYI, I fixed thefailling test in_read_msg_thread() method. Loop was infinite, I insert abreak onqueue.Shutdow exception. .... (and I removed alltime.sleep())

Where can we see this?

FYI, I fixed thefailling test in_read_msg_thread() method. Loop was infinite, I insert abreak onqueue.Shutdow exception. .... (and I removed alltime.sleep())

Where can we see this?

https://github.com/YvesDup/cpython/blob/Threading-queue-shutdown-test-failed/Lib/test/test_queue.py#L350
Debug logsr andw will be removed.
See nowhttps://github.com/YvesDup/cpython/blob/Threading-queue-shutdown-test-failed/Lib/test/test_queue.py#L370

Thanks, I'll wait for the pull request.

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.

@YvesDup, I am a little confused. There is indeed#115940, by you, but there is alsogh-115898 by@EpicWink that also claims to fix these tests. How do the two relate to each other?

How do the two relate to each other?

There are related because there are both about on the same issue. But#115898 is only a draft.
I had informed@EpicWink that I had fixed the bug on the test, which is why I had submitted a new PR.
Maybe, should we wait for his feedback before going head ?

@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!

'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.