Improve handling of FASTAPI Asyncio in API

[JC-wk]

Resolves #4923

What is being addressed

This pull request resolves a reliability issue where background Service Bus message processors (DeploymentStatusUpdater and AirlockStatusUpdater) could potentially be silently garbage-collected. It refactors task lifecycle management to use application-scoped state via a type-safe BackgroundTaskManager container, implements safe concurrent task cancellation during shutdown with a bounded timeout, gates error logging to prevent misleading runtime errors during graceful shutdown/reload cycles, and implements gather-result inspection.

Problem Description

Previously, background workers were instantiated and launched using:

asyncio.create_task(deploymentStatusUpdater.receive_messages())
asyncio.create_task(airlockStatusUpdater.receive_messages())

In Python, the asyncio event loop only maintains weak references to tasks. Discarding the return value of asyncio.create_task() without keeping a strong reference elsewhere left the task instances vulnerable to garbage collection during runtime, leading to silent processing failures.

Additionally, this introduced other architectural concerns:

• No Lifecycle Scoping: Global mutable state (e.g. module-level sets) poses contamination and race risks across multiple test runs or application reloads.
• Unsafe Mutation: Iterating over active tasks while they remove themselves from the set on completion via a done callback triggers a RuntimeError: Set changed size during iteration.
• Swallowed Exceptions: If any background loop crashed, the exception went completely unlogged and unnoticed, or tracebacks were dropped.
• Misleading/Duplicate Error Paths: During app shutdown, tasks are explicitly cancelled, but cancellation timing races could still surface non-cancel exceptions from teardown paths and log them as runtime failures even when shutdown was intentional.
• No Shutdown Timeout: The shutdown asyncio.gather had no timeout, risking indefinite hangs if a task ignored cancellation or blocked in an uninterruptible await path.
• Lack of Type Contracts: app.state.background_tasks was dynamically typed as a raw set, making it prone to type safety violations during future refactorings.

Solution

Refactored lifespan in api_app/main.py to implement a correct, type-safe, and robust asyncio background worker pattern:

1. App-Scoped & Type-Annotated State Tracking

Background tasks are managed via a dedicated BackgroundTaskManager registered in FastAPI's app.state.background_tasks:

class BackgroundTaskManager:
    def __init__(self) -> None:
        self._tasks: set[asyncio.Task[Any]] = set()
        self.is_shutting_down: bool = False
    ...

This guarantees task contexts are cleanly isolated per application instance and enforces explicit type constraints.

2. Task Naming and Observability

Tasks are created with human-readable names for improved debuggability and stack tracing:

• name="deployment-status-updater"
• name="airlock-status-updater"

3. Safe, Controlled Shutdown with Bounded Timeout

During teardown (after yield), tasks are copied to a list before cancellation to prevent mutation-during-iteration errors. The gather call is wrapped in a 10.0 second timeout to prevent indefinite hangs:

app.state.background_tasks.is_shutting_down = True
tasks = app.state.background_tasks.get_tasks()
for task in tasks:
    task.cancel()

try:
    results = await asyncio.wait_for(
        asyncio.gather(*tasks, return_exceptions=True),
        timeout=10.0
    )
    ...

4. Gated Logging & Gather-Result Inspection

• In the task's _done_callback, we gate error logging behind the manager's is_shutting_down flag to avoid noisy and misleading error logs during normal shutdown/reload cycles.
• During shutdown, we inspect the gathered results: any unexpected non-cancellation exceptions (excluding asyncio.CancelledError) are caught, classified, and logged as warnings with full tracebacks.

---

Changes

• api_app/main.py: Refactored lifespan management, added BackgroundTaskManager, and implemented shutdown timeout/gather-result inspection.
• api_app/tests_ma/test_main_lifecycle.py: Added unit tests validating task manager behavior, lifespan lifecycle, and error logging/teardown exceptions.
• api_app/_version.py: Bumped the API version from 0.25.26 to 0.25.27.
• CHANGELOG.md: Expanded release notes with detailed operational impact (task lifecycle, cancellation, and logging changes).

Testing

• Verified the changes by running the local test suite using pytest.
• All 682 test cases completed successfully (including the new lifecycle and task management tests) with no regressions.

[github-actions]

Unit Test Results

682 tests   682 ✅  8s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 8a403f8.

♻️ This comment has been updated with latest results.

[Copilot]

Pull request overview

This pull request improves reliability of long-running background Service Bus workers in the FastAPI API by managing their lifecycle within the application lifespan, aiming to prevent silent task garbage collection and to make shutdown behavior more controlled/observable.

Changes:

• Track background worker tasks in app.state to keep strong references scoped to the FastAPI application lifecycle.
• Add controlled shutdown logic that cancels tracked tasks, awaits completion, and logs failures.
• Add an unreleased changelog entry describing the fix.

Reviewed changes

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

File	Description
api_app/main.py	Refactors FastAPI lifespan to track, name, cancel, and gather background tasks during shutdown.
CHANGELOG.md	Adds an Unreleased BUG FIXES entry for the background task lifecycle fix.

[maxmartin-cgi]

/test-extended

[github-actions]

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/28435506268 (with refid 599e251c)

(in response to this comment from @maxmartin-cgi)

[marrobi]

From Opus 4.8:

1. Potential duplicate logging / misleading error path during shutdown

• File: api_app/main.py
• In _done_callback, any non-cancelled exception is logged as an error.
• During app shutdown, tasks are explicitly cancelled and then awaited with gather(..., return_exceptions=True), but cancellation timing races can still surface non-cancel exceptions from teardown paths and be logged as runtime failures even when shutdown is intentional.
• Risk: noisy or misleading error logs during normal shutdown/reload cycles.
• Suggested fix: gate error logging on an app “is_shutting_down” flag, or distinguish expected teardown exceptions from true runtime failures.

2. Missing explicit type annotation / structure for app.state storage

• File: api_app/main.py
• app.state.background_tasks = set() is dynamically typed and reused across lifecycle logic without an explicit contract.
• Risk: future refactors can accidentally store non-Task objects, causing runtime failures at .get_name(), .cancel(), or callback handling.
• Suggested fix: initialize with an annotated container (e.g., set[asyncio.Task[Any]]) via helper function or typed local alias and only expose add/remove through a small utility.

3. No timeout on shutdown gather for background workers

• File: api_app/main.py
• await asyncio.gather(*tasks, return_exceptions=True) has no timeout.
• Risk: if a task ignores cancellation or blocks in an uninterruptible await path, application shutdown can hang indefinitely.
• Suggested fix: wrap gather with asyncio.wait_for(..., timeout=<bounded>) and log any tasks still pending after timeout.

4. PR description and implementation diverge on exception handling details

• File: PR description vs api_app/main.py
• PR text claims exception handling via results = await asyncio.gather(...) + per-result logging with exc_info=result, but implementation logs in done-callback and does not process gathered results for failure classification.
• Risk: maintainers/reviewers may approve based on behavior that is not actually present; future debugging expectations mismatch.
• Suggested fix: align description to actual code or implement the documented gather-result inspection pattern.

5. Version/changelog bump appears bundled with behavioral change but no migration/release note granularity

• Files: api_app/_version.py, CHANGELOG.md
• Version increment and changelog entry are very terse for a lifecycle/shutdown behavior change.
• Risk: downstream operators may not realize shutdown semantics and logging behavior changed (important for ops playbooks).
• Suggested fix: expand changelog entry with explicit operational impact (startup/shutdown task lifecycle, cancellation behavior, and logging changes).

[Copilot]

Pull request overview

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