Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions astrbot/core/initial_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def __init__(self, db: BaseDatabase, log_broker: LogBroker) -> None:
self.logger = logger
self.log_broker = log_broker
self.webui_dir: str | None = None
self.webui_available = True

async def start(self) -> None:
core_lifecycle = AstrBotCoreLifecycle(self.log_broker, self.db)
Expand All @@ -35,6 +36,14 @@ async def start(self) -> None:

core_task = core_lifecycle.start()

if not self.webui_available:
try:
await core_task
except asyncio.CancelledError:
logger.info("🌈 正在关闭 AstrBot...")
await core_lifecycle.stop()
return
Comment on lines +39 to +45

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When webui_available is False, the core_task is awaited directly without being wrapped in a try...except asyncio.CancelledError block. This means if the application is cancelled or shut down during this time, the CancelledError will propagate immediately, and core_lifecycle.stop() will never be called. This prevents proper cleanup of resources, database connections, and plugin termination.

Wrapping the await in a try...except block ensures that stop() is always called upon cancellation.

        if not self.webui_available:
            try:
                await core_task
            except asyncio.CancelledError:
                logger.info("🌈 正在关闭 AstrBot...")
                await core_lifecycle.stop()
            return

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I pushed 92335ce16 to give the no-WebUI path the same cancellation cleanup as the dashboard path: if core_task is cancelled, core_lifecycle.stop() is awaited before returning.

Focused validation:

python -m py_compile astrbot\core\initial_loader.py main.py tests\test_main.py
python -m pytest tests\test_main.py -q
python -m ruff check astrbot\core\initial_loader.py tests\test_main.py main.py
git diff --check upstream/master..HEAD

tests/test_main.py now includes a cancellation regression for this branch.


webui_dir = self.webui_dir

self.dashboard_server = AstrBotDashboard(
Expand Down
1 change: 1 addition & 0 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ async def main_async(webui_dir_arg: str | None) -> None:

core_lifecycle = InitialLoader(db, log_broker)
core_lifecycle.webui_dir = webui_dir
core_lifecycle.webui_available = webui_dir is not None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你好,我简单看了这行代码,想问一下: webui_dir is not None 真的可以等价于 webui_available=True吗?如果可以,能否简单说明一下:一个非 None 的 webui_dir 如何保证它一定指向可用的WebUI dist?如果不能,那我可以认为,存在一个反例,webui_dir 非 None,但 WebUI 实际不可用? 例如在issue #8784 中,data/dist 可以存在并被返回为 webui_dir,但它可能缺少 index.html 或 assets。这种情况下 webui_dir is not None 为 True,但 WebUI 并不可用。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right that webui_dir is not None is not a standalone proof that the dist is complete.

The intended split is:

So this line is safe only with that check_dashboard_files() contract. Without #8785, your counterexample is valid. I kept the changes separate because one PR validates the cached dist and this one handles the no-WebUI runtime path.

await core_lifecycle.start()


Expand Down
66 changes: 66 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import os
import sys
from pathlib import Path
Expand Down Expand Up @@ -257,3 +258,68 @@ async def test_check_dashboard_files_with_webui_dir_arg(monkeypatch):
assert result == valid_dir
mock_download.assert_not_called()
mock_get_version.assert_not_called()


@pytest.mark.asyncio
async def test_main_async_disables_webui_when_dashboard_check_fails():
with mock.patch(
"main.check_dashboard_files",
mock.AsyncMock(return_value=None),
):
with mock.patch("main.log_broker", create=True):
with mock.patch("main.InitialLoader") as mock_loader:
mock_loader.return_value.start = mock.AsyncMock()

from main import main_async

await main_async(None)

assert mock_loader.return_value.webui_dir is None
assert mock_loader.return_value.webui_available is False
mock_loader.return_value.start.assert_awaited_once()


@pytest.mark.asyncio
async def test_initial_loader_skips_unavailable_webui():
from astrbot.core.initial_loader import InitialLoader

core_lifecycle = mock.MagicMock()
core_lifecycle.initialize = mock.AsyncMock()
core_lifecycle.start = mock.AsyncMock()

with mock.patch(
"astrbot.core.initial_loader.AstrBotCoreLifecycle",
return_value=core_lifecycle,
):
with mock.patch(
"astrbot.core.initial_loader.AstrBotDashboard",
) as mock_dashboard:
loader = InitialLoader(mock.MagicMock(), mock.MagicMock())
loader.webui_available = False

await loader.start()

core_lifecycle.initialize.assert_awaited_once()
core_lifecycle.start.assert_awaited_once()
mock_dashboard.assert_not_called()


@pytest.mark.asyncio
async def test_initial_loader_stops_core_when_unavailable_webui_is_cancelled():
from astrbot.core.initial_loader import InitialLoader

core_lifecycle = mock.MagicMock()
core_lifecycle.initialize = mock.AsyncMock()
core_lifecycle.start = mock.AsyncMock(side_effect=asyncio.CancelledError)
core_lifecycle.stop = mock.AsyncMock()

with mock.patch(
"astrbot.core.initial_loader.AstrBotCoreLifecycle",
return_value=core_lifecycle,
):
loader = InitialLoader(mock.MagicMock(), mock.MagicMock())
loader.webui_available = False

await loader.start()

core_lifecycle.stop.assert_awaited_once()
Loading