Skip to content

Commit 88fa10f

Browse files
committed
fix(receiver): harden callback draining during shutdown
- keep callback tasks owned until cleanup completes - prevent AnyIO level-cancellation busy loops - preserve interruptible unbounded graceful waits - release execution capacity exactly once - clarify wait-tasks-timeout semantics - add deterministic lifecycle and cancellation tests
1 parent ae2b788 commit 88fa10f

5 files changed

Lines changed: 653 additions & 47 deletions

File tree

docs/guide/cli.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,10 @@ kill -HUP <main pid>
143143

144144
### Graceful and force shutdowns
145145

146-
If you send `SIGINT` or `SIGKILL` to the main process by pressing <kbd>Ctrl</kbd>+<kbd>C</kbd> or using the `kill` command, it will initiate the shutdown process.
146+
If you send `SIGINT` or `SIGTERM` to the main process by pressing <kbd>Ctrl</kbd>+<kbd>C</kbd> or using the `kill` command, it will initiate the shutdown process.
147147
By default, it will stop fetching new messages immediately after receiving the signal but will wait for the completion of all currently executing tasks.
148148

149-
If you don't want to wait too long for tasks to complete each time you shut down the worker, you can either send termination signals three times to the main process to perform a hard kill or configure the `--wait-tasks-timeout` to set a hard time limit for shutting down.
149+
If you don't want to wait indefinitely for tasks to complete, configure `--wait-tasks-timeout` to set their graceful completion period. Once that period expires, the worker requests cancellation of asynchronous task callbacks that are still running and waits for their cleanup before continuing shutdown. Synchronous functions already running in a thread or process executor cannot be forcibly stopped by asyncio cancellation and may continue until the executor shuts down. This is not an absolute process deadline; repeat the termination signal until the configured hard-kill threshold is reached if the process must stop immediately.
150150

151151
::: tip Cool tip
152152
The number of signals before a hard kill can be configured with the `--hardkill-count` CLI argument.
@@ -171,7 +171,7 @@ The number of signals before a hard kill can be configured with the `--hardkill-
171171
* `--max-tasks-per-child` - maximum number of tasks to be executed by a single worker process before restart.
172172
* `--max-fails` - Maximum number of child process exits.
173173
* `--shutdown-timeout` - maximum amount of time for graceful broker's shutdown in seconds (default 5).
174-
* `--wait-tasks-timeout` - if cannot read new messages from the broker or maximum number of tasks is reached, worker will wait for all current tasks to finish. This parameter sets the maximum amount of time to wait until shutdown.
174+
* `--wait-tasks-timeout` - graceful completion period for current tasks during shutdown. Cancellation is requested for callbacks still running after the period, and their cleanup is awaited. The default `None` waits without a timeout.
175175
* `--hardkill-count` - Number of termination signals to the main process before performing a hardkill.
176176

177177
## Scheduler

taskiq/cli/worker/args.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,10 @@ def from_cli(
256256
"--wait-tasks-timeout",
257257
type=float,
258258
default=None,
259-
help="Maximum time to wait for all current tasks to finish before exiting.",
259+
help=(
260+
"Grace period for current task callbacks before cancellation is "
261+
"requested during shutdown."
262+
),
260263
)
261264
parser.add_argument(
262265
"--hardkill-count",

taskiq/receiver/receiver.py

Lines changed: 93 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import sys
77
from collections.abc import Callable
88
from concurrent.futures import Executor, ProcessPoolExecutor
9+
from contextlib import suppress
910
from logging import getLogger
1011
from time import time
1112
from typing import Any, get_type_hints
@@ -78,6 +79,7 @@ def __init__(
7879
self.known_tasks: set[str] = set()
7980
self.max_tasks_to_execute = max_tasks_to_execute
8081
self.wait_tasks_timeout = wait_tasks_timeout
82+
self._active_tasks: set[asyncio.Task[Any]] = set()
8183
for task in self.broker.get_all_tasks().values():
8284
self._prepare_task(task.task_name, task.original_func)
8385
self.sem: asyncio.Semaphore | None = None
@@ -461,63 +463,111 @@ async def runner(
461463
462464
:param queue: queue with prefetched data.
463465
"""
464-
tasks: set[asyncio.Task[Any]] = set()
465-
466-
def task_cb(task: "asyncio.Task[Any]") -> None:
467-
"""
468-
Callback for tasks.
469-
470-
This function used to remove task
471-
from the list of active tasks and release
472-
the semaphore, so other tasks can use it.
473-
474-
:param task: finished task
475-
"""
476-
tasks.discard(task)
477-
if self.sem is not None:
478-
self.sem.release()
479-
480-
while True:
481-
try:
466+
capacity_acquired = False
467+
graceful_shutdown = False
468+
try:
469+
while True:
482470
# Waits for semaphore to be released.
483471
if self.sem is not None:
484472
await self.sem.acquire()
473+
capacity_acquired = True
485474

486475
self.sem_prefetch.release()
487476
message = await queue.get()
488477
if message is QUEUE_DONE:
489-
# asyncio.wait will throw an error if there is nothing to wait for
490-
if tasks:
491-
logger.info(
492-
f"Waiting for {len(tasks)} running tasks to complete...",
493-
)
494-
await asyncio.wait(tasks, timeout=self.wait_tasks_timeout)
495-
logger.info("No more tasks to wait for. Shutting down.")
478+
graceful_shutdown = True
496479
break
497480

498-
# Custom hooks for OTel and any future instrumentations
499-
for middleware in reversed(self.broker.middlewares):
500-
if hasattr(middleware, "on_prefetch_queue_remove"):
501-
await maybe_awaitable(
502-
middleware.on_prefetch_queue_remove(), # type: ignore
503-
)
481+
await self._notify_prefetch_queue_remove()
482+
self._start_callback(message)
483+
capacity_acquired = False
484+
except asyncio.CancelledError:
485+
pass
486+
finally:
487+
if capacity_acquired and self.sem is not None:
488+
self.sem.release()
489+
await self._drain_active_tasks(
490+
cancel_immediately=not graceful_shutdown,
491+
)
492+
logger.info("The runner is stopped.")
493+
494+
async def _run_owned_callback(self, message: bytes | AckableMessage) -> None:
495+
"""Run one callback outside repeated listener-scope cancellation."""
496+
with anyio.CancelScope(shield=True):
497+
await self.callback(message=message, raise_err=False)
504498

505-
task = asyncio.create_task(
506-
self.callback(message=message, raise_err=False),
499+
async def _notify_prefetch_queue_remove(self) -> None:
500+
"""Run instrumentation before transferring callback ownership."""
501+
for middleware in reversed(self.broker.middlewares):
502+
if hasattr(middleware, "on_prefetch_queue_remove"):
503+
await maybe_awaitable(
504+
middleware.on_prefetch_queue_remove(), # type: ignore
507505
)
508-
tasks.add(task)
509506

510-
# We want the task to remove itself from the set when it's done.
511-
#
512-
# Because if we won't save it anywhere,
513-
# python's GC can silently cancel task
514-
# and this behaviour considered to be a Hisenbug.
515-
# https://textual.textualize.io/blog/2023/02/11/the-heisenbug-lurking-in-your-async-code/
516-
task.add_done_callback(task_cb)
507+
def _start_callback(self, message: bytes | AckableMessage) -> None:
508+
"""Transfer the current execution slot to an owned callback task."""
509+
task = asyncio.create_task(self._run_owned_callback(message))
510+
self._active_tasks.add(task)
511+
task.add_done_callback(self._on_callback_done)
512+
513+
def _on_callback_done(self, task: "asyncio.Task[Any]") -> None:
514+
"""Release callback capacity and retrieve unexpected failures."""
515+
self._active_tasks.discard(task)
516+
if self.sem is not None:
517+
self.sem.release()
518+
if task.cancelled():
519+
return
520+
task_exception = task.exception()
521+
if task_exception is not None:
522+
logger.error(
523+
"Receiver callback failed outside task execution handling.",
524+
exc_info=(
525+
type(task_exception),
526+
task_exception,
527+
task_exception.__traceback__,
528+
),
529+
)
530+
531+
async def _drain_active_tasks(
532+
self,
533+
*,
534+
cancel_immediately: bool,
535+
) -> None:
536+
"""Wait for callbacks and cancel work beyond the graceful boundary."""
537+
tasks = set(self._active_tasks)
538+
if not tasks:
539+
return
517540

541+
logger.info("Waiting for %d running tasks to complete...", len(tasks))
542+
if cancel_immediately:
543+
pending = {task for task in tasks if not task.done()}
544+
else:
545+
try:
546+
_, pending = await asyncio.wait(
547+
tasks,
548+
timeout=self.wait_tasks_timeout,
549+
)
518550
except asyncio.CancelledError:
519-
break
520-
logger.info("The runner is stopped.")
551+
pending = {task for task in tasks if not task.done()}
552+
553+
if pending:
554+
logger.warning("Cancelling %d running callback tasks.", len(pending))
555+
with anyio.CancelScope(shield=True):
556+
await self._cancel_callback_tasks(pending)
557+
logger.info("No more tasks to wait for. Shutting down.")
558+
559+
@staticmethod
560+
async def _cancel_callback_tasks(
561+
tasks: set[asyncio.Task[Any]],
562+
) -> None:
563+
"""Cancel callbacks once and await cleanup despite outer cancellation."""
564+
for task in tasks:
565+
task.cancel()
566+
waiter = asyncio.gather(*tasks, return_exceptions=True)
567+
while not waiter.done():
568+
with suppress(asyncio.CancelledError):
569+
await asyncio.shield(waiter)
570+
waiter.result()
521571

522572
def _prepare_task(self, name: str, handler: Callable[..., Any]) -> None:
523573
"""
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import asyncio
2+
from collections.abc import Awaitable
3+
from typing import Any
4+
5+
import pytest
6+
7+
from taskiq.abc.broker import AckableMessage
8+
from taskiq.abc.middleware import TaskiqMiddleware
9+
from taskiq.brokers.inmemory_broker import InMemoryBroker
10+
from taskiq.receiver.receiver import QUEUE_DONE, Receiver
11+
12+
13+
class ReceiverLifecycleError(RuntimeError):
14+
"""Marker error for deterministic runner failures."""
15+
16+
17+
class ShieldCallCounter:
18+
"""Count shield calls made by one named asyncio task."""
19+
20+
def __init__(self, task_name: str) -> None:
21+
self.task_name = task_name
22+
self.runner_calls = 0
23+
self._shield = asyncio.shield
24+
25+
def __call__(self, awaitable: Awaitable[Any]) -> asyncio.Future[Any]:
26+
current_task = asyncio.current_task()
27+
if current_task is not None and current_task.get_name() == self.task_name:
28+
self.runner_calls += 1
29+
return self._shield(awaitable)
30+
31+
32+
class FailingRemoveMiddleware(TaskiqMiddleware):
33+
"""Fail before the runner transfers capacity to a callback."""
34+
35+
def on_prefetch_queue_remove(self) -> None:
36+
raise ReceiverLifecycleError("prefetch remove failed")
37+
38+
39+
class FailingSecondRemoveMiddleware(TaskiqMiddleware):
40+
"""Fail while one previously admitted callback is still active."""
41+
42+
def __init__(self) -> None:
43+
super().__init__()
44+
self.calls = 0
45+
46+
def on_prefetch_queue_remove(self) -> None:
47+
self.calls += 1
48+
if self.calls == 2:
49+
raise ReceiverLifecycleError("second prefetch remove failed")
50+
51+
52+
class BlockingRemoveMiddleware(TaskiqMiddleware):
53+
"""Expose cancellation before callback ownership is transferred."""
54+
55+
def __init__(self) -> None:
56+
super().__init__()
57+
self.started = asyncio.Event()
58+
59+
async def on_prefetch_queue_remove(self) -> None:
60+
self.started.set()
61+
await asyncio.Event().wait()
62+
63+
64+
class ReceiverQueue(asyncio.Queue[bytes | AckableMessage]):
65+
"""Expose runner reads and graceful-shutdown sentinel delivery."""
66+
67+
def __init__(self) -> None:
68+
super().__init__()
69+
self.get_started = asyncio.Event()
70+
self.shutdown_received = asyncio.Event()
71+
72+
async def get(self) -> bytes | AckableMessage:
73+
self.get_started.set()
74+
message = await super().get()
75+
if message is QUEUE_DONE:
76+
self.shutdown_received.set()
77+
return message
78+
79+
80+
class ControlledReceiver(Receiver):
81+
"""Receiver with deterministic callback and cleanup checkpoints."""
82+
83+
def __init__(
84+
self,
85+
*,
86+
wait_tasks_timeout: float | None,
87+
max_async_tasks: int | None = 1,
88+
fail: bool = False,
89+
) -> None:
90+
super().__init__(
91+
InMemoryBroker(),
92+
max_async_tasks=max_async_tasks,
93+
run_startup=False,
94+
wait_tasks_timeout=wait_tasks_timeout,
95+
)
96+
self.fail = fail
97+
self.started_callbacks: asyncio.Queue[None] = asyncio.Queue()
98+
self.release_callback = asyncio.Event()
99+
self.cleanup_started_callbacks: asyncio.Queue[None] = asyncio.Queue()
100+
self.release_cleanup = asyncio.Event()
101+
self.finished_callbacks: asyncio.Queue[None] = asyncio.Queue()
102+
self.callback_tasks: list[asyncio.Task[None]] = []
103+
104+
@property
105+
def callback_task(self) -> asyncio.Task[None] | None:
106+
"""Return the most recently started callback task."""
107+
if not self.callback_tasks:
108+
return None
109+
return self.callback_tasks[-1]
110+
111+
async def callback(
112+
self,
113+
message: bytes | AckableMessage,
114+
raise_err: bool = False,
115+
) -> None:
116+
del message, raise_err
117+
callback_task = asyncio.current_task()
118+
assert callback_task is not None
119+
self.callback_tasks.append(callback_task)
120+
self.started_callbacks.put_nowait(None)
121+
try:
122+
await self.release_callback.wait()
123+
if self.fail:
124+
raise ReceiverLifecycleError("callback failed")
125+
finally:
126+
self.cleanup_started_callbacks.put_nowait(None)
127+
await self.release_cleanup.wait()
128+
for _ in range(20):
129+
await asyncio.sleep(0)
130+
self.finished_callbacks.put_nowait(None)
131+
132+
async def settle(self, runner_task: asyncio.Task[None]) -> None:
133+
"""Release all checkpoints and settle test-owned tasks."""
134+
self.release_callback.set()
135+
self.release_cleanup.set()
136+
callback_tasks = set(self.callback_tasks) | set(self._active_tasks)
137+
for callback_task in callback_tasks:
138+
if not callback_task.done():
139+
callback_task.cancel()
140+
if not runner_task.done():
141+
runner_task.cancel()
142+
await asyncio.gather(
143+
*callback_tasks,
144+
runner_task,
145+
return_exceptions=True,
146+
)
147+
148+
149+
async def start_callback(
150+
receiver: ControlledReceiver,
151+
) -> tuple[ReceiverQueue, asyncio.Task[None]]:
152+
"""Start one controlled callback through the real runner boundary."""
153+
queue = ReceiverQueue()
154+
await queue.put(b"payload")
155+
runner_task = asyncio.create_task(receiver.runner(queue))
156+
await wait_for_signals(receiver.started_callbacks)
157+
return queue, runner_task
158+
159+
160+
async def wait_for_signals(
161+
signals: asyncio.Queue[None],
162+
count: int = 1,
163+
) -> None:
164+
"""Wait for an exact number of deterministic lifecycle checkpoints."""
165+
await asyncio.wait_for(
166+
asyncio.gather(*(signals.get() for _ in range(count))),
167+
timeout=1,
168+
)
169+
170+
171+
async def assert_exact_capacity(receiver: Receiver, slots: int = 1) -> None:
172+
"""Assert that exactly the expected execution permits were returned."""
173+
assert receiver.sem is not None
174+
for _ in range(slots):
175+
await asyncio.wait_for(receiver.sem.acquire(), timeout=0.1)
176+
with pytest.raises(asyncio.TimeoutError):
177+
await asyncio.wait_for(receiver.sem.acquire(), timeout=0.01)
178+
for _ in range(slots):
179+
receiver.sem.release()

0 commit comments

Comments
 (0)