Skip to content

Conversation

@fluidcat
Copy link
Contributor

@fluidcat fluidcat commented Dec 29, 2025

修复aiodocker.Docker()资源泄露问题

Modifications / 改动点

astrbot/builtin_stars/python_interpreter/main.py资源泄露,使用async with自动close

  • This is NOT a breaking change. / 这不是一个破坏性变更。

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

修复前:
bb1e1427-db26-42bd-9bb9-124c31397586
修复后:
06056f89-99e2-4749-83e7-56116ea54664


Checklist / 检查清单

  • 😊 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。/ If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  • 👀 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。/ My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  • 🤓 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到了 requirements.txtpyproject.toml 文件相应位置。/ 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.
  • 😮 我的更改没有引入恶意代码。/ My changes do not introduce malicious code.

Summary by Sourcery

通过在 Python 解释器沙箱集成中确保正确关闭 aiodocker.Docker 客户端,修复资源泄漏问题。

错误修复:

  • 在检查 Docker 可用性和重新拉取沙箱镜像时,防止 aiodocker.Docker 客户端资源泄漏。
  • 在 Python 解释器中多次运行沙箱代码执行时,避免 Docker 客户端资源泄漏。
Original summary in English

Summary by Sourcery

Fix resource leaks by ensuring aiodocker.Docker clients are properly closed in the Python interpreter sandbox integration.

Bug Fixes:

  • Prevent aiodocker.Docker client resource leaks when checking Docker availability and repulling sandbox images.
  • Avoid leaking Docker client resources during repeated sandbox code execution runs in the Python interpreter.

@auto-assign auto-assign bot requested review from Fridemn and Raven95676 December 29, 2025 09:53
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Dec 29, 2025
Copy link
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 - 我发现了 1 个问题

给 AI 代理的提示
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `astrbot/builtin_stars/python_interpreter/main.py:283-291` </location>
<code_context>
     async def pi_repull(self, event: AstrMessageEvent):
         """重新拉取沙箱镜像"""
-        docker = aiodocker.Docker()
-        image_name = await self.get_image_name()
-        try:
-            await docker.images.get(image_name)
</code_context>

<issue_to_address>
**suggestion (performance):** 镜像存在性检查和可能的拉取操作在每次迭代中都会执行;建议将其提升到循环外,以避免重复工作。

在 `python_interpreter` 中,`image_name = await self.get_image_name()` 以及随后的 `docker.images.get` / `docker.images.pull` 会在每次重试时运行。由于镜像名不会改变,并且镜像只需要拉取一次,这会引入不必要的延迟和 Docker 调用。可以在重试循环之前先解析镜像并执行一次 get/pull,以避免这些重复操作。

建议实现如下:

```python
    async def python_interpreter(self, event: AstrMessageEvent):
        # ... other setup code ...
        async with aiodocker.Docker() as docker:
            # resolve and ensure image once before retry loop to avoid redundant work
            image_name = await self.get_image_name()
            try:
                await docker.images.get(image_name)
            except aiodocker.exceptions.DockerError:
                await docker.images.pull(from_image=image_name)

            # retry loop
            for i in range(self.max_retries):
                # ... run container / handle execution using `image_name` ...

```

我根据你的注释假定了 `python_interpreter` 和重试循环的大致结构;你需要调整 `<<<<<<< SEARCH` 代码块,使其与当前实现完全一致。

具体来说,请更新 `python_interpreter`,使其满足:
1. 将重试循环内部的 `image_name = await self.get_image_name()` 以及后续的 `docker.images.get` / `docker.images.pull` 调用移动到循环之前。
2. 循环体重复使用已经解析好的 `image_name`,不要在每次迭代中再次调用 `get_image_name` 或执行 `get`/`pull`3. 如果存在多个重试循环,或依赖镜像已存在的提前返回路径,请确保它们都使用这一次的循环前镜像解析和存在性检查。
</issue_to_address>

Sourcery 对开源项目是免费的——如果你觉得我们的评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue

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

## Individual Comments

### Comment 1
<location> `astrbot/builtin_stars/python_interpreter/main.py:283-291` </location>
<code_context>
     async def pi_repull(self, event: AstrMessageEvent):
         """重新拉取沙箱镜像"""
-        docker = aiodocker.Docker()
-        image_name = await self.get_image_name()
-        try:
-            await docker.images.get(image_name)
</code_context>

<issue_to_address>
**suggestion (performance):** The image existence check and potential pull run on every iteration; consider hoisting this out of the loop to avoid redundant work.

In `python_interpreter`, `image_name = await self.get_image_name()` and the subsequent `docker.images.get` / `docker.images.pull` run on every retry. Since the image name doesn’t change and the image only needs to be pulled once, this adds unnecessary latency and Docker calls. Resolve the image and perform the get/pull once before the retry loop to avoid this repeated work.

Suggested implementation:

```python
    async def python_interpreter(self, event: AstrMessageEvent):
        # ... other setup code ...
        async with aiodocker.Docker() as docker:
            # resolve and ensure image once before retry loop to avoid redundant work
            image_name = await self.get_image_name()
            try:
                await docker.images.get(image_name)
            except aiodocker.exceptions.DockerError:
                await docker.images.pull(from_image=image_name)

            # retry loop
            for i in range(self.max_retries):
                # ... run container / handle execution using `image_name` ...

```

I assumed the shape of `python_interpreter` and the retry loop based on your comment; you will need to adjust the `<<<<<<< SEARCH` block to exactly match your current implementation.

Concretely, update `python_interpreter` so that:
1. Any `image_name = await self.get_image_name()` and subsequent `docker.images.get` / `docker.images.pull` calls inside the retry loop are moved to just before the loop.
2. The loop body reuses the already-resolved `image_name` without re-calling `get_image_name` or performing `get`/`pull` on each iteration.
3. If there are multiple retry loops or early-return paths that depend on the image being present, ensure they all use this single pre-loop image resolution and existence check.
</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.

@dosubot dosubot bot added the area:core The bug / feature is about astrbot's core, backend label Dec 29, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 29, 2025
@Soulter Soulter merged commit 90142bc into AstrBotDevs:master Dec 29, 2025
6 checks passed
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 lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants