[rwzdk]

Symptom

Opening the dashboard Backup page (or polling GET /api/v1/backups) runs the whole backup-directory scan — os.listdir, os.stat, and a zipfile.ZipFile() open plus read("manifest.json") for every stored backup — directly on the asyncio event loop thread. With several backups on disk this freezes the loop for the duration of the read: WebChat SSE streams stall, plugin Web handlers queue up, and AstrBot's own lag monitor (astrbot/core/utils/event_loop_diagnostics.py:15, threshold 15 s) fires exactly on this kind of stall.

Trigger input

Any authenticated dashboard user hitting the list endpoint, e.g. with three archives present in data/backups:

GET /api/v1/backups?page=1&page_size=20        # -> astrbot/dashboard/api/backups.py:115
GET /api/backup/list?page=1&page_size=20      # legacy route -> backups.py:128

The dashboard polls this endpoint whenever the Backup tab is open, so the stall repeats.

Root cause

BackupService.list_backups is a synchronous method (astrbot/dashboard/services/backup_service.py:183) that performs blocking filesystem work per file: os.listdir at line 188 and a real ZIP open + manifest read via get_backup_manifest at line 196 (zipfile.ZipFile(zip_path, "r") at line 174).

Both routes pass it to _run as a plain callable:

return await _run(
    lambda: service.list_backups(page=page, page_size=page_size),
    ...
)

and _run → run_maybe_async (astrbot/dashboard/async_utils.py:17) simply calls the lambda and only awaits the result if it happens to be awaitable. A sync callable therefore executes inline on the event loop thread — there is no thread hop anywhere in the path.

Sibling dashboard code already treats this pattern as the bug it is: stat_service.py:231 (await asyncio.to_thread(self.storage_cleaner.get_status)), logs.py:96, bot_cleanup_service.py:148 all offload sync service calls before awaiting them.

Fix

Route both list endpoints through asyncio.to_thread, matching the established pattern in the dashboard layer. No signature, schema, or route surface changes, so the generated dashboard API client is unaffected.

return await _run(
    lambda: asyncio.to_thread(service.list_backups, page=page, page_size=page_size),
    prefix="获取备份列表失败",
)

asyncio.to_thread is available from Python 3.9, so this stays 3.10-compatible and cross-platform. The thread hop is asserted on thread identity, not on any timing/sleep, so the test is deterministic.

Red (test added, source unchanged at origin/master)

E       AssertionError: backup ZIP was opened on the event loop thread
E       assert 8028 != 8028
...
FAILED tests/unit/test_backup_list_off_event_loop.py::test_list_backups_reads_manifests_off_the_event_loop
FAILED tests/unit/test_backup_list_off_event_loop.py::test_legacy_backup_list_reads_manifests_off_the_event_loop
2 failed, 1 warning in 13.29s

Green

2 passed, 1 warning in 9.97s

Regression check

tests/test_backup.py tests/test_fastapi_v1_dashboard.py tests/test_dashboard.py
tests/unit/test_backup_list_off_event_loop.py
-> 359 passed, 1 skipped, 4 warnings in 170.55s

Repo-wide ruff format --check . (526 files already formatted) and ruff check . (All checks passed) both pass.

Prepared with an AI coding agent.

Summary by Sourcery

Offload backup manifest scanning from both dashboard list endpoints while preserving event-loop cleanup scheduling.

Bug Fixes:

• Prevent backup listing from blocking the asyncio event loop while reading backup manifests, keeping dashboard and WebChat activity responsive.

Enhancements:

• Ensure upload cleanup is initialized on the event loop before backup scanning is offloaded to a worker thread.

Tests:

• Add coverage verifying that both backup list routes perform ZIP manifest reads off the event loop and initialize cleanup on the event-loop thread.

[sourcery-ai]

Hey - I've found 2 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/dashboard/api/backups.py" line_range="119" />
<code_context>
+    # run on the event loop thread that also serves WebChat and the platform adapters.
     return await _run(
-        lambda: service.list_backups(page=page, page_size=page_size),
+        lambda: asyncio.to_thread(service.list_backups, page=page, page_size=page_size),
         prefix="获取备份列表失败",
     )
</code_context>
<issue_to_address>
**Backup listing fails during concurrent changes**

When a backup is renamed or deleted after its manifest is read but before `os.stat`, `os.stat` raises `FileNotFoundError`, and `_run` returns an error for the entire listing instead of returning the remaining backups.

Handle `FileNotFoundError` for an archive that disappears during the scan and skip that archive.

Also at `astrbot/dashboard/api/backups.py:132`.
</issue_to_address>

### Comment 2
<location path="astrbot/dashboard/api/backups.py" line_range="119" />
<code_context>
+    # run on the event loop thread that also serves WebChat and the platform adapters.
     return await _run(
-        lambda: service.list_backups(page=page, page_size=page_size),
+        lambda: asyncio.to_thread(service.list_backups, page=page, page_size=page_size),
         prefix="获取备份列表失败",
     )
</code_context>
<issue_to_address>
**Upload cleanup never starts**

When the cleanup task is unset or has stopped when a list endpoint runs, `asyncio.to_thread` runs `list_backups` in a worker, where `ensure_cleanup_task_started` calls `asyncio.create_task` without a running event loop; the caught `RuntimeError` leaves the coroutine unawaited and cleanup does not start, so polling emits a `RuntimeWarning` and expired upload chunks remain.

Start the cleanup task on the event-loop thread before running the backup scan in a worker.

Also at `astrbot/dashboard/api/backups.py:132`.
</issue_to_address>

Sourcery assessment

Approval pending. 2 findings to address first.

Blocking findings: astrbot/dashboard/api/backups.py:119, astrbot/dashboard/api/backups.py:119

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

[rwzdk]

Both findings checked; the second one was a regression in this PR and is fixed.

Upload cleanup never starts — valid, fixed in 7d71388.
BackupService.list_backups calls ensure_cleanup_task_started() (backup_service.py:184), which reaches chunked_upload_service.py:302-309:

try:
    self._cleanup_task = asyncio.create_task(self._cleanup_expired_sessions())
except RuntimeError:
    pass

asyncio.create_task needs a running loop, so once the whole call moved to a worker thread the RuntimeError was swallowed, the cleanup coroutine was dropped unawaited, and expired upload chunks stopped being reclaimed — silently, because the except is already there. Both list endpoints now start the cleanup on the event loop thread before handing the ZIP scan to the worker:

service.ensure_cleanup_task_started()
return await _run(
    lambda: asyncio.to_thread(service.list_backups, page=page, page_size=page_size),
    prefix="获取备份列表失败",
)

Added a test that pins this by thread identity (no sleeps): test_upload_cleanup_is_scheduled_on_the_event_loop asserts the starter runs on the loop thread. Reverting just the endpoint change makes it fail exactly as described — the only thread that scheduled cleanup was the worker:

>       assert loop_thread in started_threads, (
E       assert 13424 in [45104]
1 failed, 2 passed, 2 warnings in 10.76s

With the fix, the same file:

pytest tests/unit/test_backup_list_off_event_loop.py -q
3 passed

and the surrounding suites:

pytest tests/test_backup.py tests/test_fastapi_v1_dashboard.py tests/unit/test_backup_list_off_event_loop.py -q
273 passed, 4 warnings in 110.95s

ruff format / ruff check are clean on both changed files.

Backup listing fails during concurrent changes — real, but pre-existing and out of scope here.
list_backups (backup_service.py:196-201) reads the manifest and then calls os.stat on the same path, so an archive deleted in between makes the whole listing fail. That behaviour is unchanged by this PR — it only moves where the same code runs — and the fix belongs in the service, not the route. Happy to send a separate small change that skips a vanished archive (except FileNotFoundError: continue) with a test that makes os.stat raise for one entry, if you want that tracked.