Skip to content

fix: keep strong refs to pipeline tasks to prevent GC#8618

Open
KBVsent wants to merge 1 commit into
AstrBotDevs:masterfrom
KBVsent:fix/pipeline-task-gc
Open

fix: keep strong refs to pipeline tasks to prevent GC#8618
KBVsent wants to merge 1 commit into
AstrBotDevs:masterfrom
KBVsent:fix/pipeline-task-gc

Conversation

@KBVsent
Copy link
Copy Markdown
Contributor

@KBVsent KBVsent commented Jun 6, 2026

EventBus dispatched every incoming event with a bare asyncio.create_task(scheduler.execute(event)) whose return value was discarded. Because asyncio only holds weak references to tasks, a suspended pipeline task could be garbage-collected while still pending whenever it lost the reference to the future it was awaiting — for example when an underlying platform/network connection was dropped mid-handler. The result was a recurring Task was destroyed but it is pending! warning followed by a cascade of aclose(): asynchronous generator is already running errors, as the GC tried to finalize the nested suspended async generators of the onion-model pipeline. The work itself usually completed, but the noise masked real failures and exceptions were silently swallowed (Task exception was never retrieved).
18e16e0c2d72de861828ce880d3f0dc0

Modifications / 改动点

  • astrbot/core/event_bus.py: Hold a strong reference to each dispatched pipeline task in a set for the duration of its execution, preventing premature GC. A _on_task_done callback removes the task when finished and logs any uncaught exception via logger.error(..., exc_info=exc), so pipeline failures now surface as a real log entry instead of being lost.
  • This is NOT a breaking change. / 这不是一个破坏性变更。

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


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 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.txtpyproject.toml 文件相应位置。

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

Summary by Sourcery

Ensure event pipeline tasks remain alive for the duration of their execution so their failures are surfaced instead of being silently discarded.

Bug Fixes:

  • Prevent dispatched pipeline tasks from being garbage-collected while still pending by keeping strong references until completion.
  • Log uncaught exceptions from pipeline task execution so previously hidden task failures are now visible in logs.

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels Jun 6, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/event_bus.py" line_range="63" />
<code_context>
+            return
+        exc = task.exception()
+        if exc is not None:
+            logger.error("pipeline 任务执行异常", exc_info=exc)

     def _print_event(self, event: AstrMessageEvent, conf_name: str) -> None:
</code_context>
<issue_to_address>
**issue (bug_risk):** Passing the exception instance directly to `exc_info` will not log the task traceback as intended.

`exc_info` should be either a truthy flag (to use `sys.exc_info()`) or an `(exc_type, exc, tb)` tuple. Passing the exception instance (`exc_info=exc`) uses the current exception context, which is likely empty here, so the task’s traceback is lost. Instead, build the tuple explicitly so the task stack trace is logged:

```python
exc = task.exception()
if exc is not None:
    logger.error(
        "pipeline 任务执行异常",
        exc_info=(type(exc), exc, exc.__traceback__),
    )
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/core/event_bus.py
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to hold strong references to running asyncio tasks in astrbot/core/event_bus.py to prevent them from being garbage collected while in a pending state. It adds a _pending_tasks set to store these tasks and a _on_task_done callback to discard completed tasks and log any unhandled exceptions. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant