Asyncio.Condition prevents cancellation

[bharel]

BPO	32841
Nosy	@njsmith, @asvetlov, @1st1, @bharel
PRs	bpo-32841: Asyncio.Condition prevents cancellation #5665 [3.7] bpo-32841: Fix cancellation in awaiting asyncio.Condition (GH-5665) #5682 [3.6] bpo-32841: Fix cancellation in awaiting asyncio.Condition (GH-5665) #5683
Files	le_bug.py le_patch.diff

^{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 2018-05-25.20:17:45.261>
created_at = <Date 2018-02-13.21:30:02.123>
labels = ['3.8', 'type-bug', '3.7', 'expert-asyncio']
title = 'Asyncio.Condition prevents cancellation'
updated_at = <Date 2018-05-25.20:17:45.260>
user = 'https://github.com/bharel'

bugs.python.org fields:

activity = <Date 2018-05-25.20:17:45.260>
actor = 'bar.harel'
assignee = 'none'
closed = True
closed_date = <Date 2018-05-25.20:17:45.261>
closer = 'bar.harel'
components = ['asyncio']
creation = <Date 2018-02-13.21:30:02.123>
creator = 'bar.harel'
dependencies = []
files = ['47439', '47440']
hgrepos = []
issue_num = 32841
keywords = ['patch']
message_count = 13.0
messages = ['312140', '312141', '312143', '312157', '312163', '312164', '312165', '312166', '312167', '312168', '312169', '317702', '317715']
nosy_count = 4.0
nosy_names = ['njs', 'asvetlov', 'yselivanov', 'bar.harel']
pr_nums = ['5665', '5682', '5683']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32841'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[bharel]

Hey guys,

A week after a serious asyncio.Lock bug, I found another bug that makes asyncio.Condition ignore and prevent cancellation in some cases due to an "except: pass" which tbh is a little embarrassing.

What happens is that during the time it takes to get back a conditional lock after notifying it, asyncio completely ignores all cancellations sent to the waiting task.

le_bug.py: Contains the bug
le_patch.diff: Contains a very simple fix (will send a pull on Github too)
le_meme.jpg: Contains my face after debugging this for 4 hours

Yuri, I hope you didn't miss me during this week ;-)

-- Bar

[1st1]

le_meme.jpg is a pretty common facial expression when one debugs locks/conditions, so let's not attach it in the future :) We try to keep the bug tracker free of unnecessary information.

As for the bug—please submit a PR!

[njsmith]

Interesting case! This is super subtle. I think the patch is correct though.

(In Trio this is one of the cases where we use shielding: https://github.com/python-trio/trio/blob/f48d8922dfe2118bfaaf9a85f2524e58146ba369/trio/_sync.py#L749-L752 . But asyncio.shield has different semantics -- it still delivers the CancelledError to the caller, which is no good in this case.)

[njsmith]

It does make me wonder if asyncio.shield *should* wait for the thing it's shielding though, so that it *would* work in this case? (Similar to bpo-32751.)

[bharel]

I don't think so. Having shield not cancel immediately but rather wait and
cancel will cause long timed shielded operations to stall the task
cancellation, usually for no good. This isn't the general case.
However, adding another function which does so might just be a good idea. I
think another parameter to shield to choose cancellation time will clutter
the function call.

On Wed, Feb 14, 2018, 7:28 AM Nathaniel Smith <report@bugs.python.org>
wrote:

Nathaniel Smith <njs@pobox.com> added the comment:

It does make me wonder if asyncio.shield *should* wait for the thing it's
shielding though, so that it *would* work in this case? (Similar to
bpo-32751.)

----------

---

Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue32841\>

---

[asvetlov]

New changeset 5746510 by Andrew Svetlov (Bar Harel) in branch 'master':
bpo-32841: Fix cancellation in awaiting asyncio.Condition (bpo-5665)
5746510

[miss-islington]

New changeset 8caee0f by Miss Islington (bot) in branch '3.7':
bpo-32841: Fix cancellation in awaiting asyncio.Condition (GH-5665)
8caee0f

[asvetlov]

New changeset a23eeca by Andrew Svetlov (Miss Islington (bot)) in branch '3.6':
bpo-32841: Fix cancellation in awaiting asyncio.Condition (GH-5665) (GH-5683)
a23eeca

[asvetlov]

Thanks Bar Harel

[njsmith]

Having shield not cancel immediately but rather wait and cancel will cause long timed shielded operations to stall the task cancellation, usually for no good. This isn't the general case.

What I'm suggesting is that maybe it actually is good in the general case :-). If an operation can't be cancelled, that's too bad, but it's generally still better to wait then to silently leak it into the background.

[asvetlov]

Well, makes sense

[1st1]

Should this issue be closed now?

[bharel]

Yup. Thanks Yuri :-)