X Tutup
The Wayback Machine - https://web.archive.org/web/20240106043313/https://github.com/python/cpython/pull/112501
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-83055: asyncio.Queue: putting items out of order when it is full #112501

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

YvesDup
Copy link
Contributor

@YvesDup YvesDup commented Nov 28, 2023

Comment

Currently, when the queue is full, new items are added to the queue._putters deque via an attached future. As soon as an item is removed from this queue, one future of the deque is set to done, then removed. This future is in transit, not referenced anywhere.

First I suggest removing the future from queue._putters only after it had been woken up, not before. All pending or moving item/task will stay into queue._putters
Second when put() is called, if the queue is full or if queue._putters is not empty, this item/task will be added to queue._putters.
When put_nowait() is called, if the queue is not full and queue._putters is not empty, we raise a new QueueFullWithPendingPutTasks exception derived from QueueFull.

The behavior of the get part should be identical.

Modified methods of Queue class :

  • put()
  • put_nowait()
  • get()
  • get_nowait()
  • _wakeup_next()

New private methods of Queue class:

  • _put_and_wakeup_next() called from put() and put_nowait().
  • _get_and_wakeup_next() called from get()and get_nowait().

New exceptions of queues.py:

  • QueueFullWithPendingPutTasks raised from put_nowait()
  • QueueEmptyWithPendingGetTasks raised from get_nowait()

Update:
These two exceptions are derived from QueueFull and QueueEmpy respectively. This is necessary to preserve the
backwards compatibility of put_nowait() and get_nowait() methods.
May be their names are too long or not explicit enough, so I am open to suggestions.

@YvesDup YvesDup changed the title gh-83055: asyncio.Queue: putting items out of order when it is fullinitial commit gh-83055: asyncio.Queue: putting items out of order when it is full Nov 29, 2023
@gvanrossum gvanrossum self-requested a review December 20, 2023 20:23
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this is not a real review. Given the esoteric use case and the subtlety of the changes (witnessed by the size of the tests!) I am not sure that I can accept this. The 1-2 comments below are just what I collected until I realized that this is going to take more effort to fully understand the consequences.

Lib/asyncio/queues.py Show resolved Hide resolved
Lib/asyncio/queues.py Show resolved Hide resolved
@YvesDup
Copy link
Contributor Author

YvesDup commented Dec 21, 2023

Hi, this is not a real review. Given the esoteric use case and the subtlety of the changes (witnessed by the size of the tests!) I am not sure that I can accept this. The 1-2 comments below are just what I collected until I realized that this is going to take more effort to fully understand the consequences.

I understand your point of view. This fix is not simple neither minor. Issue is old and there is not other related issue for a long time.

Fix WARNING: Bullet list ends without a blank line; unexpected unindent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants
X Tutup