[JosephTian876]

Modifications / 改动点

AstrBotExporter.export_all() ran every ZIP compression and directory walk on the asyncio event loop thread. On the reporting instance (2.4 GB of plugin data) that froze the whole bot for 76 s, and AstrBot's own diagnostic recorded it:

[02:11:06.451] [Core] [INFO] [backup.exporter:92]: 开始导出备份到 ...astrbot_backup_20261006_021106.zip
[02:13:10.498] [Core] [INFO] [backup.exporter:196]: 备份导出完成: ...
[02:13:10.534] [Core] [WARN] [utils.event_loop_diagnostics:93]: Event loop lag detected: 119.578s (threshold 15.000s).

This is not merely a slow export. While the loop is frozen the reverse WebSocket to the OneBot implementation cannot answer; the implementation times out after ~90 s, declares the connection half-open and reconnects. On the reporting instance that reconnect then collided with an unrelated aiocqhttp API-client bookkeeping race, after which every outbound API call raised ApiNotAvailable and the bot stopped responding entirely until it was restarted.

Changes in astrbot/core/backup/exporter.py:

• Added a dedicated single-worker ThreadPoolExecutor owned by export_all, created at the start and always shut down in finally.
• Added _run_in_archive_thread() and routed every access to the shared ZipFile (5 × writestr, the directory walk, attachments, knowledge-base media, FAISS index) through it. A single dedicated worker is used rather than the shared default pool (asyncio.to_thread) because zipfile.ZipFile is not safe for concurrent use.
• The ZipFile is now opened and closed on that worker instead of via with on the event loop. A single-worker executor is a FIFO queue, so a close submitted after an in-flight write cannot overlap it. Previously the with block's __exit__ could call ZipFile.close() on the loop thread while a write was still running, raising Can't close the ZIP file while there is an open writing handle on it and leaving an unusable partial archive.
• Teardown is BaseException-safe, so an asyncio.CancelledError (which does not inherit from Exception) can no longer skip the partial-archive cleanup.
• Added a re-entrancy guard so a second concurrent export_all() on the same instance fails immediately with a clear error instead of silently subverting the first call's executor.
• A failed archive finalisation is now reported as a failure. If writing the central directory (where ENOSPC/EIO/quota errors surface) fails, the partial archive is removed and the error is raised, rather than returning a path to an unopenable file that callers record as a completed backup.

tests/test_backup.py: added regression tests.

No changes to the backup format, ZIP layout, manifest.json, checksums, progress_callback semantics, any public signature, importer.py, or BACKUP_MANIFEST_VERSION. No new dependencies.

Screenshots or Test Results / 运行截图或测试结果

Real 2399 MB dataset, same machine, same AstrBot, only the exporter implementation differing:

	event-loop freeze	export wall time	heartbeats sampled
before	76.72 s	76.72 s	0
after	0.024 s	73.13 s	4688

The "before" row is the point: the heartbeat coroutine collected zero samples during a 76.7 s export, because the loop never got a chance to run. Export wall time is essentially unchanged (76.7 s → 73.1 s), i.e. the work moved off the loop rather than becoming faster.

Output is unchanged. Exporting the same real dataset with the old and new code produces archives matching on: member path list and order (3524 members), per-member CRC / uncompressed size / compress type, all recorded checksums, manifest.json apart from the wall-clock exported_at, and statistics.directories. Spot-checked members are byte-identical. With the clock frozen, the two archives are byte-identical.

A failed finalisation no longer reports success. Patching the real zipfile.ZipFile._write_end_record to raise OSError(ENOSPC):

	outcome	file left behind
before this fix	returns normally, caller records completed	corrupt, BadZipFile
after this fix	raises OSError with the real errno	none; instance reusable

Verification:

• uv run pytest tests/test_backup.py -q → 88 passed (76 before this change)

• uv run pytest tests/unit -q → 1449 passed, 23 skipped

• uvx --from ruff==0.15.22 ruff format --check . → 514 files already formatted

• uvx --from ruff==0.15.22 ruff check . → All checks passed!

• The new guards fail on the previous implementation (verified by swapping it back: 4 failed), so they genuinely guard the fix rather than merely passing.

• Independent adversarial verification across three review rounds and three test rounds at hash-pinned revisions: cancellation (36 checks), concurrency (120 checks), failure paths and thread/handle leak checks (20 checks), byte-equivalence and edge inputs (34 checks: CJK/emoji filenames, zero-length files, empty/absent directories, deep nesting, files deleted mid-walk, __pycache__/.pyc exclusion), and the knowledge-base path (23 checks). 12 sequential exports grew neither thread count nor OS handles.

• This is NOT a breaking change.

---

Checklist / 检查清单

• 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  / 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。

• 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  / 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。

• 📚 I checked the affected WebUI instructions and screenshots in docs/zh and docs/en against the changed navigation, page structure, and labels, and updated them in this PR (or explained why no documentation update is needed). For renamed, moved, or merged entry points, I included an old entry → new entry mapping in the documentation and changelog.
  / 我已对照变化后的 WebUI 入口、页面结构和术语，核对并在本 PR 中更新 docs/zh 和 docs/en 的相关操作说明与截图（或说明无需更新文档的原因）。入口改名、移动或合并时，已在文档和 changelog 中补充 旧入口 → 新入口 对照。

• 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  / 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到 requirements.txt 和 pyproject.toml 文件相应位置。

• 😮 My changes do not introduce malicious code.
  / 我的更改没有引入恶意代码。

Summary by Sourcery

Move backup archive generation off the asyncio event loop while preserving the existing backup format and strengthening failure and cleanup handling.

Bug Fixes:

• Prevent backup exports from blocking the asyncio event loop during ZIP compression, directory traversal, serialization, and manifest generation.
• Ensure cancelled, failed, or unsuccessfully finalized exports clean up partial archives and report errors instead of returning unusable backups.
• Reset per-export checksum state so reused exporter instances do not produce stale manifest entries.

Enhancements:

• Serialize archive operations through a dedicated single-worker executor, with safeguards against concurrent exports on the same exporter instance.

Tests:

• Add regression coverage for event-loop responsiveness, thread isolation, cancellation, re-entrancy, cleanup, archive integrity, checksum consistency, and finalization failures.

[sourcery-ai]

Hey - I've found 3 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/backup/exporter.py" line_range="187" />
<code_context>
+                await progress_callback("main_db", 0, 100, "正在导出主数据库...")
+            main_data = await self._export_main_database()
+            main_db_json = json.dumps(
+                main_data, ensure_ascii=False, indent=2, default=str
+            )
+            await self._run_in_archive_thread(
</code_context>
<issue_to_address>
**Export work stalls the event loop**

When database or knowledge-base data is large, or a knowledge base has a large media tree, `export_all()` runs JSON serialization and `_generate_manifest()` synchronously on the event loop; the manifest scan also walks the media tree there. These operations delay heartbeats and other bot tasks despite ZIP writes being offloaded.

Offload JSON serialization and manifest generation, including its filesystem scan, from the event-loop thread.

Also at `astrbot/core/backup/exporter.py:208`, `astrbot/core/backup/exporter.py:232-233`, `astrbot/core/backup/exporter.py:282-283`.
</issue_to_address>

### Comment 2
<location path="astrbot/core/backup/exporter.py" line_range="282" />
<code_context>
-                # 6. 生成 manifest
-                if progress_callback:
-                    await progress_callback("manifest", 0, 100, "正在生成清单...")
-                manifest = self._generate_manifest(main_data, kb_meta_data, dir_stats)
-                manifest_json = json.dumps(manifest, ensure_ascii=False, indent=2)
-                zf.writestr("manifest.json", manifest_json)
</code_context>
<issue_to_address>
**Manifest lists stale checksums**

When the same exporter instance is reused after an optional file or knowledge-base content is omitted, `_generate_manifest()` includes entries left in `self._checksums` from the prior export, so the new manifest lists checksums for members absent from its ZIP.

Reset `_checksums` for each export before collecting checksums.

Also at `astrbot/core/backup/exporter.py:365-366`.
</issue_to_address>

### Comment 3
<location path="astrbot/core/backup/exporter.py" line_range="347" />
<code_context>
+                    )
+                    while not close_task.done():
+                        try:
+                            await asyncio.shield(close_task)
+                        except asyncio.CancelledError:
+                            cancelled_while_cleaning = True
</code_context>
<issue_to_address>
**Failed exports leave partial ZIPs**

When archive opening or closing raises a `BaseException` other than `asyncio.CancelledError`, the `open_task.result()` or shielded `close_task` path propagates the exception past the partial-file removal, leaving a corrupt ZIP in the backup directory.

Ensure partial-file cleanup runs even when archive opening or closing raises a `BaseException`.

Also at `astrbot/core/backup/exporter.py:331`, `astrbot/core/backup/exporter.py:351`.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 3 findings to address first, and if the executor, cancellation, or finalization handling is wrong, the export could leave a corrupt or incomplete backup archive, or report failure after producing one. Reverting restores the previous export behavior, and the affected archive can be removed and regenerated from the unchanged source data.

Blocking findings: astrbot/core/backup/exporter.py:187, astrbot/core/backup/exporter.py:282, astrbot/core/backup/exporter.py:347

---

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

• X
• Mastodon
• LinkedIn
• Facebook

[JosephTian876]

Sourcery's three findings are addressed in f3426110f

All three were real; the event-loop split was incomplete rather than wrong. Each fix has a regression test that fails on the previous revision.

1. Export work still stalled the event loop (exporter.py:187, 208, 232-233, 282-283)

Offloaded to the same single-worker archive executor that already owns the ZipFile:

• json.dumps for the main DB, KB metadata and per-KB documents
• _generate_manifest and its json.dumps — this is the one that mattered most, since it walks every KB media directory

Using one worker for both serialization and archive writes is deliberate: it keeps the shared ZipFile serialised while the CPU-bound work still leaves the loop. Test: test_serialization_and_manifest_leave_event_loop_thread records threading.get_ident() inside both calls and asserts neither ran on the loop thread.

2. Manifest listed stale checksums (exporter.py:282, 365-366)

self._checksums is now cleared at the top of export_all, before any state is mutated. Test: test_export_manifest_drops_stale_checksums exports twice on one instance, removes cmd_config.json between runs, and asserts the second manifest neither lists that checksum nor references a member absent from that ZIP.

3. Failed exports left partial ZIPs (exporter.py:331, 347, 351)

The open and close awaits caught only Exception, so a BaseException propagated out of open_task.result() / the shielded close and jumped over the os.remove(zip_path) cleanup. Both paths now capture BaseException into open_error / close_error, and the cleanup condition includes open_error. A failed open with no other error in flight is raised rather than returning a path to a file that was never produced.

Tests: test_base_exception_in_teardown_still_removes_partial_zip (extended with a ZIP-cleanup assertion) and test_base_exception_during_open_still_removes_partial_zip (writes a partial file inside the failing open, asserts the backup directory is empty afterwards).

Verification on f3426110f:

• tests/test_backup.py — 91 passed
• ruff format --check + ruff check on both changed files — clean
• reverted exporter.py to the pre-fix revision: the three new checks fail there and pass here