fix(agent): 修复退出时套接字残留 - #289
Open
1204244136 wants to merge 2 commits into
Open
Conversation
在应用全局退出和主窗口销毁时统一清理 AgentClient 与子进程。 监督尚未写回实例的 Agent,避免连接、停止和退出并发时遗留套接字或孤儿进程。
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些总体反馈:
- 在
agent_shutdown_requested和PendingAgentState.cancelled上使用Ordering::SeqCst可能比实际需要的更强;请考虑是否使用Ordering::AcqRel/Ordering::Relaxed即可,在保持必要的 happens-before 关系的同时降低同步开销。 - 在
cleanup_all_agents和cleanup_pending_agents中,你在不显式设置超时时间的情况下直接释放了AgentClient,而在其他地方是有设置超时时间的;请确认这些代码路径不会在长时间运行的 RPC 上发生阻塞,如果有需要的话,应让它们的超时行为与InstanceRuntime::drop和stop_agent_blocking保持一致。 - 在
remove_pending_agent_socket中硬编码的 Windows 套接字目录C:/Temp可能在某些系统上不存在或不可写;建议使用更标准的临时目录位置(例如%TEMP%),或者复用已有的临时目录工具,以避免删除失败。
给 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- The use of `Ordering::SeqCst` on `agent_shutdown_requested` and `PendingAgentState.cancelled` may be stronger than necessary; consider whether `Ordering::AcqRel` / `Ordering::Relaxed` would be sufficient to reduce synchronization cost while still preserving the required happens-before relationships.
- In `cleanup_all_agents` and `cleanup_pending_agents` you drop `AgentClient`s without explicitly setting timeouts like elsewhere; confirm that these code paths won't end up blocking on long-running RPCs and, if needed, align their timeout behavior with `InstanceRuntime::drop` and `stop_agent_blocking`.
- The hardcoded Windows socket directory `C:/Temp` in `remove_pending_agent_socket` may not exist or be user-writeable on all systems; consider using a more standard temp location (e.g., `%TEMP%`) or reusing an existing temp directory utility to avoid deletion failures.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The use of
Ordering::SeqCstonagent_shutdown_requestedandPendingAgentState.cancelledmay be stronger than necessary; consider whetherOrdering::AcqRel/Ordering::Relaxedwould be sufficient to reduce synchronization cost while still preserving the required happens-before relationships. - In
cleanup_all_agentsandcleanup_pending_agentsyou dropAgentClients without explicitly setting timeouts like elsewhere; confirm that these code paths won't end up blocking on long-running RPCs and, if needed, align their timeout behavior withInstanceRuntime::dropandstop_agent_blocking. - The hardcoded Windows socket directory
C:/Tempinremove_pending_agent_socketmay not exist or be user-writeable on all systems; consider using a more standard temp location (e.g.,%TEMP%) or reusing an existing temp directory utility to avoid deletion failures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The use of `Ordering::SeqCst` on `agent_shutdown_requested` and `PendingAgentState.cancelled` may be stronger than necessary; consider whether `Ordering::AcqRel` / `Ordering::Relaxed` would be sufficient to reduce synchronization cost while still preserving the required happens-before relationships.
- In `cleanup_all_agents` and `cleanup_pending_agents` you drop `AgentClient`s without explicitly setting timeouts like elsewhere; confirm that these code paths won't end up blocking on long-running RPCs and, if needed, align their timeout behavior with `InstanceRuntime::drop` and `stop_agent_blocking`.
- The hardcoded Windows socket directory `C:/Temp` in `remove_pending_agent_socket` may not exist or be user-writeable on all systems; consider using a more standard temp location (e.g., `%TEMP%`) or reusing an existing temp directory utility to avoid deletion failures.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
将仅承载取消信号的原子变量改为宽松内存序,由生命周期锁和子进程槽锁建立必要的先行发生关系。 清理待连接 Agent 的套接字时同时覆盖系统临时目录和 Windows IPC 兼容目录。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
在应用全局退出和主窗口销毁时统一清理 AgentClient 与子进程。
监督尚未写回实例的 Agent,避免连接、停止和退出并发时遗留套接字或孤儿进程。
Summary by Sourcery
确保在应用程序关闭和主窗口销毁期间,一致地清理代理及其子进程,防止遗留套接字或孤立进程。
Bug Fixes(错误修复):
Enhancements(增强改进):
Original summary in English
Summary by Sourcery
Ensure agents and their child processes are consistently cleaned up during application shutdown and main window destruction to prevent leftover sockets or orphaned processes.
Bug Fixes:
Enhancements: