Browse code

fix the panic caused by resizing a starting exec

Signed-off-by: Shijiang Wei <mountkin@gmail.com>

Shijiang Wei authored on 2015/07/23 20:34:40
Showing 4 changed files
... ...
@@ -812,8 +812,6 @@ func (container *Container) Exec(execConfig *execConfig) error {
812 812
 	container.Lock()
813 813
 	defer container.Unlock()
814 814
 
815
-	waitStart := make(chan struct{})
816
-
817 815
 	callback := func(processConfig *execdriver.ProcessConfig, pid int) {
818 816
 		if processConfig.Tty {
819 817
 			// The callback is called after the process Start()
... ...
@@ -823,7 +821,7 @@ func (container *Container) Exec(execConfig *execConfig) error {
823 823
 				c.Close()
824 824
 			}
825 825
 		}
826
-		close(waitStart)
826
+		close(execConfig.waitStart)
827 827
 	}
828 828
 
829 829
 	// We use a callback here instead of a goroutine and an chan for
... ...
@@ -832,7 +830,7 @@ func (container *Container) Exec(execConfig *execConfig) error {
832 832
 
833 833
 	// Exec should not return until the process is actually running
834 834
 	select {
835
-	case <-waitStart:
835
+	case <-execConfig.waitStart:
836 836
 	case err := <-cErr:
837 837
 		return err
838 838
 	}
... ...
@@ -29,6 +29,9 @@ type execConfig struct {
29 29
 	OpenStdout bool
30 30
 	Container  *Container
31 31
 	canRemove  bool
32
+
33
+	// waitStart will be closed immediately after the exec is really started.
34
+	waitStart chan struct{}
32 35
 }
33 36
 
34 37
 type execStore struct {
... ...
@@ -70,6 +73,11 @@ func (e *execStore) List() []string {
70 70
 }
71 71
 
72 72
 func (execConfig *execConfig) Resize(h, w int) error {
73
+	select {
74
+	case <-execConfig.waitStart:
75
+	case <-time.After(time.Second):
76
+		return fmt.Errorf("Exec %s is not running, so it can not be resized.", execConfig.ID)
77
+	}
73 78
 	return execConfig.ProcessConfig.Terminal.Resize(h, w)
74 79
 }
75 80
 
... ...
@@ -146,6 +154,7 @@ func (d *Daemon) ContainerExecCreate(config *runconfig.ExecConfig) (string, erro
146 146
 		ProcessConfig: processConfig,
147 147
 		Container:     container,
148 148
 		Running:       false,
149
+		waitStart:     make(chan struct{}),
149 150
 	}
150 151
 
151 152
 	d.registerExecCommand(execConfig)
... ...
@@ -1,6 +1,10 @@
1 1
 package main
2 2
 
3 3
 import (
4
+	"bytes"
5
+	"encoding/json"
6
+	"fmt"
7
+	"io"
4 8
 	"net/http"
5 9
 	"strings"
6 10
 
... ...
@@ -16,3 +20,59 @@ func (s *DockerSuite) TestExecResizeApiHeightWidthNoInt(c *check.C) {
16 16
 	c.Assert(status, check.Equals, http.StatusInternalServerError)
17 17
 	c.Assert(err, check.IsNil)
18 18
 }
19
+
20
+// Part of #14845
21
+func (s *DockerSuite) TestExecResizeImmediatelyAfterExecStart(c *check.C) {
22
+	name := "exec_resize_test"
23
+	dockerCmd(c, "run", "-d", "-i", "-t", "--name", name, "--restart", "always", "busybox", "/bin/sh")
24
+
25
+	// The panic happens when daemon.ContainerExecStart is called but the
26
+	// container.Exec is not called.
27
+	// Because the panic is not 100% reproducible, we send the requests concurrently
28
+	// to increase the probability that the problem is triggered.
29
+	n := 10
30
+	ch := make(chan struct{})
31
+	for i := 0; i < n; i++ {
32
+		go func() {
33
+			defer func() {
34
+				ch <- struct{}{}
35
+			}()
36
+
37
+			data := map[string]interface{}{
38
+				"AttachStdin": true,
39
+				"Cmd":         []string{"/bin/sh"},
40
+			}
41
+			status, body, err := sockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), data)
42
+			c.Assert(err, check.IsNil)
43
+			c.Assert(status, check.Equals, http.StatusCreated)
44
+
45
+			out := map[string]string{}
46
+			err = json.Unmarshal(body, &out)
47
+			c.Assert(err, check.IsNil)
48
+
49
+			execID := out["Id"]
50
+			if len(execID) < 1 {
51
+				c.Fatal("ExecCreate got invalid execID")
52
+			}
53
+
54
+			payload := bytes.NewBufferString(`{"Tty":true}`)
55
+			conn, _, err := sockRequestHijack("POST", fmt.Sprintf("/exec/%s/start", execID), payload, "application/json")
56
+			c.Assert(err, check.IsNil)
57
+			defer conn.Close()
58
+
59
+			_, rc, err := sockRequestRaw("POST", fmt.Sprintf("/exec/%s/resize?h=24&w=80", execID), nil, "text/plain")
60
+			// It's probably a panic of the daemon if io.ErrUnexpectedEOF is returned.
61
+			if err == io.ErrUnexpectedEOF {
62
+				c.Fatal("The daemon might have crashed.")
63
+			}
64
+
65
+			if err == nil {
66
+				rc.Close()
67
+			}
68
+		}()
69
+	}
70
+
71
+	for i := 0; i < n; i++ {
72
+		<-ch
73
+	}
74
+}
... ...
@@ -1,6 +1,7 @@
1 1
 package main
2 2
 
3 3
 import (
4
+	"bufio"
4 5
 	"bytes"
5 6
 	"encoding/json"
6 7
 	"errors"
... ...
@@ -347,6 +348,36 @@ func sockRequest(method, endpoint string, data interface{}) (int, []byte, error)
347 347
 }
348 348
 
349 349
 func sockRequestRaw(method, endpoint string, data io.Reader, ct string) (*http.Response, io.ReadCloser, error) {
350
+	req, client, err := newRequestClient(method, endpoint, data, ct)
351
+	if err != nil {
352
+		return nil, nil, err
353
+	}
354
+
355
+	resp, err := client.Do(req)
356
+	if err != nil {
357
+		client.Close()
358
+		return nil, nil, err
359
+	}
360
+	body := ioutils.NewReadCloserWrapper(resp.Body, func() error {
361
+		defer resp.Body.Close()
362
+		return client.Close()
363
+	})
364
+
365
+	return resp, body, nil
366
+}
367
+
368
+func sockRequestHijack(method, endpoint string, data io.Reader, ct string) (net.Conn, *bufio.Reader, error) {
369
+	req, client, err := newRequestClient(method, endpoint, data, ct)
370
+	if err != nil {
371
+		return nil, nil, err
372
+	}
373
+
374
+	client.Do(req)
375
+	conn, br := client.Hijack()
376
+	return conn, br, nil
377
+}
378
+
379
+func newRequestClient(method, endpoint string, data io.Reader, ct string) (*http.Request, *httputil.ClientConn, error) {
350 380
 	c, err := sockConn(time.Duration(10 * time.Second))
351 381
 	if err != nil {
352 382
 		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
363 363
 	if ct != "" {
364 364
 		req.Header.Set("Content-Type", ct)
365 365
 	}
366
-
367
-	resp, err := client.Do(req)
368
-	if err != nil {
369
-		client.Close()
370
-		return nil, nil, fmt.Errorf("could not perform request: %v", err)
371
-	}
372
-	body := ioutils.NewReadCloserWrapper(resp.Body, func() error {
373
-		defer resp.Body.Close()
374
-		return client.Close()
375
-	})
376
-
377
-	return resp, body, nil
366
+	return req, client, nil
378 367
 }
379 368
 
380 369
 func readBody(b io.ReadCloser) ([]byte, error) {