Add timeout for I/O waitgroups

crosbymichael · thaJeztah · commit 42aba6e0fe5e · 2019-10-14T17:33:04.000+02:00
Closes #3286 This and a combination of a couple Docker changes are needed to fully resolve the issue on the Docker side. However, this ensures that after processes exit, we still leave some time for the I/O to fully flush before closing. Without this timeout, the delete methods would block forever. Signed-off-by: Michael Crosby <crosbymichael@gmail.com> (cherry picked from commit 2450522) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
diff --git a/runtime/v1/linux/proc/exec.go b/runtime/v1/linux/proc/exec.go
@@ -106,7 +106,7 @@ func (e *execProcess) Delete(ctx context.Context) error {
 }
 
 func (e *execProcess) delete(ctx context.Context) error {
-	e.wg.Wait()
+	waitTimeout(ctx, &e.wg, 2*time.Second)
 	if e.io != nil {
 		for _, c := range e.closers {
 			c.Close()
diff --git a/runtime/v1/linux/proc/init.go b/runtime/v1/linux/proc/init.go
@@ -278,7 +278,7 @@ func (p *Init) Delete(ctx context.Context) error {
 }
 
 func (p *Init) delete(ctx context.Context) error {
-	p.wg.Wait()
+	waitTimeout(ctx, &p.wg, 2*time.Second)
 	err := p.runtime.Delete(ctx, p.id, nil)
 	// ignore errors if a runtime has already deleted the process
 	// but we still hold metadata and pipes
diff --git a/runtime/v1/linux/proc/utils.go b/runtime/v1/linux/proc/utils.go
@@ -19,6 +19,7 @@
 package proc
 
 import (
+	"context"
 	"encoding/json"
 	"io"
 	"os"
@@ -117,3 +118,21 @@ func checkKillError(err error) error {
 func hasNoIO(r *CreateConfig) bool {
 	return r.Stdin == "" && r.Stdout == "" && r.Stderr == ""
 }
+
+// waitTimeout handles waiting on a waitgroup with a specified timeout.
+// this is commonly used for waiting on IO to finish after a process has exited
+func waitTimeout(ctx context.Context, wg *sync.WaitGroup, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+	done := make(chan struct{}, 1)
+	go func() {
+		wg.Wait()
+		close(done)
+	}()
+	select {
+	case <-done:
+		return nil
+	case <-ctx.Done():
+		return ctx.Err()
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ func (e *execProcess) Delete(ctx context.Context) error {`
`106`	`106`	`}`
`107`	`107`
`108`	`108`	`func (e *execProcess) delete(ctx context.Context) error {`
`109`		`- e.wg.Wait()`
	`109`	`+ waitTimeout(ctx, &e.wg, 2*time.Second)`
`110`	`110`	`if e.io != nil {`
`111`	`111`	`for _, c := range e.closers {`
`112`	`112`	`c.Close()`
Original file line number	Diff line number	Diff line change
`@@ -278,7 +278,7 @@ func (p *Init) Delete(ctx context.Context) error {`
`278`	`278`	`}`
`279`	`279`
`280`	`280`	`func (p *Init) delete(ctx context.Context) error {`
`281`		`- p.wg.Wait()`
	`281`	`+ waitTimeout(ctx, &p.wg, 2*time.Second)`
`282`	`282`	`err := p.runtime.Delete(ctx, p.id, nil)`
`283`	`283`	`// ignore errors if a runtime has already deleted the process`
`284`	`284`	`// but we still hold metadata and pipes`