Repository navigation
Let Queue#pop and SizedQueue#push wait through the fiber scheduler - #9805
Open
sampokuokkanen wants to merge 3 commits into
Open
sampokuokkanen wants to merge 3 commits into
sampokuokkanen wants to merge 3 commits into
Conversation
The timed poll loop only checked the item count, so the signal from closeInternal woke the waiter, it found the queue still empty, and went back to awaitNanos until the full timeout passed. A closed, empty queue now returns nil at once and passes the wakeup on to the next consumer, as takeInternal already does for the untimed pop.
Queue#pop on an empty queue parked the whole thread. Under a fiber scheduler that meant the fiber that would push into the queue never got to run. io-event and async hand results between fibers this way. Fibers now block through the scheduler, as MRI's queue_sleep does. Like MRI, a push wakes one waiting fiber and close wakes them all. A woken fiber checks the queue again and blocks again if someone else got the item first. num_waiting counts these fibers too. SizedQueue#push still blocks the thread; that comes next.
SizedQueue#push on a full queue parked the whole thread, so under a fiber scheduler the fiber that would pop never got to run. Pushing fibers now block through the scheduler the same way Queue#pop does. A pop wakes one of them; clear, close and max= wake them all, as in MRI. num_waiting counts them. If the queue is closed while a fiber waits to push, it raises ClosedQueueError. Without that check the fiber would retry forever, because a full queue is reported before a closed one.
Member
|
Thank you! I'll give this a review soon (traveling until Saturday). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Under a fiber scheduler,
Queue#popon an empty queue andSizedQueue#pushon a full one blocked the whole thread. The fiber that would fill or drain the queue never got to run, so the thread hung for good.Both now block through the scheduler, the way MRI's
queue_sleepdoes. Without a scheduler, the extra cost is one read of a null field. The waiter list is only created on first use, like the Mutex code in #9790.This branch includes #9791's commit as I'm touching the same file and rows here and it has already been approved.
These async and io-event tests used to hang, and now they pass:
test/async/queue.rb: 29/29test/async/limited_queue.rb: 31/31test/thread/queue.rb: 1/1test/tcp_socket.rb: finishesThe code at some places feels a bit repetive, but it's repetitive also on MRI, but might be worth a refactor once we get more tests passing.