From 7632c4585f52cfe0dd3f3a61a93bfd43229714f5 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 5 Oct 2015 19:38:27 -0400 Subject: [PATCH] Fix for race from error on process start This rather naively fixes an error observed where a processes stdio streams are not written to when there is an error upon starting up the process, such as when the executable doesn't exist within the container's rootfs. Before the "fix", when an error occurred on start, `terminate` is called immediately, which calls `cmd.Process.Kill()`, then calling `Wait()` on the process. In some cases when this `Kill` is called the stdio stream have not yet been written to, causing non-deterministic output. The error itself is properly preserved but users attached to the process will not see this error. With the fix it is just calling `Wait()` when an error occurs rather than trying to `Kill()` the process first. This seems to preserve stdio. Signed-off-by: Brian Goff --- libcontainer/integration/execin_test.go | 27 ++++++++++++++++--------- libcontainer/process_linux.go | 1 + 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 33d7b1cc..2eb510da 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -133,16 +133,23 @@ func TestExecInError(t *testing.T) { }() ok(t, err) - unexistent := &libcontainer.Process{ - Args: []string{"unexistent"}, - Env: standardEnvironment, - } - err = container.Start(unexistent) - if err == nil { - t.Fatal("Should be an error") - } - if !strings.Contains(err.Error(), "executable file not found") { - t.Fatalf("Should be error about not found executable, got %s", err) + for i := 0; i < 42; i++ { + var out bytes.Buffer + unexistent := &libcontainer.Process{ + Args: []string{"unexistent"}, + Env: standardEnvironment, + Stdout: &out, + } + err = container.Start(unexistent) + if err == nil { + t.Fatal("Should be an error") + } + if !strings.Contains(err.Error(), "executable file not found") { + t.Fatalf("Should be error about not found executable, got %s", err) + } + if !bytes.Contains(out.Bytes(), []byte("executable file not found")) { + t.Fatalf("executable file not found error not delivered to stdio:\n%s", out.String()) + } } } diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 758f88a4..01021f75 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -84,6 +84,7 @@ func (p *setnsProcess) start() (err error) { return newSystemError(err) } if ierr != nil { + p.wait() return newSystemError(ierr) }