asyncio.Lock deadlock after cancellation

[sss]

BPO	27585
Nosy	@vstinner, @bitdancer, @1st1, @msornay
PRs	bpo-27585: Cancelled lock waiter wakes up the next one #1031 bpo-27585: Add a NEWS entry for #1031 #2036 [3.6] bpo-27585: Fix waiter cancellation in asyncio.Lock #2037 [3.5] bpo-27585: Fix waiter cancellation in asyncio.Lock #2038
Files	lock.py lock2.py: reproducer deadlock.py deadlock2.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-06-09.21:09:58.599>
created_at = <Date 2016-07-21.17:06:41.344>
labels = ['3.7', 'expert-asyncio']
title = 'asyncio.Lock deadlock after cancellation'
updated_at = <Date 2017-06-09.21:09:58.597>
user = 'https://bugs.python.org/sss'

bugs.python.org fields:

activity = <Date 2017-06-09.21:09:58.597>
actor = 'yselivanov'
assignee = 'none'
closed = True
closed_date = <Date 2017-06-09.21:09:58.599>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2016-07-21.17:06:41.344>
creator = 'sss'
dependencies = []
files = ['43820', '44007', '45489', '46876']
hgrepos = []
issue_num = 27585
keywords = []
message_count = 15.0
messages = ['270944', '270947', '271962', '271986', '271987', '271990', '271999', '272042', '280873', '280874', '280938', '293922', '293925', '293929', '295568']
nosy_count = 6.0
nosy_names = ['vstinner', 'r.david.murray', 'yselivanov', 'msornay', 'sss', 'Alexey Popravka']
pr_nums = ['1031', '2036', '2037', '2038']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue27585'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[sss]

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.

[bitdancer]

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.

[sss]

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.

[gvanrossum]

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\>

---

[sss]

Yes I'm running python-3.5.2.

[gvanrossum]

Tentative fix is in python/asyncio#393

[sss]

Thank you. This looks good to me

[gvanrossum]

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\>

---

[msornay]

I might have found another pathological case not fixed by python/asyncio#393

Tested in 3.6.0b3

The deadlock.py file prints :

DEADLOCK HERE
_locked: False
_waiters: deque([<Future finished result=True>])

[gvanrossum]

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?

[msornay]

Fix attempt : python/asyncio#467

[AlexeyPopravka]

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.

[AlexeyPopravka]

Also there is the same problem with asyncio.Condition wait() / notify() couple as it repeats the same Lock.acquire()/release() logic

[gvanrossum]

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.

[1st1]

New changeset 7be651c by Yury Selivanov in branch 'master':
bpo-27585: Add a NEWS entry for bpo-1031 (bpo-2036)
7be651c