Don't block on STDIN open

crosbymichael · crosbymichael · commit bc1ff5141196 · 2018-08-27T10:44:53.000-04:00
This was found testing other runtime shims that are faster than runc(no
containerization).  This is a race that can cause the shim to block
forever.  It's not an issue for out/err because we open both sides of
the pipe, but for stdin, it expects the client to have it opened.

Signed-off-by: Michael Crosby &lt;crosbymichael@gmail.com&gt;
diff --git a/runtime/v1/linux/proc/exec.go b/runtime/v1/linux/proc/exec.go
@@ -164,10 +164,7 @@ func (e *execProcess) start(ctx context.Context) (err error) {
 		return e.parent.runtimeError(err, "OCI runtime exec failed")
 	}
 	if e.stdio.Stdin != "" {
-		fifoCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
-		defer cancel()
-
-		sc, err := fifo.OpenFifo(fifoCtx, e.stdio.Stdin, syscall.O_WRONLY|syscall.O_NONBLOCK, 0)
+		sc, err := fifo.OpenFifo(ctx, e.stdio.Stdin, syscall.O_WRONLY|syscall.O_NONBLOCK, 0)
 		if err != nil {
 			return errors.Wrapf(err, "failed to open stdin fifo %s", e.stdio.Stdin)
 		}
@@ -184,10 +181,7 @@ func (e *execProcess) start(ctx context.Context) (err error) {
 			return errors.Wrap(err, "failed to start console copy")
 		}
 	} else if !e.stdio.IsNull() {
-		fifoCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
-		defer cancel()
-
-		if err := copyPipes(fifoCtx, e.io, e.stdio.Stdin, e.stdio.Stdout, e.stdio.Stderr, &e.wg, &copyWaitGroup); err != nil {
+		if err := copyPipes(ctx, e.io, e.stdio.Stdin, e.stdio.Stdout, e.stdio.Stderr, &e.wg, &copyWaitGroup); err != nil {
 			return errors.Wrap(err, "failed to start io pipe copy")
 		}
 	}
diff --git a/runtime/v1/linux/proc/io.go b/runtime/v1/linux/proc/io.go
@@ -112,7 +112,7 @@ func copyPipes(ctx context.Context, rio runc.IO, stdin, stdout, stderr string, w
 		rio.Stdin().Close()
 		return nil
 	}
-	f, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0)
+	f, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)
 	if err != nil {
 		return fmt.Errorf("containerd-shim: opening %s failed: %s", stdin, err)
 	}
diff --git a/runtime/v2/runc/service.go b/runtime/v2/runc/service.go
@@ -98,8 +98,7 @@ type service struct {
 	ec        chan runcC.Exit
 	ep        *epoller
 
-	id string
-	// Filled by Create()
+	id     string
 	bundle string
 	cg     cgroups.Cgroup
 }
diff --git a/runtime/v2/runc/service_linux.go b/runtime/v2/runc/service_linux.go
@@ -42,7 +42,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console
 	}
 
 	if stdin != "" {
-		in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0)
+		in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)
 		if err != nil {
 			return nil, err
 		}

Original file line number	Diff line number	Diff line change
`@@ -164,10 +164,7 @@ func (e *execProcess) start(ctx context.Context) (err error) {`
`164`	`164`	`return e.parent.runtimeError(err, "OCI runtime exec failed")`
`165`	`165`	`}`
`166`	`166`	`if e.stdio.Stdin != "" {`
`167`		`- fifoCtx, cancel := context.WithTimeout(ctx, 15*time.Second)`
`168`		`- defer cancel()`
`169`		`-`
`170`		`- sc, err := fifo.OpenFifo(fifoCtx, e.stdio.Stdin, syscall.O_WRONLY\|syscall.O_NONBLOCK, 0)`
	`167`	`+ sc, err := fifo.OpenFifo(ctx, e.stdio.Stdin, syscall.O_WRONLY\|syscall.O_NONBLOCK, 0)`
`171`	`168`	`if err != nil {`
`172`	`169`	`return errors.Wrapf(err, "failed to open stdin fifo %s", e.stdio.Stdin)`
`173`	`170`	`}`
`@@ -184,10 +181,7 @@ func (e *execProcess) start(ctx context.Context) (err error) {`
`184`	`181`	`return errors.Wrap(err, "failed to start console copy")`
`185`	`182`	`}`
`186`	`183`	`} else if !e.stdio.IsNull() {`
`187`		`- fifoCtx, cancel := context.WithTimeout(ctx, 15*time.Second)`
`188`		`- defer cancel()`
`189`		`-`
`190`		`- if err := copyPipes(fifoCtx, e.io, e.stdio.Stdin, e.stdio.Stdout, e.stdio.Stderr, &e.wg, &copyWaitGroup); err != nil {`
	`184`	`+ if err := copyPipes(ctx, e.io, e.stdio.Stdin, e.stdio.Stdout, e.stdio.Stderr, &e.wg, &copyWaitGroup); err != nil {`
`191`	`185`	`return errors.Wrap(err, "failed to start io pipe copy")`
`192`	`186`	`}`
`193`	`187`	`}`
Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,7 @@ func copyPipes(ctx context.Context, rio runc.IO, stdin, stdout, stderr string, w`
`112`	`112`	`rio.Stdin().Close()`
`113`	`113`	`return nil`
`114`	`114`	`}`
`115`		`- f, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0)`
	`115`	`+ f, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY\|syscall.O_NONBLOCK, 0)`
`116`	`116`	`if err != nil {`
`117`	`117`	`return fmt.Errorf("containerd-shim: opening %s failed: %s", stdin, err)`
`118`	`118`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,8 +98,7 @@ type service struct {`
`98`	`98`	`ec chan runcC.Exit`
`99`	`99`	`ep *epoller`
`100`	`100`
`101`		`- id string`
`102`		`- // Filled by Create()`
	`101`	`+ id string`
`103`	`102`	`bundle string`
`104`	`103`	`cg cgroups.Cgroup`
`105`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ func (p *linuxPlatform) CopyConsole(ctx context.Context, console console.Console`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`if stdin != "" {`
`45`		`- in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0)`
	`45`	`+ in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY\|syscall.O_NONBLOCK, 0)`
`46`	`46`	`if err != nil {`
`47`	`47`	`return nil, err`
`48`	`48`	`}`