Skip to content

fix: ignore SIGCHLD to prevent server shutdown from Chrome exits#565

Open
shanrichard wants to merge 3 commits intoxpzouying:mainfrom
shanrichard:fix/browser-process-leak
Open

fix: ignore SIGCHLD to prevent server shutdown from Chrome exits#565
shanrichard wants to merge 3 commits intoxpzouying:mainfrom
shanrichard:fix/browser-process-leak

Conversation

@shanrichard
Copy link
Copy Markdown

Problem

The MCP server shuts down unexpectedly after ~120 seconds idle or immediately after handling requests.

time="2026-03-05T15:28:54Z" level=info msg="启动 HTTP 服务器: :18060"
time="2026-03-05T15:30:54Z" level=info msg="正在关闭服务器..."    # exactly 120s later

Root cause: When Chrome subprocesses (renderers, GPU process, crashpad handlers) exit, the OS sends SIGCHLD to the parent Go process. Go's signal handling can misdeliver this as an unexpected signal, interfering with the server's graceful shutdown logic and causing premature termination.

Fixes #515

Fix

Explicitly ignore SIGCHLD before setting up the shutdown signal handler:

signal.Ignore(syscall.SIGCHLD)

This is a one-line fix in app_server.go. Only SIGINT/SIGTERM are handled for graceful shutdown — all other signals (especially SIGCHLD from Chrome) are ignored.

Additional note

There is also a related Chrome process leak issue in the upstream dependency headless_browser. When Close() is called, Chrome processes may not be forcefully killed, leading to accumulation of zombie Chrome processes over time. A fix has been submitted upstream: xpzouying/headless_browser#9

Impact

  • Server stays running even when Chrome subprocesses exit
  • Graceful shutdown via SIGINT/SIGTERM still works correctly
  • No behavior change for normal operations

🤖 Generated with Claude Code

… exits

When Chrome subprocesses (renderers, GPU process, crashpad) exit, they
send SIGCHLD to the parent process. Go's runtime delivers this signal
to signal.Notify channels. Since only SIGINT and SIGTERM are registered,
the SIGCHLD gets delivered as an unexpected signal that can interfere
with the server's signal handling, causing premature shutdown.

This explicitly ignores SIGCHLD before setting up the shutdown signal
handler, ensuring Chrome subprocess lifecycle events don't affect the
HTTP server.

Fixes xpzouying#515

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@shanrichard shanrichard requested a review from xpzouying as a code owner March 10, 2026 03:51
@kimi-agent-bot
Copy link
Copy Markdown
Contributor

🔍 Kimi Code Agent 正在自动审查此 PR...

@kimi-agent-bot
Copy link
Copy Markdown
Contributor

我来帮你审查这个 PR。首先获取 PR 的详细信息和 diff。
让我查看相关文件以更好地理解上下文:
根据获取的信息,我现在进行代码审查。


PR 审查报告:#565

基本信息

项目 内容
标题 fix: ignore SIGCHLD to prevent server shutdown from Chrome exits
作者 @shanrichard
分支 fix/browser-process-leakmain
变更 +5 行, -0 行, 1 个文件

问题背景(Issue #515

服务器在 Docker 容器中运行约 120 秒后会自动关闭,或在处理请求后立即关闭。根本原因是 Chrome 子进程退出时产生的 SIGCHLD 信号干扰了 Go 的信号处理机制。


代码审查

1. ✅ 正确性

修复方案正确。signal.Ignore(syscall.SIGCHLD) 会告诉 Go 运行时完全忽略 SIGCHLD 信号,阻止其被传递到任何信号通道。这解决了 Chrome 子进程退出时触发服务器意外关闭的问题。

2. ✅ 代码位置

代码放置在 signal.Notify 之前是正确的顺序,确保在注册关闭信号处理器之前就忽略 SIGCHLD

3. ✅ 注释质量

注释清晰地解释了:

  • 为什么需要忽略 SIGCHLD
  • 具体防止什么问题
  • 关联的 Issue 链接

4. ⚠️ 建议改进

虽然修复是正确的,但建议考虑以下改进:

建议 1:添加调试日志

如果将来再次出现信号相关的问题,有日志会更容易调试:

// Ignore SIGCHLD to prevent Chrome child process exits from
// being misdelivered to the signal channel and shutting down the server.
// See: https://github.com/xpzouying/xiaohongshu-mcp/issues/515
if err := signal.Ignore(syscall.SIGCHLD); err != nil {
    logrus.Warnf("无法忽略 SIGCHLD: %v", err)
}

注意:signal.Ignore 实际上不会返回错误,所以上面的代码只是示例。但如果需要更严格的错误处理,可以考虑包装。

建议 2:记录收到的信号

在 Issue #515 的讨论中,建议添加信号源调试。考虑在关闭时记录实际收到的信号:

sig := <-quit
logrus.Infof("收到信号 %v,正在关闭服务器...", sig)

这有助于未来诊断类似的信号问题。


验证检查清单

  • 修复针对 Chrome 子进程退出导致的 SIGCHLD 问题
  • SIGINTSIGTERM 仍能正常触发优雅关闭
  • 代码注释清晰,包含 Issue 引用
  • 变更最小化,专注于修复问题
  • 无破坏性变更

结论

APPROVE

这个 PR 提供了一个简洁有效的修复方案,解决了 Issue #515 中描述的 Chrome 子进程导致服务器意外关闭的问题。代码质量良好,注释清晰。

可选改进(非阻塞):

  1. 在关闭时记录实际收到的信号,便于 future debugging
  2. 在注释中添加更详细的技术解释(已足够清晰)

建议合并。


审查者:Kimi Code Agent
时间:2026-03-10

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 930b9c26da

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app_server.go
// Ignore SIGCHLD to prevent Chrome child process exits from
// being misdelivered to the signal channel and shutting down the server.
// See: https://github.com/xpzouying/xiaohongshu-mcp/issues/515
signal.Ignore(syscall.SIGCHLD)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid globally ignoring SIGCHLD

Calling signal.Ignore(syscall.SIGCHLD) changes child-process semantics for the entire process: exited children are auto-reaped, so later exec.Cmd.Wait/Run calls can fail with waitid: no child processes. This server launches browser subprocesses through newBrowser() in service.go, so startup/teardown paths that rely on waiting for Chrome can become unreliable after this line runs, turning a shutdown-signal workaround into a runtime regression for subprocess management.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker container auto-shuts down after ~120s idle / immediately after handling request

1 participant