Skip to content

Commit

Permalink
bugfix: avoid data race in set exec exit status
Browse files Browse the repository at this point in the history
there are two goroutines response for changed exec config information,
if exec process got error, data race appears, exec exit hook runs in a
goroutine, it modify exec exit status, StartExec also modify exec exit
status in another goroutine since error happend. Fix it by only one
place can modify exec exit status.

Signed-off-by: Ace-Tang <aceapril@126.com>
  • Loading branch information
Ace-Tang committed Aug 28, 2018
1 parent b7b42b1 commit 7fd9086
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
15 changes: 7 additions & 8 deletions ctrd/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
1 change: 0 additions & 1 deletion daemon/mgr/container_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 12 additions & 1 deletion test/cli_exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 7fd9086

Please sign in to comment.