classification
Title: asyncio.Lock deadlock after cancellation
Type: Stage: resolved
Components: asyncio Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Alexey Popravka, haypo, msornay, r.david.murray, sss, yselivanov
Priority: normal Keywords:

Created on 2016-07-21 17:06 by sss, last changed 2017-06-09 21:09 by yselivanov. This issue is now closed.

Files
File name Uploaded Description Edit
lock.py sss, 2016-07-21 17:06
lock2.py sss, 2016-08-04 09:33 reproducer
deadlock.py msornay, 2016-11-15 18:26
deadlock2.py Alexey Popravka, 2017-05-18 12:47
Pull Requests
URL Status Linked Edit
PR 1031 merged msornay, 2017-04-07 13:47
PR 2036 merged yselivanov, 2017-06-09 20:23
PR 2037 merged yselivanov, 2017-06-09 20:39
PR 2038 merged yselivanov, 2017-06-09 20:40
Messages (15)
msg270944 - (view) Author: (sss) Date: 2016-07-21 17:06
The lock.py file prints

DEADLOCK HERE
_locked:  False
_waiters: deque([<Future cancelled>])

I think the problem is that acquire() will block because of the cancelled future in _waiters, but release() has already ran so no one will wake it up.
msg270947 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-07-21 17:26
If you give the loop a chance to run (an await asyncio.sleep(0)) before doing the acquire) it does not deadlock.  I don't understand enough of the asyncio internals to know if this is expected or a bug.
msg271962 - (view) Author: (sss) Date: 2016-08-04 09:33
Yes, await asyncio.sleep(0) does fix it, but it would be weird if I'm supposed to put those all over my code. I made a new reproducer now that uses 2 threads in case it's not allowed to take the same lock twice from the same thread.
msg271986 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-04 15:06
Does this repro with the latest Python 3.5.2? IIRC the lock code was
overhauled after 3.5.0 or 3.5.1.

(PS. These are tasks, not threads.)

--Guido (mobile)

On Aug 4, 2016 2:33 AM, "sss" <report@bugs.python.org> wrote:

sss added the comment:

Yes, await asyncio.sleep(0) does fix it, but it would be weird if I'm
supposed to put those all over my code. I made a new reproducer now that
uses 2 threads in case it's not allowed to take the same lock twice from
the same thread.

----------
Added file: http://bugs.python.org/file44007/lock2.py

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue27585>
_______________________________________
msg271987 - (view) Author: (sss) Date: 2016-08-04 15:09
Yes I'm running python-3.5.2.
msg271990 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-04 16:13
Tentative fix is in https://github.com/python/asyncio/pull/393
msg271999 - (view) Author: (sss) Date: 2016-08-04 20:28
Thank you. This looks good to me
msg272042 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-05 16:55
OK, merged upstream. It'll eventually come down to Python 3.5.3 and 3.6.

On Thu, Aug 4, 2016 at 1:28 PM, sss <report@bugs.python.org> wrote:
>
> sss added the comment:
>
> Thank you. This looks good to me
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue27585>
> _______________________________________
msg280873 - (view) Author: Mathieu Sornay (msornay) * Date: 2016-11-15 18:26
I might have found another pathological case not fixed by https://github.com/python/asyncio/pull/393

Tested in 3.6.0b3

The deadlock.py file prints :

DEADLOCK HERE
_locked:  False
_waiters: deque([<Future finished result=True>])
msg280874 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-11-15 18:53
Looks like create_waiter() entered the with-statement but didn't leave it
properly due to the cancellation, right? @1st1 any idea? async with bug or
asyncio bug?
msg280938 - (view) Author: Mathieu Sornay (msornay) * Date: 2016-11-16 12:08
Fix attempt : https://github.com/python/asyncio/pull/467
msg293922 - (view) Author: Alexey Popravka (Alexey Popravka) Date: 2017-05-18 12:47
Hi guys! Any update on this?
I've just hit this issue.
Cancelled futures in _waiters is not a problem (any more).

There is still a problem when release() wakes up next waiter
but task waiting for it gets cancelled and so all the rest waiters
are in pending state forever.
I've attach one more test to reproduce this issue.
msg293925 - (view) Author: Alexey Popravka (Alexey Popravka) Date: 2017-05-18 15:03
Also there is the same problem with asyncio.Condition wait() / notify() couple as it repeats the same Lock.acquire()/release() logic
msg293929 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-05-18 17:14
I'm really sorry, but I personally don't have time to look into this. If you have a fix in mind you can send a PR to GitHub mentioning bpi-27585.
msg295568 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2017-06-09 20:34
New changeset 7be651c7aad8e4d46012205811b58ef127b08e0e by Yury Selivanov in branch 'master':
bpo-27585: Add a NEWS entry for #1031 (#2036)
https://github.com/python/cpython/commit/7be651c7aad8e4d46012205811b58ef127b08e0e
History
Date User Action Args
2017-06-09 21:09:58yselivanovsetstatus: open -> closed
stage: resolved
resolution: fixed
versions: + Python 3.5, Python 3.7
2017-06-09 20:43:48gvanrossumsetnosy: - gvanrossum
2017-06-09 20:40:37yselivanovsetpull_requests: + pull_request2103
2017-06-09 20:39:00yselivanovsetpull_requests: + pull_request2102
2017-06-09 20:34:31yselivanovsetmessages: + msg295568
2017-06-09 20:23:45yselivanovsetpull_requests: + pull_request2101
2017-05-18 17:14:53gvanrossumsetmessages: + msg293929
2017-05-18 15:03:15Alexey Popravkasetmessages: + msg293925
2017-05-18 12:47:14Alexey Popravkasetfiles: + deadlock2.py
nosy: + Alexey Popravka
messages: + msg293922

2017-04-07 13:47:37msornaysetpull_requests: + pull_request1188
2016-11-16 12:08:55msornaysetmessages: + msg280938
2016-11-15 18:53:08gvanrossumsetmessages: + msg280874
2016-11-15 18:26:06msornaysetfiles: + deadlock.py
versions: + Python 3.6, - Python 3.5
nosy: + msornay

messages: + msg280873
2016-08-05 16:55:11gvanrossumsetmessages: + msg272042
2016-08-04 20:28:31ssssetmessages: + msg271999
2016-08-04 16:13:33gvanrossumsetmessages: + msg271990
2016-08-04 15:09:29ssssetmessages: + msg271987
2016-08-04 15:06:30gvanrossumsetmessages: + msg271986
2016-08-04 09:33:34ssssetfiles: + lock2.py

messages: + msg271962
2016-07-21 17:26:37r.david.murraysetnosy: + r.david.murray
messages: + msg270947
2016-07-21 17:06:41ssscreate