From 43eccfaee2c9dbe5757f50e0d47ae14e38454723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 3 Mar 2026 19:54:12 +0000 Subject: [PATCH] feat: improve closing resources (PTYs, connections) (#355) --- .../agent/lifecycle/manager_interaction.go | 14 ++++++++------ apps/backend/internal/agentctl/client/client.go | 3 ++- .../internal/agentctl/server/process/manager.go | 3 +++ .../internal/agentctl/server/process/runner.go | 3 +++ .../internal/agentctl/server/shell/session.go | 8 ++++++++ 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/apps/backend/internal/agent/lifecycle/manager_interaction.go b/apps/backend/internal/agent/lifecycle/manager_interaction.go index fae80b04d1..23818a8fdf 100644 --- a/apps/backend/internal/agent/lifecycle/manager_interaction.go +++ b/apps/backend/internal/agent/lifecycle/manager_interaction.go @@ -90,12 +90,14 @@ func (m *Manager) StopAgentWithReason(ctx context.Context, executionID string, r zap.Bool("force", force), zap.String("runtime", execution.RuntimeName)) - // Try to gracefully stop via agentctl first - if execution.agentctl != nil && !force { - if err := execution.agentctl.Stop(ctx); err != nil { - m.logger.Warn("failed to stop agent via agentctl", - zap.String("execution_id", executionID), - zap.Error(err)) + // Try to gracefully stop via agentctl first, then always close connections + if execution.agentctl != nil { + if !force { + if err := execution.agentctl.Stop(ctx); err != nil { + m.logger.Warn("failed to stop agent via agentctl", + zap.String("execution_id", executionID), + zap.Error(err)) + } } execution.agentctl.Close() } diff --git a/apps/backend/internal/agentctl/client/client.go b/apps/backend/internal/agentctl/client/client.go index 2e7a4eabc9..a8cc12aabf 100644 --- a/apps/backend/internal/agentctl/client/client.go +++ b/apps/backend/internal/agentctl/client/client.go @@ -532,10 +532,11 @@ type ( ProcessStatusUpdate = types.ProcessStatusUpdate ) -// Close closes all connections +// Close closes all connections and releases resources func (c *Client) Close() { c.CloseUpdatesStream() c.CloseWorkspaceStream() + c.httpClient.CloseIdleConnections() } // readResponseBody reads and returns the response body diff --git a/apps/backend/internal/agentctl/server/process/manager.go b/apps/backend/internal/agentctl/server/process/manager.go index 85e999e02c..2e1a54380b 100644 --- a/apps/backend/internal/agentctl/server/process/manager.go +++ b/apps/backend/internal/agentctl/server/process/manager.go @@ -412,10 +412,13 @@ func (m *Manager) startProcessPipes() error { } m.stdout, err = m.cmd.StdoutPipe() if err != nil { + _ = m.stdin.Close() return fmt.Errorf("failed to create stdout pipe: %w", err) } m.stderr, err = m.cmd.StderrPipe() if err != nil { + _ = m.stdin.Close() + _ = m.stdout.Close() return fmt.Errorf("failed to create stderr pipe: %w", err) } return nil diff --git a/apps/backend/internal/agentctl/server/process/runner.go b/apps/backend/internal/agentctl/server/process/runner.go index 1a758453dc..a9b83afa7f 100644 --- a/apps/backend/internal/agentctl/server/process/runner.go +++ b/apps/backend/internal/agentctl/server/process/runner.go @@ -245,6 +245,7 @@ func (r *ProcessRunner) Start(ctx context.Context, req StartProcessRequest) (*Pr } stderr, err := cmd.StderrPipe() if err != nil { + _ = stdout.Close() return nil, fmt.Errorf("failed to attach stderr: %w", err) } @@ -296,6 +297,8 @@ func (r *ProcessRunner) Start(ctx context.Context, req StartProcessRequest) (*Pr // On failure, marks the process as failed and removes it from tracking. func (r *ProcessRunner) startAndActivate(proc *commandProcess, cmd *exec.Cmd, id string, stdout, stderr io.ReadCloser) error { if err := cmd.Start(); err != nil { + _ = stdout.Close() + _ = stderr.Close() proc.mu.Lock() proc.info.Status = types.ProcessStatusFailed proc.info.UpdatedAt = time.Now().UTC() diff --git a/apps/backend/internal/agentctl/server/shell/session.go b/apps/backend/internal/agentctl/server/shell/session.go index 6520b42fe9..bc5684b361 100644 --- a/apps/backend/internal/agentctl/server/shell/session.go +++ b/apps/backend/internal/agentctl/server/shell/session.go @@ -379,6 +379,14 @@ func (s *Session) respawn() error { return nil } + // Close old PTY to prevent file descriptor leak. + // Don't nil out s.pty — readOutput() may still be reading from it + // concurrently without the mutex. Closing is sufficient: any in-flight + // Read() will return an error and the goroutine will exit. + if s.pty != nil { + _ = s.pty.Close() + } + s.cmd = exec.Command(s.shell, s.shellArgs...) s.cmd.Dir = s.workDir s.cmd.Env = buildShellEnv(s.workDir)