diff --git a/daemon/container.go b/daemon/container.go index 0b19034b04..ddfa6e5427 100644 --- a/daemon/container.go +++ b/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 } diff --git a/daemon/exec.go b/daemon/exec.go index 790fad5d73..54b3e1d527 100644 --- a/daemon/exec.go +++ b/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) diff --git a/integration-cli/docker_api_exec_resize_test.go b/integration-cli/docker_api_exec_resize_test.go index 01061ca67f..924a77a24b 100644 --- a/integration-cli/docker_api_exec_resize_test.go +++ b/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 + } +} diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index 5f5729d4fd..c6ed51f5fc 100644 --- a/integration-cli/docker_utils.go +++ b/integration-cli/docker_utils.go @@ -1,6 +1,7 @@ package main import ( + "bufio" "bytes" "encoding/json" "errors" @@ -347,6 +348,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) @@ -363,18 +394,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) {