Skip to content

Commit 50705cc

Browse files
committed
fix: Harden join_handle against swallowing caller cancellation
Rewrite join_handle on asyncio.wait instead of wait_for+shield, which reported completion via done/pending sets rather than raising. Caller cancellation now propagates cleanly and is never conflated with the joined task's own cancellation. Adds a regression test.
1 parent c0b9240 commit 50705cc

2 files changed

Lines changed: 41 additions & 13 deletions

File tree

ldclient/impl/aio/concurrency.py

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -119,20 +119,24 @@ def spawn_handle(name: str, fn: Callable) -> TaskHandle:
119119

120120
async def join_handle(handle: TaskHandle, timeout: float) -> None:
121121
"""Waits up to ``timeout`` seconds for a spawned task to finish, mirroring
122-
``Thread.join(timeout)``: the task's exception (if any) is not re-raised.
123-
On timeout the task is cancelled so it does not leak."""
124-
try:
125-
await asyncio.wait_for(asyncio.shield(handle), timeout)
126-
except asyncio.TimeoutError:
122+
``Thread.join(timeout)``: the task's result/exception is not re-raised, and
123+
on timeout the task is cancelled so it does not leak. If the *calling* task
124+
is cancelled while joining, that cancellation propagates and the joined task
125+
is left running (its lifecycle is owned elsewhere).
126+
127+
Uses ``asyncio.wait`` rather than ``wait_for``: it never cancels the joined
128+
task itself and never raises the task's result into us — completion is
129+
reported via the returned sets — so caller cancellation propagates without
130+
having to disambiguate it from the joined task's own cancellation."""
131+
done, _ = await asyncio.wait({handle}, timeout=timeout)
132+
if handle not in done:
133+
# Timed out — cancel so the task does not outlive the join.
127134
handle.cancel()
128-
except asyncio.CancelledError:
129-
# The awaited task was cancelled elsewhere; do not treat that as a
130-
# cancellation of the caller.
131-
if handle.cancelled():
132-
return
133-
raise
134-
except Exception:
135-
pass
135+
return
136+
if not handle.cancelled():
137+
# Consume any exception so it is neither re-raised nor reported as
138+
# "never retrieved" (background failures are logged by spawn_handle).
139+
handle.exception()
136140

137141

138142
class AsyncCallbackScheduler:

ldclient/testing/test_aio.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,30 @@ async def boom():
336336
handle = aio.spawn_handle("test.boom", boom)
337337
await aio.join_handle(handle, 2) # does not raise
338338

339+
@pytest.mark.asyncio
340+
async def test_async_join_propagates_caller_cancellation(self):
341+
# Cancelling the task that is *doing* the join must propagate the
342+
# cancellation, not swallow it, and must leave the joined task running
343+
# (its lifecycle is owned elsewhere).
344+
started = aio.AsyncEvent()
345+
346+
async def slow():
347+
started.set()
348+
await asyncio.sleep(60)
349+
350+
handle = aio.spawn_handle("test.slow", slow)
351+
await started.wait(2)
352+
353+
joiner = asyncio.ensure_future(aio.join_handle(handle, 30))
354+
await asyncio.sleep(0.05) # let the joiner park inside join_handle
355+
joiner.cancel()
356+
357+
with pytest.raises(asyncio.CancelledError):
358+
await joiner
359+
360+
assert not handle.cancelled() # joined task left running
361+
handle.cancel() # cleanup
362+
339363

340364
# ---------------------------------------------------------------------------
341365
# Callback scheduler

0 commit comments

Comments
 (0)