Merge pull request #2152 from Ace-Tang/exec_exit

bugfix: avoid data race in set exec exit status

ctrd/container.go

@@ -122,14 +122,13 @@ func (c *Client) execContainer(ctx context.Context, process *Process) error {
  exitTime: status.ExitTime(),
  }
 
- if err := <-fail; err != nil {
- msg.err = err
- }
-
- for _, hook := range c.hooks {
- if err := hook(process.ExecID, msg); err != nil {
- logrus.Errorf("failed to execute the exec exit hooks: %v", err)
- break
+ // run hook if not got fail here
+ if err := <-fail; err == nil {
+ for _, hook := range c.hooks {
+ if err := hook(process.ExecID, msg); err != nil {
+ logrus.Errorf("failed to execute the exec exit hooks: %v", err)
+ break
+ }
  }
  }
 

daemon/mgr/container_exec.go

@@ -77,7 +77,6 @@ func (mgr *ContainerManager) StartExec(ctx context.Context, execid string, attac
  exitCode := 126
  execConfig.ExitCode = int64(exitCode)
  }
- mgr.ExecProcesses.Put(execid, execConfig)
  }()
 
  if attach != nil {

test/cli_exec_test.go

@@ -144,12 +144,23 @@ func (suite *PouchExecSuite) TestExecExitCode(c *check.C) {
  command.PouchRun("exec", name, "sh", "-c", "exit 0").Assert(c, icmd.Success)
 }
 
-// TestExecFail test exec fail should not hang.
+// TestExecFail test exec fail should not hang, and test failed exec exit code should not be zero.
 func (suite *PouchExecSuite) TestExecFail(c *check.C) {
  name := "TestExecFail"
  res := command.PouchRun("run", "-d", "--name", name, "-u", name, busyboxImage, "top")
  defer DelContainerForceMultyTime(c, name)
  c.Assert(res.Stderr(), check.NotNil)
+
+ // test a 'executable file not found' fail should get exit code 126.
+ name = "TestExecFailCode"
+ command.PouchRun("run", "-d", "--name", name, busyboxImage, "top").Assert(c, icmd.Success)
+ command.PouchRun("exec", name, "shouldnotexit").Assert(c, icmd.Expected{ExitCode: 126})
+
+ // test a 'ls /nosuchfile' fail should get exit code not equal to 0.
+ res = command.PouchRun("exec", name, "ls", "/nosuchfile")
+ if res.ExitCode == 0 {
+ c.Fatalf("failed exec process exit code should not be 0")
+ }
 }
 
 // TestExecUser test exec with user.