The Wayback Machine - https://web.archive.org/web/20200905043639/https://github.com/python/asyncio/pull/467
Skip to content
This repository has been archived by the owner. It is now read-only.

Cancelled lock waiter wakes up the next one if any #467

Open
wants to merge 2 commits into
base: master
from

Conversation

@msornay
Copy link

msornay commented Nov 16, 2016

Fixes http://bugs.python.org/issue27585

Without the fix the unit test deadlock, I haven't managed to make it fail (using wait_for() seems to avoid the deadlock)

@1st1
Copy link
Member

1st1 commented Nov 16, 2016

@gvanrossum @asvetlov I'm not sure I can review this (I never touched/used that code). Can you take a look if you have time and you are familiar with the code?

@gvanrossum
Copy link
Member

gvanrossum commented Nov 16, 2016

@@ -71,6 +71,31 @@ def test_context_manager_with_await(self):
self.loop.run_until_complete(test(primitive))
self.assertFalse(primitive.locked())

def test_finished_waiter_cancelled(self):

async def create_waiter(lock, fut):

This comment has been minimized.

@1st1

1st1 Nov 17, 2016

Member

Please use the @coroutine decorator and generators in unittests. We'll start using 'async def' when 3.6 is out and we start working on Python 3.7.

This comment has been minimized.

@msornay

msornay Nov 17, 2016

Author

Even in test_pep492.py ? Should I move the test ?

This comment has been minimized.

@1st1

1st1 Nov 17, 2016

Member

test_pep492 is ignored on Pythons < 3.5 and it tests specifically async/await and asyncio.

Yeah, this test should be where all tests of lock/synchronization primitives are.

@1st1
Copy link
Member

1st1 commented Nov 17, 2016

Looks like this PR is doing a right thing. @msornay please update the tests, and I will merge it.

Python issue #27585
@the-knights-who-say-ni
Copy link

the-knights-who-say-ni commented Nov 17, 2016

Hello, and thanks for your contribution!

Unfortunately we couldn't find an account corresponding to your GitHub username at bugs.python.org (b.p.o). If you don't already have an account at b.p.o, please create one and make sure to add your GitHub username. If you do already have an account at b.p.o then please go there and under "Your Details" add your GitHub username.

And in case you haven't already, please make sure to sign the PSF contributor agreement (CLA); we can't legally look at your contribution until you have signed the CLA.

Once you have done everything that's needed, please reply here and someone will verify everything is in order.

@msornay
Copy link
Author

msornay commented Nov 18, 2016

Everything should be in order.

@pklanke
Copy link

pklanke commented Jan 9, 2017

After applying patch #363 and #467 to Python 3.5.2, the dead-lock did indeed no longer occur, but the opposite did; Sometimes on releasing the lock, a RuntimeError exception is raised telling me the lock was not acquired.

lock.acquire()
try:
... do stuff ...
finally:
lock.release() # RuntimeError occured here

So far, I did not manage to create a simple test case, and I know this does not help much without it, but should someone experience this same behavior, then just know you are not the only one.

@1st1
Copy link
Member

1st1 commented Mar 2, 2017

Please reopen this PR at https://github.com/python/cpython. This repo is going to be deleted soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.