Skip to content

node: reap the stdio bridge backend after killing it - #431

Merged
aojea merged 1 commit into
google:mainfrom
aojea:node-reap-bridge-backend
Sep 18, 2026
Merged

aojea merged 1 commit into
google:mainfrom
aojea:node-reap-bridge-backend

Conversation

@aojea

@aojea aojea commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

baseService.Teardown killed the command backend's process but nothing ever called Wait, so every torn-down backend stayed a zombie until the node exited. The bridge's stdout reader goroutine now reaps it: it is the only reader of the pipe, so Wait cannot race a read on the pipe it closes, and every exit path (natural exit, crash, Teardown) ends there with an EOF. Pending callers are released before Wait runs so a slow kernel teardown of the child never holds up a request.

Teardown treats os.ErrProcessDone as success, since a backend that exited on its own has already been reaped by the time it is torn down.

baseService.Teardown killed the command backend's process but nothing
ever called Wait, so every torn-down backend stayed a zombie until the
node exited. The bridge's stdout reader goroutine now reaps it: it is
the only reader of the pipe, so Wait cannot race a read on the pipe it
closes, and every exit path (natural exit, crash, Teardown) ends there
with an EOF. Pending callers are released before Wait runs so a slow
kernel teardown of the child never holds up a request.

Teardown treats os.ErrProcessDone as success, since a backend that
exited on its own has already been reaped by the time it is torn down.

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

Copy link
Copy Markdown
Contributor

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 improves process lifecycle management and cleanup in the node service. Specifically, it ensures that killed command backends are properly reaped by calling Wait() in the StdioBridge to prevent zombie processes, and updates Teardown to ignore os.ErrProcessDone errors when a process has already exited. A new unit test has been added to verify this reaping behavior. No review comments were provided, and the changes align with the repository's testing guidelines.

@aojea
aojea merged commit 46dd156 into google:main Sep 18, 2026
18 checks passed
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.

1 participant