shimv2: fix the issue of close IO stream

It should wait until the stdin io copy termianted to close the process's io stream, otherwise, it would miss forwarding some contents to process stdin. Fixes: kata-containers#2884 Signed-off-by: fupan.lfp <[email protected]>
lifupan · Aug 11, 2020 · d4d247f · d4d247f
1 parent bdcd2dd
commit d4d247f
Show file tree

Hide file tree

Showing 6 changed files with 1,006 additions and 47 deletions.
diff --git a/containerd-shim-v2/container.go b/containerd-shim-v2/container.go
@@ -6,6 +6,7 @@
 package containerdshim
 
 import (
+	"io"
 	"time"
 
 	"github.com/containerd/containerd/api/types/task"
@@ -17,23 +18,25 @@ import (
 )
 
 type container struct {
-	s        *service
-	ttyio    *ttyIO
-	spec     *specs.Spec
-	exitTime time.Time
-	execs    map[string]*exec
-	exitIOch chan struct{}
-	exitCh   chan uint32
-	id       string
-	stdin    string
-	stdout   string
-	stderr   string
-	bundle   string
-	cType    vc.ContainerType
-	exit     uint32
-	status   task.Status
-	terminal bool
-	mounted  bool
+	s           *service
+	ttyio       *ttyIO
+	spec        *specs.Spec
+	exitTime    time.Time
+	execs       map[string]*exec
+	exitIOch    chan struct{}
+	stdinPipe   io.WriteCloser
+	stdinCloser chan struct{}
+	exitCh      chan uint32
+	id          string
+	stdin       string
+	stdout      string
+	stderr      string
+	bundle      string
+	cType       vc.ContainerType
+	exit        uint32
+	status      task.Status
+	terminal    bool
+	mounted     bool
 }
 
 func newContainer(s *service, r *taskAPI.CreateTaskRequest, containerType vc.ContainerType, spec *specs.Spec, mounted bool) (*container, error) {
@@ -47,20 +50,21 @@ func newContainer(s *service, r *taskAPI.CreateTaskRequest, containerType vc.Con
 	}
 
 	c := &container{
-		s:        s,
-		spec:     spec,
-		id:       r.ID,
-		bundle:   r.Bundle,
-		stdin:    r.Stdin,
-		stdout:   r.Stdout,
-		stderr:   r.Stderr,
-		terminal: r.Terminal,
-		cType:    containerType,
-		execs:    make(map[string]*exec),
-		status:   task.StatusCreated,
-		exitIOch: make(chan struct{}),
-		exitCh:   make(chan uint32, 1),
-		mounted:  mounted,
+		s:           s,
+		spec:        spec,
+		id:          r.ID,
+		bundle:      r.Bundle,
+		stdin:       r.Stdin,
+		stdout:      r.Stdout,
+		stderr:      r.Stderr,
+		terminal:    r.Terminal,
+		cType:       containerType,
+		execs:       make(map[string]*exec),
+		status:      task.StatusCreated,
+		exitIOch:    make(chan struct{}),
+		exitCh:      make(chan uint32, 1),
+		stdinCloser: make(chan struct{}),
+		mounted:     mounted,
 	}
 	return c, nil
 }
diff --git a/containerd-shim-v2/exec.go b/containerd-shim-v2/exec.go
@@ -7,6 +7,7 @@ package containerdshim
 
 import (
 	"fmt"
+	"io"
 	"strings"
 	"time"
 
@@ -32,6 +33,9 @@ type exec struct {
 	exitIOch chan struct{}
 	exitCh   chan uint32
 
+	stdinCloser chan struct{}
+	stdinPipe   io.WriteCloser
+
 	exitTime time.Time
 }
 
@@ -108,13 +112,14 @@ func newExec(c *container, stdin, stdout, stderr string, terminal bool, jspec *g
 	}
 
 	exec := &exec{
-		container: c,
-		cmds:      cmds,
-		tty:       tty,
-		exitCode:  exitCode255,
-		exitIOch:  make(chan struct{}),
-		exitCh:    make(chan uint32, 1),
-		status:    task.StatusCreated,
+		container:   c,
+		cmds:        cmds,
+		tty:         tty,
+		exitCode:    exitCode255,
+		exitIOch:    make(chan struct{}),
+		stdinCloser: make(chan struct{}),
+		exitCh:      make(chan uint32, 1),
+		status:      task.StatusCreated,
 	}
 
 	return exec, nil

diff --git a/containerd-shim-v2/service.go b/containerd-shim-v2/service.go
@@ -725,19 +725,23 @@ func (s *service) CloseIO(ctx context.Context, r *taskAPI.CloseIORequest) (_ *pt
 		return nil, err
 	}
 
-	tty := c.ttyio
+	stdin := c.stdinPipe
+	stdinCloser := c.stdinCloser
+
 	if r.ExecID != "" {
 		execs, err := c.getExec(r.ExecID)
 		if err != nil {
 			return nil, err
 		}
-		tty = execs.ttyio
+		stdin = execs.stdinPipe
+		stdinCloser = execs.stdinCloser
 	}
 
-	if tty != nil && tty.Stdin != nil {
-		if err := tty.Stdin.Close(); err != nil {
-			return nil, errors.Wrap(err, "close stdin")
-		}
+	// wait until the stdin io copy terminated, otherwise
+	// some contents would not be forwarded to the process.
+	<-stdinCloser
+	if err := stdin.Close(); err != nil {
+		return nil, errors.Wrap(err, "close stdin")
 	}
 
 	return empty, nil