Merge pull request #14899 from mountkin/fix-exec-resize-panic

fix the panic caused by resizing a starting exec

daemon/container.go

@@ -812,8 +812,6 @@ func (container *Container) Exec(execConfig *execConfig) error {
 	container.Lock()
 	defer container.Unlock()
 
-	waitStart := make(chan struct{})
-
 	callback := func(processConfig *execdriver.ProcessConfig, pid int) {
 		if processConfig.Tty {
 			// The callback is called after the process Start()
@@ -823,7 +821,7 @@ func (container *Container) Exec(execConfig *execConfig) error {
 				c.Close()
 			}
 		}
-		close(waitStart)
+		close(execConfig.waitStart)
 	}
 
 	// We use a callback here instead of a goroutine and an chan for
@@ -832,7 +830,7 @@ func (container *Container) Exec(execConfig *execConfig) error {
 
 	// Exec should not return until the process is actually running
 	select {
-	case <-waitStart:
+	case <-execConfig.waitStart:
 	case err := <-cErr:
 		return err
 	}

daemon/exec.go

@@ -29,6 +29,9 @@ type execConfig struct {
 	OpenStdout bool
 	Container  *Container
 	canRemove  bool
+
+	// waitStart will be closed immediately after the exec is really started.
+	waitStart chan struct{}
 }
 
 type execStore struct {
@@ -70,6 +73,11 @@ func (e *execStore) List() []string {
 }
 
 func (execConfig *execConfig) Resize(h, w int) error {
+	select {
+	case <-execConfig.waitStart:
+	case <-time.After(time.Second):
+		return fmt.Errorf("Exec %s is not running, so it can not be resized.", execConfig.ID)
+	}
 	return execConfig.ProcessConfig.Terminal.Resize(h, w)
 }
 
@@ -146,6 +154,7 @@ func (d *Daemon) ContainerExecCreate(config *runconfig.ExecConfig) (string, erro
 		ProcessConfig: processConfig,
 		Container:     container,
 		Running:       false,
+		waitStart:     make(chan struct{}),
 	}
 
 	d.registerExecCommand(execConfig)

integration-cli/docker_api_exec_resize_test.go

@@ -1,6 +1,10 @@
 package main
 
 import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
 	"net/http"
 	"strings"
 
@@ -16,3 +20,59 @@ func (s *DockerSuite) TestExecResizeApiHeightWidthNoInt(c *check.C) {
 	c.Assert(status, check.Equals, http.StatusInternalServerError)
 	c.Assert(err, check.IsNil)
 }
+
+// Part of #14845
+func (s *DockerSuite) TestExecResizeImmediatelyAfterExecStart(c *check.C) {
+	name := "exec_resize_test"
+	dockerCmd(c, "run", "-d", "-i", "-t", "--name", name, "--restart", "always", "busybox", "/bin/sh")
+
+	// The panic happens when daemon.ContainerExecStart is called but the
+	// container.Exec is not called.
+	// Because the panic is not 100% reproducible, we send the requests concurrently
+	// to increase the probability that the problem is triggered.
+	n := 10
+	ch := make(chan struct{})
+	for i := 0; i < n; i++ {
+		go func() {
+			defer func() {
+				ch <- struct{}{}
+			}()
+
+			data := map[string]interface{}{
+				"AttachStdin": true,
+				"Cmd":         []string{"/bin/sh"},
+			}
+			status, body, err := sockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), data)
+			c.Assert(err, check.IsNil)
+			c.Assert(status, check.Equals, http.StatusCreated)
+
+			out := map[string]string{}
+			err = json.Unmarshal(body, &out)
+			c.Assert(err, check.IsNil)
+
+			execID := out["Id"]
+			if len(execID) < 1 {
+				c.Fatal("ExecCreate got invalid execID")
+			}
+
+			payload := bytes.NewBufferString(`{"Tty":true}`)
+			conn, _, err := sockRequestHijack("POST", fmt.Sprintf("/exec/%s/start", execID), payload, "application/json")
+			c.Assert(err, check.IsNil)
+			defer conn.Close()
+
+			_, rc, err := sockRequestRaw("POST", fmt.Sprintf("/exec/%s/resize?h=24&w=80", execID), nil, "text/plain")
+			// It's probably a panic of the daemon if io.ErrUnexpectedEOF is returned.
+			if err == io.ErrUnexpectedEOF {
+				c.Fatal("The daemon might have crashed.")
+			}
+
+			if err == nil {
+				rc.Close()
+			}
+		}()
+	}
+
+	for i := 0; i < n; i++ {
+		<-ch
+	}
+}

integration-cli/docker_utils.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bufio"
 	"bytes"
 	"encoding/json"
 	"errors"
@@ -354,6 +355,36 @@ func sockRequest(method, endpoint string, data interface{}) (int, []byte, error)
 }
 
 func sockRequestRaw(method, endpoint string, data io.Reader, ct string) (*http.Response, io.ReadCloser, error) {
+	req, client, err := newRequestClient(method, endpoint, data, ct)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	resp, err := client.Do(req)
+	if err != nil {
+		client.Close()
+		return nil, nil, err
+	}
+	body := ioutils.NewReadCloserWrapper(resp.Body, func() error {
+		defer resp.Body.Close()
+		return client.Close()
+	})
+
+	return resp, body, nil
+}
+
+func sockRequestHijack(method, endpoint string, data io.Reader, ct string) (net.Conn, *bufio.Reader, error) {
+	req, client, err := newRequestClient(method, endpoint, data, ct)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	client.Do(req)
+	conn, br := client.Hijack()
+	return conn, br, nil
+}
+
+func newRequestClient(method, endpoint string, data io.Reader, ct string) (*http.Request, *httputil.ClientConn, error) {
 	c, err := sockConn(time.Duration(10 * time.Second))
 	if err != nil {
 		return nil, nil, fmt.Errorf("could not dial docker daemon: %v", err)
@@ -370,18 +401,7 @@ func sockRequestRaw(method, endpoint string, data io.Reader, ct string) (*http.R
 	if ct != "" {
 		req.Header.Set("Content-Type", ct)
 	}
-
-	resp, err := client.Do(req)
-	if err != nil {
-		client.Close()
-		return nil, nil, fmt.Errorf("could not perform request: %v", err)
-	}
-	body := ioutils.NewReadCloserWrapper(resp.Body, func() error {
-		defer resp.Body.Close()
-		return client.Close()
-	})
-
-	return resp, body, nil
+	return req, client, nil
 }
 
 func readBody(b io.ReadCloser) ([]byte, error) {