Skip to content

Commit 4e24efa

Browse files
committed
fix(envd): fix EndEvent delivery race when process is killed with orphan children
After cmd.Wait() reaps the child, use SetReadDeadline on the pipe read-ends so readers drain any buffered data (reads with available data return instantly) then exit on deadline instead of blocking forever when an orphan grandchild holds the write-end open. Readers treat the deadline timeout the same as EOF — clean exit, no data loss. Add a dedicated readersDone channel that closes when stdout/stderr reader goroutines actually exit, replacing the outCtx/outCancel mechanism which is no longer needed. Fixes TestStart_OrphanGrandchildDoesNotHangStream and the TestCommandKillNextApp CI failure.
1 parent abb4404 commit 4e24efa

1 file changed

Lines changed: 21 additions & 34 deletions

File tree

  • packages/envd/internal/services/process/handler

packages/envd/internal/services/process/handler/handler.go

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ type Handler struct {
5151

5252
cancel context.CancelFunc
5353

54-
outCtx context.Context //nolint:containedctx // todo: refactor so this can be removed
55-
outCancel context.CancelFunc
54+
readersDone chan struct{} // closed when stdout/stderr reader goroutines have exited
5655

5756
stdinMu sync.Mutex
5857
stdin io.WriteCloser
@@ -173,20 +172,15 @@ func New(
173172

174173
var outWg sync.WaitGroup
175174

176-
// Create a context for waiting for and cancelling output pipes.
177-
// Cancellation of the process via timeout will propagate and cancel this context too.
178-
outCtx, outCancel := context.WithCancel(ctx)
179-
180175
h := &Handler{
181-
Config: req.GetProcess(),
182-
cmd: cmd,
183-
Tag: req.Tag,
184-
DataEvent: outMultiplex,
185-
cancel: cancel,
186-
outCtx: outCtx,
187-
outCancel: outCancel,
188-
EndEvent: NewMultiplexedChannel[rpc.ProcessEvent_End](1),
189-
logger: logger,
176+
Config: req.GetProcess(),
177+
cmd: cmd,
178+
Tag: req.Tag,
179+
DataEvent: outMultiplex,
180+
cancel: cancel,
181+
readersDone: make(chan struct{}),
182+
EndEvent: NewMultiplexedChannel[rpc.ProcessEvent_End](1),
183+
logger: logger,
190184
}
191185

192186
if req.GetPty() != nil {
@@ -267,7 +261,7 @@ func New(
267261
stdoutLogs <- buf[:n]
268262
}
269263

270-
if errors.Is(readErr, io.EOF) {
264+
if errors.Is(readErr, io.EOF) || os.IsTimeout(readErr) {
271265
break
272266
}
273267

@@ -316,7 +310,7 @@ func New(
316310
stderrLogs <- buf[:n]
317311
}
318312

319-
if errors.Is(readErr, io.EOF) {
313+
if errors.Is(readErr, io.EOF) || os.IsTimeout(readErr) {
320314
break
321315
}
322316

@@ -343,9 +337,9 @@ func New(
343337
go func() {
344338
outWg.Wait()
345339

346-
outMultiplex.CloseSource()
340+
close(h.readersDone)
347341

348-
outCancel()
342+
outMultiplex.CloseSource()
349343
}()
350344

351345
return h, nil
@@ -364,10 +358,6 @@ func (p *Handler) SendSignal(signal syscall.Signal) error {
364358
return errors.New("process not started")
365359
}
366360

367-
if signal == syscall.SIGKILL || signal == syscall.SIGTERM {
368-
p.outCancel()
369-
}
370-
371361
return p.cmd.Process.Signal(signal)
372362
}
373363

@@ -473,19 +463,16 @@ func (p *Handler) Wait() {
473463
// send unblocks, loops back, and sees EOF.
474464
p.DataEvent.Drain()
475465

476-
// Wait for readers to finish. If they don't exit promptly
477-
// (orphan grandchildren keeping the pipe open), close the
478-
// read-ends to force them out.
479-
select {
480-
case <-p.outCtx.Done():
481-
case <-time.After(5 * time.Second):
482-
for _, f := range p.pipeRead {
483-
f.Close()
484-
}
485-
486-
<-p.outCtx.Done()
466+
// Set a read deadline on the pipe read-ends so readers drain
467+
// any buffered data (reads with available data return instantly)
468+
// and then exit cleanly instead of blocking forever when an
469+
// orphan grandchild holds the write-end open.
470+
for _, f := range p.pipeRead {
471+
f.SetReadDeadline(time.Now().Add(1 * time.Second))
487472
}
488473

474+
<-p.readersDone
475+
489476
p.tty.Close()
490477

491478
var errMsg *string

0 commit comments

Comments
 (0)