Fix shutdown() task-wait to respect SHUTDOWN_TIMEOUT deadline (PCC-989)

[jamsea]

The bug

WaitingServer.shutdown() in pipecat-base/waiting_server.py has two wait loops:

1. Wait for open connections to close.
2. Wait for background tasks to finish.

The first loop checks should_exit_deadline (built from SHUTDOWN_TIMEOUT) and gives up once the deadline passes. The second loop did not check the deadline at all. It only ended when server_state.tasks emptied or force_exit became true.

force_exit only flips on a second SIGTERM/SIGINT, which Kubernetes never sends. Kubernetes sends one SIGTERM, waits the pod grace period, then SIGKILLs. There is no second signal.

A live bot session is started with background_tasks.add_task(run_bot, ...) (see app.py), a FastAPI BackgroundTask that uvicorn tracks in server_state.tasks until the request finishes. run_bot runs for the whole session, so the task set never empties while a session is active.

Net effect: if SIGTERM arrives during an active session, shutdown() blocks in the second loop with no timeout, no matter what SHUTDOWN_TIMEOUT is set to. From the outside this looks exactly like SIGTERM being swallowed until Kubernetes kills the pod. It silently defeats the timeout that SHUTDOWN_TIMEOUT is supposed to give on the main bot-session path.

Reproduced locally: an idle container exits in ~1s on SIGTERM, but a container with an in-flight background task stays alive and unresponsive well past a 5s SHUTDOWN_TIMEOUT, log frozen at "Waiting for background tasks to complete."

Why it matters (PCC-989)

PCC-989 tracks bot containers getting SIGKILLed mid-session with no error logs, well after SIGTERM was sent. There is a separate operator-side fix in progress (pipecat-cloud-operator #232) that caps the grace period for a boot-time liveness restart. This change is independent of that one: it fixes the runtime side so SHUTDOWN_TIMEOUT is actually honored on the primary session code path, instead of the shutdown hanging until the pod grace period runs out.

The fix

One-line change: add the same three-part deadline condition the connections loop already uses to the tasks loop.

while (
    self.server_state.tasks
    and not self.force_exit
    and (should_exit_deadline is None or time.time() < should_exit_deadline)
):
    await asyncio.sleep(0.1)

Test

Added pipecat-base/tests/test_waiting_server.py with a regression test that the task-wait loop gives up at the deadline instead of hanging, plus a control case for no active tasks. Confirmed the deadline test fails (times out) against the old code and passes with the fix. Full suite: 21 passed. Ruff check and format clean.

Draft: opening for review, not ready to merge.

[linear]

PCC-989

[Copilot]

Pull request overview

This PR fixes WaitingServer.shutdown() so the background-task wait loop honors the same SHUTDOWN_TIMEOUT/should_exit_deadline deadline as the open-connections wait loop, preventing shutdown from hanging indefinitely during in-flight bot sessions (PCC-989).

Changes:

• Apply the shutdown deadline condition to the background-tasks wait loop in WaitingServer.shutdown().
• Add a regression test ensuring shutdown returns after the deadline even when server_state.tasks never empties.
• Add a control test ensuring shutdown returns quickly when there are no tasks.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
pipecat-base/waiting_server.py	Adds the missing deadline condition to the background-task wait loop in shutdown() so SHUTDOWN_TIMEOUT is respected.
pipecat-base/tests/test_waiting_server.py	Adds regression coverage for the deadline behavior (and a no-tasks control case).

Comments suppressed due to low confidence (1)

pipecat-base/tests/test_waiting_server.py:54

• Use time.monotonic() for interval measurement to avoid flakiness from wall-clock adjustments.

    start = time.time()
    asyncio.run(asyncio.wait_for(server.shutdown(), timeout=5.0))
    elapsed = time.time() - start

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mattshep]

Verified the diagnosis independently — it's correct, and the fix is right. Approving. Details of what I checked:

Diagnosis verification

• Tasks loop lacks the deadline on main — confirmed: the connections loop checks should_exit_deadline, the tasks loop only checks self.server_state.tasks and not self.force_exit.
• A live session really does pin server_state.tasks — the HTTP start paths (app.py lines 355/476) launch run_bot via Starlette BackgroundTasks, which runs inside the same ASGI call, so uvicorn's per-request task stays in server_state.tasks for the whole session. (The await run_bot(...) paths at lines 229/258 hold a connection instead, and that loop already honored the deadline.)
• force_exit can never rescue the loop in k8s — current uvicorn's handle_exit only sets force_exit on a second SIGINT (if self.should_exit and sig == signal.SIGINT). A second SIGTERM wouldn't even flip it, and k8s never sends one anyway. The claim in the PR body is if anything understated.

Test verification

Ran the suite on the PR head in a clean worktree: 22 passed. Then swapped waiting_server.py back to main's version and re-ran the new tests:

• test_tasks_wait_gives_up_at_deadline — fails via asyncio.wait_for TimeoutError (the hang, exactly the reported symptom)
• test_string_timeout_does_not_crash_shutdown — fails with TypeError at the deadline computation

So both regression tests genuinely bite against the old code.

One thing worth calling out more loudly in the PR body

The float() coercion is really a second, distinct bug fix, not a hygiene tweak. Pre-fix behavior forked on whether the env var was set:

• SHUTDOWN_TIMEOUT unset → default is the int 7200 → deadline math works → the tasks-loop hang described in the body.
• SHUTDOWN_TIMEOUT set (env vars are always strings) → time.time() + "300" raises TypeError inside shutdown() → shutdown aborts with a traceback instead of hanging. Different failure signature, same net effect of SHUTDOWN_TIMEOUT never being honored.

Worth a sentence in the description so anyone matching this against prod symptoms knows there were two presentations. Side note: with the fix, a garbage value (e.g. SHUTDOWN_TIMEOUT=abc) now fails fast at import/boot via float() in app.py rather than at shutdown — I think that's the right trade.

Behavior-change note (fine, but should be conscious)

With PCC defaults (SHUTDOWN_TIMEOUT=7200, pod grace 7500), a bot that gets SIGTERM mid-session now exits cleanly at 7200s instead of riding to SIGKILL at 7500s. Sessions are bounded by maxSessionDuration=7200 anyway, so nothing is newly cut short — but customers who set SHUTDOWN_TIMEOUT low will now have in-flight sessions actually terminated at that bound, which is the documented contract finally being enforced.

The bounded lifespan-shutdown (asyncio.wait_for + skip-if-expired) looks right too; cancelling lifespan.shutdown() at that point is safe since the process is exiting regardless.

Relation to PCC-989

Agree with the framing that this is independent of the operator-side grace-cap fix (pipecat-cloud-operator PR 232): for dscout specifically, PID 1 never forwards SIGTERM, so this code path never even runs there. This fixes the adjacent population — any pipecat-base bot with an active session — which presents with the same "SIGTERM apparently swallowed until SIGKILL" fingerprint.

[markbackman]

@jamsea Before merging, please update the version in pyproject.toml and add a CHANGELOG.md entry.