Skip to content

Commit

Permalink
sys/reaper: avoid leaky goroutine when exec timeout
Browse files Browse the repository at this point in the history
The channel is created with no capacity that it needs receiver when
sending data. Otherwise, the sending-data goroutine will be blocked
forever. For the #6166 pr, the exec command timeout will return and
no receiver for the data. It will cause goroutine leaky.

This commit allocates buffered channel for the command status and closes
the channel after sending. And also use time.Timer with Stop for
performance concern.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
  • Loading branch information
fuweid committed Oct 31, 2021
1 parent d0bdb0b commit 6ee8577
Showing 1 changed file with 21 additions and 11 deletions.
32 changes: 21 additions & 11 deletions sys/reaper/reaper_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,23 +119,33 @@ func (m *Monitor) Wait(c *exec.Cmd, ec chan runc.Exit) (int, error) {

// WaitTimeout is used to skip the blocked command and kill the left process.
func (m *Monitor) WaitTimeout(c *exec.Cmd, ec chan runc.Exit, timeout time.Duration) (int, error) {
sch := make(chan int)
ech := make(chan error)
type exitStatusWrapper struct {
status int
err error
}

// capacity can make sure that the following goroutine will not be
// blocked if there is no receiver when timeout.
waitCh := make(chan *exitStatusWrapper, 1)
go func() {
defer close(waitCh)

status, err := m.Wait(c, ec)
sch <- status
if err != nil {
ech <- err
waitCh <- &exitStatusWrapper{
status: status,
err: err,
}
}()

timer := time.NewTimer(timeout)
defer timer.Stop()

select {
case <-time.After(timeout):
case <-timer.C:
syscall.Kill(c.Process.Pid, syscall.SIGKILL)
return 0, errors.Errorf("timeout %ds for cmd(pid=%d): %s, %s", timeout/time.Second, c.Process.Pid, c.Path, c.Args)
case status := <-sch:
return status, nil
case err := <-ech:
return -1, err
return 0, errors.Errorf("timeout %v for cmd(pid=%d): %s, %s", timeout, c.Process.Pid, c.Path, c.Args)
case res := <-waitCh:
return res.status, res.err
}
}

Expand Down

0 comments on commit 6ee8577

Please sign in to comment.