Merge pull request #79 from thaJeztah/18.09_backport_bugfix_issue_37870

[18.09 backport] bugfix: wait for stdin creation before CloseIO

integration/container/exec_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"io/ioutil"
 	"testing"
+	"time"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/strslice"
@@ -15,6 +16,74 @@ import (
 	"gotest.tools/skip"
 )
 
+// TestExecWithCloseStdin adds case for moby#37870 issue.
+func TestExecWithCloseStdin(t *testing.T) {
+	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.39"), "broken in earlier versions")
+	defer setupTest(t)()
+
+	ctx := context.Background()
+	client := request.NewAPIClient(t)
+
+	// run top with detached mode
+	cID := container.Run(t, ctx, client)
+
+	expected := "closeIO"
+	execResp, err := client.ContainerExecCreate(ctx, cID,
+		types.ExecConfig{
+			AttachStdin:  true,
+			AttachStdout: true,
+			Cmd:          strslice.StrSlice([]string{"sh", "-c", "cat && echo " + expected}),
+		},
+	)
+	assert.NilError(t, err)
+
+	resp, err := client.ContainerExecAttach(ctx, execResp.ID,
+		types.ExecStartCheck{
+			Detach: false,
+			Tty:    false,
+		},
+	)
+	assert.NilError(t, err)
+	defer resp.Close()
+
+	// close stdin to send EOF to cat
+	assert.NilError(t, resp.CloseWrite())
+
+	var (
+		waitCh = make(chan struct{})
+		resCh  = make(chan struct {
+			content string
+			err     error
+		})
+	)
+
+	go func() {
+		close(waitCh)
+		defer close(resCh)
+		r, err := ioutil.ReadAll(resp.Reader)
+
+		resCh <- struct {
+			content string
+			err     error
+		}{
+			content: string(r),
+			err:     err,
+		}
+	}()
+
+	<-waitCh
+	select {
+	case <-time.After(3 * time.Second):
+		t.Fatal("failed to read the content in time")
+	case got := <-resCh:
+		assert.NilError(t, got.err)
+
+		// NOTE: using Contains because no-tty's stream contains UX information
+		// like size, stream type.
+		assert.Assert(t, is.Contains(got.content, expected))
+	}
+}
+
 func TestExec(t *testing.T) {
 	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.35"), "broken in earlier versions")
 	defer setupTest(t)()

libcontainerd/client_daemon.go

@@ -328,6 +328,13 @@ func (c *client) Start(ctx context.Context, id, checkpointDir string, withStdin
 	return int(t.Pid()), nil
 }
 
+// Exec creates exec process.
+//
+// The containerd client calls Exec to register the exec config in the shim side.
+// When the client calls Start, the shim will create stdin fifo if needs. But
+// for the container main process, the stdin fifo will be created in Create not
+// the Start call. stdinCloseSync channel should be closed after Start exec
+// process.
 func (c *client) Exec(ctx context.Context, containerID, processID string, spec *specs.Process, withStdin bool, attachStdio StdioCallback) (int, error) {
 	ctr := c.getContainer(containerID)
 	if ctr == nil {
@@ -372,7 +379,9 @@ func (c *client) Exec(ctx context.Context, containerID, processID string, spec *
 	ctr.addProcess(processID, p)
 
 	// Signal c.createIO that it can call CloseIO
-	close(stdinCloseSync)
+	//
+	// the stdin of exec process will be created after p.Start in containerd
+	defer close(stdinCloseSync)
 
 	if err = p.Start(ctx); err != nil {
 		p.Delete(context.Background())