From bf52457c27e2f5bc42a923d3c8248d3cb09bfa45 Mon Sep 17 00:00:00 2001 From: wangtinghu Date: Fri, 4 Sep 2026 16:31:55 +0800 Subject: [PATCH] fix(wrapper): bound child bootstrap handshake with timeout and parent cancellation (#910) A child that never answers the synthetic initialize request blocked launchCodexChildSession forever: the bootstrap read loops have no deadline and the launch path ignored ctx. Each wedged launch left the session process and its whole child tree resident, and repeated headless launches accumulated them without bound until the cgroup hit its task limit, after which every clone() in the unit failed with EAGAIN (observed in the field as a 13-hour crash loop, 527 coredumps of external commands and the daemon's own Go runtime dying with "fatal error: newosproc"). Wrap all three backend bootstrap call sites (codex, claude, opencode) in runChildBootstrap, which bounds the handshake with wrapperBootstrapTimeout (15s), honours parent cancellation, and cancels the child context on either so the process is killed and its stdout pipe closes, unblocking the bootstrap goroutine. Co-Authored-By: Claude --- internal/app/wrapper/app.go | 1 + internal/app/wrapper/app_child_session.go | 36 +++++++++- .../app/wrapper/app_child_session_claude.go | 5 +- .../app/wrapper/app_child_session_opencode.go | 4 +- .../app/wrapper/app_child_session_test.go | 68 +++++++++++++++++++ 5 files changed, 111 insertions(+), 3 deletions(-) diff --git a/internal/app/wrapper/app.go b/internal/app/wrapper/app.go index 48c0f02c0..7fad0632a 100644 --- a/internal/app/wrapper/app.go +++ b/internal/app/wrapper/app.go @@ -29,6 +29,7 @@ const ( steerCommandResponseTimeout = 5 * time.Second wrapperChildStopGrace = 2 * time.Second wrapperChildWaitTimeout = 5 * time.Second + wrapperBootstrapTimeout = 15 * time.Second ) type shutdownRequest struct { diff --git a/internal/app/wrapper/app_child_session.go b/internal/app/wrapper/app_child_session.go index f87d599d9..dd1540483 100644 --- a/internal/app/wrapper/app_child_session.go +++ b/internal/app/wrapper/app_child_session.go @@ -2,6 +2,7 @@ package wrapper import ( "context" + "fmt" "io" "os/exec" "sync/atomic" @@ -49,7 +50,9 @@ func (a *App) launchCodexChildSession(ctx context.Context, rawLogger *debuglog.R } a.debugf("child started: binary=%s pid=%d cwd=%s", a.config.CodexRealBinary, cmd.Process.Pid, a.config.WorkspaceRoot) - bootstrappedStdout, err := a.bootstrapHeadlessCodex(childStdin, childStdout, rawLogger, reportProblem) + bootstrappedStdout, err := a.runChildBootstrap(ctx, wrapperBootstrapTimeout, childCancel, func() (io.Reader, error) { + return a.bootstrapHeadlessCodex(childStdin, childStdout, rawLogger, reportProblem) + }) if err != nil { childCancel() _ = cmd.Wait() @@ -73,6 +76,37 @@ func (a *App) launchCodexChildSession(ctx context.Context, rawLogger *debuglog.R }, nil } +// runChildBootstrap bounds the synthetic initialize handshake. A child that +// never answers initialize would otherwise block the launch forever, keeping +// the session process and its whole child tree resident and accumulating under +// repeated launches (issue #910). On timeout or parent cancellation the child +// context is cancelled so the process is killed and its stdout pipe closes, +// which unblocks the bootstrap goroutine and lets it drain into the buffered +// channel instead of leaking. The caller still owns reaping via cmd.Wait(). +func (a *App) runChildBootstrap(ctx context.Context, timeout time.Duration, childCancel context.CancelFunc, boot func() (io.Reader, error)) (io.Reader, error) { + type bootstrapResult struct { + stdout io.Reader + err error + } + done := make(chan bootstrapResult, 1) + go func() { + stdout, err := boot() + done <- bootstrapResult{stdout, err} + }() + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case res := <-done: + return res.stdout, res.err + case <-ctx.Done(): + childCancel() + return nil, ctx.Err() + case <-timer.C: + childCancel() + return nil, fmt.Errorf("child bootstrap: initialize response not received within %s", timeout) + } +} + func startChildSessionIO(ctx context.Context, session *childSession, parentStdout, parentStderr io.Writer, writeCh chan []byte, runtime backendRuntime, client *relayws.Client, commandResponses *commandResponseTracker, turnTracker *runtimeTurnTracker, activeGeneration *int64, generation int64, errCh chan<- error, debugf func(string, ...any), rawLogger *debuglog.RawLogger, reportProblem func(agentproto.ErrorInfo)) { if session == nil { return diff --git a/internal/app/wrapper/app_child_session_claude.go b/internal/app/wrapper/app_child_session_claude.go index ed395375f..b15a798f4 100644 --- a/internal/app/wrapper/app_child_session_claude.go +++ b/internal/app/wrapper/app_child_session_claude.go @@ -3,6 +3,7 @@ package wrapper import ( "context" "encoding/json" + "io" "os" "strings" @@ -31,7 +32,9 @@ func (a *App) launchClaudeChildSession(ctx context.Context, rawLogger *debuglog. } a.debugf("claude child started: binary=%s pid=%d cwd=%s", claudeBinary, cmd.Process.Pid, a.config.WorkspaceRoot) - bootstrappedStdout, err := a.bootstrapClaude(childStdin, childStdout, rawLogger, reportProblem) + bootstrappedStdout, err := a.runChildBootstrap(ctx, wrapperBootstrapTimeout, childCancel, func() (io.Reader, error) { + return a.bootstrapClaude(childStdin, childStdout, rawLogger, reportProblem) + }) if err != nil { childCancel() _ = cmd.Wait() diff --git a/internal/app/wrapper/app_child_session_opencode.go b/internal/app/wrapper/app_child_session_opencode.go index 84d1151f8..135090887 100644 --- a/internal/app/wrapper/app_child_session_opencode.go +++ b/internal/app/wrapper/app_child_session_opencode.go @@ -46,7 +46,9 @@ func (a *App) launchOpenCodeChildSession(ctx context.Context, rawLogger *debuglo } a.debugf("opencode child started: binary=%s pid=%d cwd=%s", openCodeBinary, cmd.Process.Pid, a.config.WorkspaceRoot) - bootstrappedStdout, err := a.bootstrapOpenCodeACP(translator, childStdin, childStdout, rawLogger, reportProblem) + bootstrappedStdout, err := a.runChildBootstrap(ctx, wrapperBootstrapTimeout, childCancel, func() (io.Reader, error) { + return a.bootstrapOpenCodeACP(translator, childStdin, childStdout, rawLogger, reportProblem) + }) if err != nil { childCancel() _ = cmd.Wait() diff --git a/internal/app/wrapper/app_child_session_test.go b/internal/app/wrapper/app_child_session_test.go index 7706ab521..5c1a1f669 100644 --- a/internal/app/wrapper/app_child_session_test.go +++ b/internal/app/wrapper/app_child_session_test.go @@ -385,3 +385,71 @@ func TestRestartChildSessionStopsCurrentIOBeforeLaunchingReplacement(t *testing. cancel() waitForSessionIOStopped(next, 2*time.Second) } + +func TestRunChildBootstrapReturnsBootstrappedStdout(t *testing.T) { + app := New(Config{Source: "headless", Version: "test"}) + stdout, err := app.runChildBootstrap(context.Background(), wrapperBootstrapTimeout, func() {}, func() (io.Reader, error) { + return strings.NewReader("bootstrapped"), nil + }) + if err != nil { + t.Fatalf("runChildBootstrap: %v", err) + } + data, err := io.ReadAll(stdout) + if err != nil { + t.Fatalf("read stdout: %v", err) + } + if string(data) != "bootstrapped" { + t.Fatalf("expected stdout %q, got %q", "bootstrapped", string(data)) + } +} + +func TestRunChildBootstrapTimesOutWedgedChild(t *testing.T) { + pipeReader, pipeWriter := io.Pipe() + defer pipeReader.Close() + cancelCalled := make(chan struct{}) + childCancel := func() { + // Simulate the kill path: cancelling the child closes its stdout pipe, + // which is what unblocks a wedged bootstrap read. + _ = pipeWriter.Close() + close(cancelCalled) + } + boot := func() (io.Reader, error) { + buf := make([]byte, 1) + if _, err := pipeReader.Read(buf); err != nil { + return nil, err + } + return strings.NewReader("unexpected"), nil + } + + app := New(Config{Source: "headless", Version: "test"}) + _, err := app.runChildBootstrap(context.Background(), 20*time.Millisecond, childCancel, boot) + if err == nil || !strings.Contains(err.Error(), "not received within") { + t.Fatalf("expected bootstrap timeout error, got %v", err) + } + select { + case <-cancelCalled: + case <-time.After(2 * time.Second): + t.Fatal("child cancel was not invoked on bootstrap timeout") + } +} + +func TestRunChildBootstrapHonorsParentCancel(t *testing.T) { + block := make(chan struct{}) + defer close(block) + ctx, cancelParent := context.WithCancel(context.Background()) + cancelParent() + cancelCalled := make(chan struct{}) + app := New(Config{Source: "headless", Version: "test"}) + _, err := app.runChildBootstrap(ctx, wrapperBootstrapTimeout, func() { close(cancelCalled) }, func() (io.Reader, error) { + <-block + return nil, errors.New("bootstrap should not complete") + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got %v", err) + } + select { + case <-cancelCalled: + case <-time.After(2 * time.Second): + t.Fatal("child cancel was not invoked on parent cancellation") + } +}