Browse code

Fix panic on starting exec more than once

Issue was caused when exec is tarted, exits, then stated again.
In this case, `Close` is called twice, which closes a channel twice.

Changes execConfig.ExitCode to a pointer so we can test if the it has
been set or not.
This allows us to return early when the exec has already been run.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2016/01/16 01:57:23
Showing 4 changed files
... ...
@@ -135,6 +135,11 @@ func (d *Daemon) ContainerExecStart(name string, stdin io.ReadCloser, stdout io.
135 135
 	}
136 136
 
137 137
 	ec.Lock()
138
+	if ec.ExitCode != nil {
139
+		ec.Unlock()
140
+		return derr.ErrorCodeExecExited.WithArgs(ec.ID)
141
+	}
142
+
138 143
 	if ec.Running {
139 144
 		ec.Unlock()
140 145
 		return derr.ErrorCodeExecRunning.WithArgs(ec.ID)
... ...
@@ -214,7 +219,7 @@ func (d *Daemon) Exec(c *container.Container, execConfig *exec.Config, pipes *ex
214 214
 		exitStatus = 128
215 215
 	}
216 216
 
217
-	execConfig.ExitCode = exitStatus
217
+	execConfig.ExitCode = &exitStatus
218 218
 	execConfig.Running = false
219 219
 
220 220
 	return exitStatus, err
... ...
@@ -18,7 +18,7 @@ type Config struct {
18 18
 	*runconfig.StreamConfig
19 19
 	ID            string
20 20
 	Running       bool
21
-	ExitCode      int
21
+	ExitCode      *int
22 22
 	ProcessConfig *execdriver.ProcessConfig
23 23
 	OpenStdin     bool
24 24
 	OpenStderr    bool
... ...
@@ -742,6 +742,15 @@ var (
742 742
 		HTTPStatusCode: http.StatusInternalServerError,
743 743
 	})
744 744
 
745
+	// ErrorCodeExecExited is generated when we try to start an exec
746
+	// but its already running.
747
+	ErrorCodeExecExited = errcode.Register(errGroup, errcode.ErrorDescriptor{
748
+		Value:          "EXECEXITED",
749
+		Message:        "Error: Exec command %s has already run",
750
+		Description:    "An attempt to start an 'exec' was made, but 'exec' was already run",
751
+		HTTPStatusCode: http.StatusConflict,
752
+	})
753
+
745 754
 	// ErrorCodeExecCantRun is generated when we try to start an exec
746 755
 	// but it failed for some reason.
747 756
 	ErrorCodeExecCantRun = errcode.Register(errGroup, errcode.ErrorDescriptor{
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"fmt"
9 9
 	"net/http"
10 10
 	"strings"
11
+	"time"
11 12
 
12 13
 	"github.com/docker/docker/pkg/integration/checker"
13 14
 	"github.com/go-check/check"
... ...
@@ -66,33 +67,23 @@ func (s *DockerSuite) TestExecAPIStart(c *check.C) {
66 66
 	testRequires(c, DaemonIsLinux) // Uses pause/unpause but bits may be salvagable to Windows to Windows CI
67 67
 	dockerCmd(c, "run", "-d", "--name", "test", "busybox", "top")
68 68
 
69
-	startExec := func(id string, code int) {
70
-		resp, body, err := sockRequestRaw("POST", fmt.Sprintf("/exec/%s/start", id), strings.NewReader(`{"Detach": true}`), "application/json")
71
-		c.Assert(err, checker.IsNil)
72
-
73
-		b, err := readBody(body)
74
-		comment := check.Commentf("response body: %s", b)
75
-		c.Assert(err, checker.IsNil, comment)
76
-		c.Assert(resp.StatusCode, checker.Equals, code, comment)
77
-	}
78
-
79 69
 	id := createExec(c, "test")
80
-	startExec(id, http.StatusOK)
70
+	startExec(c, id, http.StatusOK)
81 71
 
82 72
 	id = createExec(c, "test")
83 73
 	dockerCmd(c, "stop", "test")
84 74
 
85
-	startExec(id, http.StatusNotFound)
75
+	startExec(c, id, http.StatusNotFound)
86 76
 
87 77
 	dockerCmd(c, "start", "test")
88
-	startExec(id, http.StatusNotFound)
78
+	startExec(c, id, http.StatusNotFound)
89 79
 
90 80
 	// make sure exec is created before pausing
91 81
 	id = createExec(c, "test")
92 82
 	dockerCmd(c, "pause", "test")
93
-	startExec(id, http.StatusConflict)
83
+	startExec(c, id, http.StatusConflict)
94 84
 	dockerCmd(c, "unpause", "test")
95
-	startExec(id, http.StatusOK)
85
+	startExec(c, id, http.StatusOK)
96 86
 }
97 87
 
98 88
 func (s *DockerSuite) TestExecAPIStartBackwardsCompatible(c *check.C) {
... ...
@@ -108,6 +99,30 @@ func (s *DockerSuite) TestExecAPIStartBackwardsCompatible(c *check.C) {
108 108
 	c.Assert(resp.StatusCode, checker.Equals, http.StatusOK, comment)
109 109
 }
110 110
 
111
+// #19362
112
+func (s *DockerSuite) TestExecAPIStartMultipleTimesError(c *check.C) {
113
+	dockerCmd(c, "run", "-d", "--name", "test", "busybox", "top")
114
+	execID := createExec(c, "test")
115
+	startExec(c, execID, http.StatusOK)
116
+
117
+	timeout := time.After(10 * time.Second)
118
+	var execJSON struct{ Running bool }
119
+	for {
120
+		select {
121
+		case <-timeout:
122
+			c.Fatal("timeout waiting for exec to start")
123
+		default:
124
+		}
125
+
126
+		inspectExec(c, execID, &execJSON)
127
+		if !execJSON.Running {
128
+			break
129
+		}
130
+	}
131
+
132
+	startExec(c, execID, http.StatusConflict)
133
+}
134
+
111 135
 func createExec(c *check.C, name string) string {
112 136
 	_, b, err := sockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), map[string]interface{}{"Cmd": []string{"true"}})
113 137
 	c.Assert(err, checker.IsNil, check.Commentf(string(b)))
... ...
@@ -118,3 +133,22 @@ func createExec(c *check.C, name string) string {
118 118
 	c.Assert(json.Unmarshal(b, &createResp), checker.IsNil, check.Commentf(string(b)))
119 119
 	return createResp.ID
120 120
 }
121
+
122
+func startExec(c *check.C, id string, code int) {
123
+	resp, body, err := sockRequestRaw("POST", fmt.Sprintf("/exec/%s/start", id), strings.NewReader(`{"Detach": true}`), "application/json")
124
+	c.Assert(err, checker.IsNil)
125
+
126
+	b, err := readBody(body)
127
+	comment := check.Commentf("response body: %s", b)
128
+	c.Assert(err, checker.IsNil, comment)
129
+	c.Assert(resp.StatusCode, checker.Equals, code, comment)
130
+}
131
+
132
+func inspectExec(c *check.C, id string, out interface{}) {
133
+	resp, body, err := sockRequestRaw("GET", fmt.Sprintf("/exec/%s/json", id), nil, "")
134
+	c.Assert(err, checker.IsNil)
135
+	defer body.Close()
136
+	c.Assert(resp.StatusCode, checker.Equals, http.StatusOK)
137
+	err = json.NewDecoder(body).Decode(out)
138
+	c.Assert(err, checker.IsNil)
139
+}