This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: asyncio.Semaphore waiters deque doesn't work
Type: Stage: patch review
Components: asyncio Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: asvetlov Nosy List: asvetlov, hyzyla, miss-islington, python-dev, yselivanov
Priority: normal Keywords: patch

Created on 2021-12-06 14:04 by hyzyla, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 30052 closed python-dev, 2021-12-11 14:42
PR 31910 merged asvetlov, 2022-03-15 16:34
PR 32047 merged miss-islington, 2022-03-22 14:03
PR 32049 merged asvetlov, 2022-03-22 14:20
Messages (9)
msg407809 - (view) Author: Yevhenii Hyzyla (hyzyla) * Date: 2021-12-06 14:04
Class `asyncio.Semaphore` has dequeue in implementation for waiters of semaphore and seems like the intention of this dequeue is to maintain FIFO queue of waiters. But it doesn't work, coroutine that releases semaphore can acquire semaphore again. Below is reproducible example:

```python
import asyncio


async def process(idx: int, semaphore: asyncio.Semaphore) -> None:
    while True:
        async with semaphore:
            print(f'ACQUIRE {idx}')
            await asyncio.sleep(1)


async def main() -> None:
    semaphore = asyncio.Semaphore(5)
    await asyncio.gather(*[process(idx, semaphore) for idx in range(20)])

asyncio.run(main())
```

In console:
```
/home/y.hyzyla/Work/asynciosemaphorebug/venv/bin/python /home/y.hyzyla/Work/asynciosemaphorebug/main.py
ACQUIRE 0
ACQUIRE 1
ACQUIRE 2
ACQUIRE 3
ACQUIRE 4
ACQUIRE 0
ACQUIRE 1
ACQUIRE 2
ACQUIRE 3
ACQUIRE 4
ACQUIRE 0
ACQUIRE 1
ACQUIRE 2
ACQUIRE 3
ACQUIRE 4
....
```

Ugly fix, is to add asyncio.sleep right before semaphore.
```python
    while True:
        await asyncio.sleep(0)
        async with semaphore:
            ...
```

Also, I found a comment on Stack Overflow about race condition in Semaphore implementation, but I don't know about the quality of that comment:

https://stackoverflow.com/questions/55951233/does-pythons-asyncio-lock-acquire-maintain-order#comment112978366_55951304
msg407836 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2021-12-06 16:43
Good point.
Currently, asyncio lock objects don't provide a strong FIFO guarantee.

In a tight loop, a task can re-acquire the lock just after releasing even if there are pending waiters that were scheduled earlier. It's true also for Lock, Conditional, Event, etc.

The solution requires *async* release method. Since the change is not backward compatible, a new method should be added, e.g. `await sem.release_and_wait()` for endorsing the context switch if there are pending waiters.

async context manager can be modified for using the new method without backward compatibility problems easily.

A hero who can help is welcome!
msg407841 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2021-12-06 16:57
Or, maybe, there is a way to do everything without changing public API.

The idea is: _wake_up_next can create a future which is set by *waked up task* on its acquiring. acquire method should wait for this future first before entering in `while self._value < 0:` loop.

If the future is cancelled, `_wake_up_next` should be called again and waiting for the next future acquiring to be performed.

If there is no *acquire waiting* future exists -- do everything as usual.

All other lock objects should be modified also.
msg407843 - (view) Author: Yevhenii Hyzyla (hyzyla) * Date: 2021-12-06 17:14
Thanks for response!

> A hero who can help is welcome!

I will try to make PR :raising_hand
msg415276 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-03-15 19:10
Andrew, the same problem exists in asyncio.Queue which is also critical. Here's how I fixed it in edgedb code base: https://github.com/edgedb/edgedb/blob/08e41341024828df22a01cd690b11fcff00bca5e/edb/server/compiler_pool/queue.py#L51-L74
msg415286 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2022-03-15 19:45
Thanks, Yuri. 
I'll take a look.
msg415772 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2022-03-22 14:03
New changeset 32e77154ddfc514a3144d5912bffdd957246fd6c by Andrew Svetlov in branch 'main':
bpo-45997: Fix asyncio.Semaphore re-acquiring order (GH-31910)
https://github.com/python/cpython/commit/32e77154ddfc514a3144d5912bffdd957246fd6c
msg415786 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2022-03-22 15:16
New changeset 9d59381a5d20157930bae34e5f5a7bc5ef09fa89 by Miss Islington (bot) in branch '3.10':
[3.10] bpo-45997: Fix asyncio.Semaphore re-acquiring order (GH-31910) (#32047)
https://github.com/python/cpython/commit/9d59381a5d20157930bae34e5f5a7bc5ef09fa89
msg415787 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2022-03-22 15:16
New changeset f47984b560f1dafe4d907cef4edadfb1746bf027 by Andrew Svetlov in branch '3.9':
[3.9] bpo-45997: Fix asyncio.Semaphore re-acquiring order (GH-31910) (GH-32049)
https://github.com/python/cpython/commit/f47984b560f1dafe4d907cef4edadfb1746bf027
History
Date User Action Args
2022-04-11 14:59:53adminsetgithub: 90155
2022-03-22 15:16:39asvetlovsetmessages: + msg415787
2022-03-22 15:16:00asvetlovsetmessages: + msg415786
2022-03-22 14:20:22asvetlovsetpull_requests: + pull_request30138
2022-03-22 14:03:20miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request30137
2022-03-22 14:03:00asvetlovsetmessages: + msg415772
2022-03-15 19:45:14asvetlovsetmessages: + msg415286
2022-03-15 19:10:19yselivanovsetmessages: + msg415276
2022-03-15 16:34:42asvetlovsetpull_requests: + pull_request30003
2021-12-11 16:12:52hyzylasetversions: + Python 3.11, - Python 3.8
2021-12-11 14:42:18python-devsetkeywords: + patch
nosy: + python-dev

pull_requests: + pull_request28277
stage: patch review
2021-12-06 17:14:55hyzylasettype: behavior ->
messages: + msg407843
versions: + Python 3.8, - Python 3.11
2021-12-06 16:57:06asvetlovsettype: behavior
title: asyncio.Semaphore waiters deqeueu doesn't work -> asyncio.Semaphore waiters deque doesn't work
messages: + msg407841
versions: + Python 3.11, - Python 3.8
2021-12-06 16:43:29asvetlovsetassignee: asvetlov
type: behavior -> (no value)
messages: + msg407836
title: asyncio.Semaphore waiters deque doesn't work -> asyncio.Semaphore waiters deqeueu doesn't work
2021-12-06 15:31:52AlexWaygoodsettype: behavior
2021-12-06 15:31:35AlexWaygoodsettitle: asyncio.Semaphore waiters deqeueu doesn't work -> asyncio.Semaphore waiters deque doesn't work
2021-12-06 14:04:23hyzylacreate