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.

Sign up for GitHub

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

Jump to bottom

asyncio.Lock deadlock after cancellation #71772

Closed
sss mannequin opened this issue Jul 21, 2016 · 15 comments
Closed

asyncio.Lock deadlock after cancellation #71772

sss mannequin opened this issue Jul 21, 2016 · 15 comments
Labels
3.7 (EOL) end of life topic-asyncio

Comments

@sss
Copy link
Mannequin

sss mannequin commented Jul 21, 2016

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
    Copy link
    Mannequin Author

    sss mannequin commented Jul 21, 2016

    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.

    @sss sss mannequin added the topic-asyncio label Jul 21, 2016
    @bitdancer
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    sss mannequin commented Aug 4, 2016

    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
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    sss mannequin commented Aug 4, 2016

    Yes I'm running python-3.5.2.

    @gvanrossum
    Copy link
    Member

    Tentative fix is in python/asyncio#393

    @sss
    Copy link
    Mannequin Author

    sss mannequin commented Aug 4, 2016

    Thank you. This looks good to me

    @gvanrossum
    Copy link
    Member

    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
    Copy link
    Mannequin

    msornay mannequin commented Nov 15, 2016

    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
    Copy link
    Member

    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
    Copy link
    Mannequin

    msornay mannequin commented Nov 16, 2016

    Fix attempt : python/asyncio#467

    @AlexeyPopravka
    Copy link
    Mannequin

    AlexeyPopravka mannequin commented May 18, 2017

    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
    Copy link
    Mannequin

    AlexeyPopravka mannequin commented May 18, 2017

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

    @gvanrossum
    Copy link
    Member

    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
    Copy link
    Member

    1st1 commented Jun 9, 2017

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

    @1st1 1st1 added the 3.7 (EOL) end of life label Jun 9, 2017
    @1st1 1st1 closed this as completed Jun 9, 2017
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life topic-asyncio
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @1st1 @bitdancer @gvanrossum