Skip to content

Commit 74ee527

Browse files
committed
Actually wait for tasks when cancelling
`await asyncio.sleep(0)` is almost always a bad idea, as we have the ability to actually await completion of something (or multiple things) instead of assuming timeouts. I would have used `asyncio.gather` if the `get_result` parameter didn't exist. I also inlined `_cancel` function that is only used by `cancel` and not descriptive of what it does.
1 parent f3e8e96 commit 74ee527

File tree

1 file changed

+14
-19
lines changed

1 file changed

+14
-19
lines changed

asyncio_pool/base_pool.py

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,19 @@ async def itermap(self, fn, iterable, cb=None, ctx=None, *, flat=True,
248248
'''
249249
raise NotImplementedError('Use one of mixins')
250250

251-
def _cancel(self, *futures):
251+
async def cancel(self, *futures, get_result=getres.flat):
252+
'''Cancels spawned or waiting tasks, found by their `futures`. If no
253+
`futures` are passed -- cancels all spwaned and waiting tasks.
254+
255+
Cancelling futures, returned by pool methods, usually won't help you
256+
to cancel executing tasks, so you have to use this method.
257+
258+
Returns tuple of (`cancelled` count of cancelled tasks, `results`
259+
collected from futures of cancelled tasks).
260+
'''
252261
tasks, _futures = [], []
253262

254-
if not len(futures): # meaning cancel all
263+
if not futures: # meaning cancel all
255264
tasks.extend(self._waiting.values())
256265
tasks.extend(self._active.values())
257266
_futures.extend(self._waiting.keys())
@@ -263,21 +272,7 @@ def _cancel(self, *futures):
263272
tasks.append(task)
264273
_futures.append(fut)
265274

266-
cancelled = sum([1 for fut in tasks if fut.cancel()])
267-
return cancelled, _futures
268-
269-
async def cancel(self, *futures, get_result=getres.flat):
270-
'''Cancels spawned or waiting tasks, found by their `futures`. If no
271-
`futures` are passed -- cancels all spwaned and waiting tasks.
272-
273-
Cancelling futures, returned by pool methods, usually won't help you
274-
to cancel executing tasks, so you have to use this method.
275-
276-
Returns tuple of (`cancelled` count of cancelled tasks, `results`
277-
collected from futures of cancelled tasks).
278-
'''
279-
cancelled, _futures = self._cancel(*futures)
280-
await aio.sleep(0) # let them actually cancel
275+
cancelled = sum(1 for task in tasks if task.cancel())
276+
await aio.wait(tasks) # let them actually cancel
281277
# need to collect them anyway, to supress warnings
282-
results = [get_result(fut) for fut in _futures]
283-
return cancelled, results
278+
return cancelled, [get_result(fut) for fut in _futures]

0 commit comments

Comments
 (0)