Avoid lingering tasks when using background backup tasks #5518
Conversation
When a backup tasks is run in background, but actually has an error early the secondary event task to release the callee is lingering around still, ultimately leading to a "Task was destroyed but it is pending!" asyncio error. Make sure we cancel the event task in case the backup returns early.
📝 WalkthroughWalkthroughThe changes modify the Changes
Sequence DiagramsequenceDiagram
participant Backup Task
participant Event Task
participant Async Wait
Backup Task->>Event Task: Create task with event.wait()
Backup Task->>Async Wait: Wait for backup completion
Async Wait-->>Backup Task: Wait resolved
alt If event task still pending
Backup Task->>Event Task: Cancel task
end
The sequence diagram illustrates the new flow of task management, showing how the Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
supervisor/api/backups.py (1)
307-310: Optional: Log the canceled task for better observability.Canceling the
event_taskis correct to avoid lingering tasks. However, you may consider adding a debug log or comment explaining why the event task is being canceled, to help with diagnosing concurrency or cancellation-related issues in the future.Here’s a small example diff for adding a debug log:
if event_task in pending: + _LOGGER.debug("Backup completed early; canceling event_task to avoid lingering tasks.") event_task.cancel()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supervisor/api/backups.py(1 hunks)
🔇 Additional comments (1)
supervisor/api/backups.py (1)
299-303: Concurrency approach looks well-implemented.
The usage of asyncio.wait(..., return_when=asyncio.FIRST_COMPLETED) to wait for either the backup task or the event task is an appropriate pattern. It ensures that once one task completes (or raises), the other can be inspected for pending status. This effectively handles concurrent operations without unnecessary runtime overhead.
|
For reference, this got rid of such errors e.g. when entering a wrong password: |
Proposed change
When a backup tasks is run in background, but actually has an error early the secondary event task to release the callee is lingering around still, ultimately leading to a "Task was destroyed but it is pending!" asyncio error.
Make sure we cancel the event task in case the backup returns early.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit