| msg274547 - (view) |
Author: Max von Tettenborn (Max von Tettenborn) |
Date: 2016-09-06 10:16 |
| Below code reproduces the problem. The resulting error is a RecursionError and it is very hard to trace that to the cause of the problem, which is the runner task and the stop task yielding from each other, forming a deadlock. I think, an easy to make mistake like that should raise a clearer exception. And maybe I am mistaken, but it should in principle be possible for the event loop to detect a cyclic yield, right? import asyncio class A: @asyncio.coroutine def start(self): self.runner_task = asyncio.ensure_future(self.runner()) @asyncio.coroutine def stop(self): self.runner_task.cancel() yield from self.runner_task @asyncio.coroutine def runner(self): try: while True: yield from asyncio.sleep(5) except asyncio.CancelledError: yield from self.stop() return def do_test(): @asyncio.coroutine def f(): a = A() yield from a.start() yield from asyncio.sleep(1) yield from a.stop() asyncio.get_event_loop().run_until_complete(f()) |
|
|
| msg274697 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2016-09-07 01:35 |
| Thanks for the report. I've never seen this before, so I doubt it is a common mistake. Yury have you ever seen this? --Guido (mobile) |
|
|
| msg278203 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-06 18:50 |
| This is an interesting mind twister. The key problem is that `self.runner_task` is blocked on *itself*: so Task._fut_waiter is set to the Task. Therefore when the task is being cancelled, `Task.cancel` simply recurses. One way to solve this is to prohibit tasks to await on themselves. Right now the following code "kind of" works: async def f(): loop.call_later(1, lambda: t.set_result(42)) return await t loop = asyncio.get_event_loop() t = loop.create_task(f()) print(loop.run_until_complete(t)) albeit it logs errors about invalid Future state. My proposal is to raise a ValueError if Task is asked to await on itself. Guido, what do you think? |
|
|
| msg278215 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-10-06 21:47 |
| It's pretty perverse. But how would you detect this case? Does it require changes to CPython or only to asyncio? Does it require a spec change anywhere? |
|
|
| msg278216 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-06 21:50 |
| > It's pretty perverse. But how would you detect this case? In Task._step, we can check if the future the task is about to await on is "self". |
|
|
| msg278217 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-10-06 21:53 |
| Is that enough? What if the recursion involves several tasks waiting for each other in a cycle? |
|
|
| msg278218 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-06 21:58 |
| > Is that enough? What if the recursion involves several tasks waiting for each other in a cycle? I'm not sure... Maybe it's OK when two tasks await on each other, I think the current Task implementation should be able to handle that. The problem with the Task awaiting itself is that the current implementation just crashes with a RecursionError. |
|
|
| msg278219 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-10-06 22:13 |
| Maybe it could be fixed rather than making this a checked failure? |
|
|
| msg278246 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-10-07 14:35 |
| I've meditated on this and I've changed my mind. A task that awaits itself is so obviously not following the task protocol that it should be shot on sight. No exceptions. (Only the task itself should ever set the result or exception, and *only* by returning or raising. So the only way a task waiting for itself could be unblocked is through cancellation, and there are better ways to wait forever until cancelled.) IOW @1st1 I think it's okay to detect this and raise an exception. |
|
|
| msg278371 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-10-09 16:21 |
| New changeset 8d877876aa89 by Yury Selivanov in branch '3.5': Issue #27972: Prohibit Tasks to await on themselves. https://hg.python.org/cpython/rev/8d877876aa89 New changeset 41c4f535b5c0 by Yury Selivanov in branch '3.6': Merge 3.5 (issue #27972) https://hg.python.org/cpython/rev/41c4f535b5c0 New changeset 47720192b318 by Yury Selivanov in branch 'default': Merge 3.6 (issue #27972) https://hg.python.org/cpython/rev/47720192b318 |
|
|
| msg278372 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-09 16:22 |
| Thank you for reporting this! Closing the issue. |
|
|
| msg278407 - (view) |
Author: Max von Tettenborn (Max von Tettenborn) |
Date: 2016-10-10 09:11 |
| You are very welcome, glad I could help. |
|
|