| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Add a stdin watchdog goroutine that monitors os.Stdin for EOF (broken pipe). When the MCP client crashes or is force-killed, stdin closes and the watchdog triggers shutdown, allowing the container to exit cleanly. Without this, orphaned containers accumulate since --rm only fires on graceful exit. After several orphans accumulate, concurrent MCP tool calls start hanging indefinitely. Fixes github#2323
|
Thank you for the approval! Is there anything else needed before merge? Happy to address any feedback. |
Sorry, something went wrong.
| buf := make([]byte, 1) | ||
| for { | ||
| n, err := os.Stdin.Read(buf) | ||
| if tn == 0 || err != nil { |
There was a problem hiding this comment.
| if tn == 0 || err != nil { | |
| if n == 0 || err != nil { |
I think you have a typo here
Sorry, something went wrong.
There was a problem hiding this comment.
Adds a stdin-closure watchdog to the stdio server so the process self-terminates when an MCP client disconnects ungracefully (especially in docker run -i --rm scenarios), preventing orphaned containers.
Changes:
| File | Description |
|---|---|
| internal/ghmcp/server.go | Adds stdin watchdog logic inside RunStdioServer to stop the server on stdin closure. |
internal/ghmcp/server.go:333
go func() {
buf := make([]byte, 1)
for {
n, err := os.Stdin.Read(buf)
if tn == 0 || err != nil {
logger.Info("stdin closed, shutting down server")
stop()
return
}
}
}()
Sorry, something went wrong.
| // Stdin watchdog: detect ungraceful client disconnect and self-terminate. | ||
| // Without this, orphaned containers accumulate when the MCP client (e.g. Cursor/VS Code) | ||
| // is force-killed or crashes, because `--rm` only fires on graceful exit. |
There was a problem hiding this comment.
The PR description mentions adding a unit test for the stdin watchdog behavior, but there are no tests in this package today (internal/ghmcp/server_test.go is empty) and this change introduces non-trivial shutdown behavior. Please add a focused unit test that exercises the EOF/error path and asserts that stop()/shutdown is triggered (e.g., by injecting an io.Reader into the stdio server wiring rather than hard-coding os.Stdin).
This issue also appears on line 323 of the same file.
Sorry, something went wrong.
|
@D2758695161 I missed the typo in my initial look, that will be to be addressed before it could be merged. |
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thanks for investigating this issue! On deeper inspection, I think unfortunately, we can't merge this PR because it introduces a potential data race on stdin.
Closing as a wontfix |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fix: Detect ungraceful stdin close and self-terminate
Fixes #2323 — Docker containers leak when MCP client disconnects ungracefully
Problem
When the MCP client (Cursor, VS Code, Claude Code) is force-killed or crashes, the docker run -i --rm container does not exit because:
Solution
Added a stdin watchdog goroutine in RunStdioServer that monitors os.Stdin for EOF. When the MCP client disconnects ungracefully, stdin returns EOF and the watchdog triggers graceful shutdown, allowing the container to exit cleanly.
Why this approach
Alternative approaches considered
Testing