bpo-42130: Fix swallowing of cancellation by wait_for by ods · Pull Request #26097 · python/cpython · GitHub
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions Lib/asyncio/tasks.py
33 changes: 33 additions & 0 deletions Lib/test/test_asyncio/test_asyncio_waitfor.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ async def run(self):

self.exited = True


class AsyncioWaitForTest(unittest.TestCase):

async def atest_asyncio_wait_for_cancelled(self):
Expand Down Expand Up @@ -56,6 +57,38 @@ async def atest_asyncio_wait_for_timeout(self):
def test_asyncio_wait_for_timeout(self):
asyncio.run(self.atest_asyncio_wait_for_timeout())

async def atest_asyncio_wait_for_hang(self, inner_steps, outer_steps):
# bpo-42130: wait_for can swallow cancellation causing task to hang
async def inner(steps):
for _ in range(steps):
await asyncio.sleep(0)

finished = False

async def wait_for_coro(steps):
nonlocal finished
await asyncio.wait_for(inner(steps), timeout=1)
await asyncio.sleep(0.1)
finished = True

task = asyncio.create_task(wait_for_coro(inner_steps))
for _ in range(outer_steps):
await asyncio.sleep(0)
assert not task.done()

task.cancel()
with self.assertRaises(asyncio.CancelledError):
await task
self.assertFalse(finished)

def test_asyncio_wait_for_hang(self):
# Test with different number of inner/outer steps to weaken dependence
# on implementation details
for inner_steps in range(3):
for outer_steps in range(3):
with self.subTest(inner_steps=inner_steps, outer_steps=outer_steps):
asyncio.run(self.atest_asyncio_wait_for_hang(inner_steps, outer_steps))


if __name__ == '__main__':
unittest.main()
16 changes: 0 additions & 16 deletions Lib/test/test_asyncio/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1147,22 +1147,6 @@ def gen():
res = loop.run_until_complete(task)
self.assertEqual(res, "ok")

def test_wait_for_cancellation_race_condition(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this test now fail if it wasn't removed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. The test is incorrect (the task must raise) and it doesn't reproduce original problem.

def gen():
yield 0.1
yield 0.1
yield 0.1
yield 0.1

loop = self.new_test_loop(gen)

fut = self.new_future(loop)
loop.call_later(0.1, fut.set_result, "ok")
task = loop.create_task(asyncio.wait_for(fut, timeout=1))
loop.call_later(0.1, task.cancel)
res = loop.run_until_complete(task)
self.assertEqual(res, "ok")

def test_wait_for_waits_for_task_cancellation(self):
loop = asyncio.new_event_loop()
self.addCleanup(loop.close)
Expand Down