From 43484e8b502be1dbf2e587524ae8ef036181b186 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 2 Apr 2013 09:22:30 -0700 Subject: [PATCH 1/4] Add a TestRunExit, make sure cmdRun returns after process dies --- commands_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/commands_test.go b/commands_test.go index aa772db74..0dffe3dc1 100644 --- a/commands_test.go +++ b/commands_test.go @@ -80,6 +80,62 @@ func TestRunHostname(t *testing.T) { } } +func TestRunExit(t *testing.T) { + runtime, err := newTestRuntime() + if err != nil { + t.Fatal(err) + } + defer nuke(runtime) + + srv := &Server{runtime: runtime} + + stdin, stdinPipe := io.Pipe() + stdout, stdoutPipe := io.Pipe() + c1 := make(chan struct{}) + go func() { + if err := srv.CmdRun(stdin, stdoutPipe, "-i", GetTestImage(runtime).Id, "/bin/cat"); err != nil { + t.Error(err) + } + close(c1) + }() + + setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + t.Fatal(err) + } + }) + + container := runtime.List()[0] + + // Closing /bin/cat stdin, expect it to exit + p, err := container.StdinPipe() + if err != nil { + t.Fatal(err) + } + if err := p.Close(); err != nil { + t.Fatal(err) + } + + // as the process exited, CmdRun must finish and unblock. Wait for it + setTimeout(t, "Waiting for CmdRun timed out", 2*time.Second, func() { + <-c1 + }) + + // Make sure that the client has been disconnected + setTimeout(t, "The client should have been disconnected once the remote process exited.", 2*time.Second, func() { + if _, err := stdin.Read([]byte{}); err != nil { + if err != io.EOF { + t.Fatal(err) + } + } + }) + + // Cleanup pipes + if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil { + t.Fatal(err) + } +} + // Expected behaviour: the process dies when the client disconnects func TestRunDisconnect(t *testing.T) { runtime, err := newTestRuntime() From 6882c78ce447b468262b53aef2ac0577151ea98c Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 2 Apr 2013 12:18:20 -0700 Subject: [PATCH 2/4] Add a stdincloser to container.Attach in order to close the client connection when needed --- commands.go | 16 ++++++++++++---- commands_test.go | 12 ++++-------- container.go | 27 +++++++++++++++++++++++---- 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/commands.go b/commands.go index 33a0997b9..ca1f7890c 100644 --- a/commands.go +++ b/commands.go @@ -799,7 +799,8 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri if container == nil { return fmt.Errorf("No such container: %s", name) } - return <-container.Attach(stdin, stdout, stdout) + + return <-container.Attach(stdin, nil, stdout, stdout) } // Ports type - Used to parse multiple -p flags @@ -901,11 +902,17 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) } } var ( - cStdin io.Reader + cStdin io.ReadCloser cStdout, cStderr io.Writer ) if config.AttachStdin { - cStdin = stdin + r, w := io.Pipe() + go func() { + defer w.Close() + defer Debugf("Closing buffered stdin pipe") + io.Copy(w, stdin) + }() + cStdin = r } if config.AttachStdout { cStdout = stdout @@ -913,7 +920,8 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) if config.AttachStderr { cStderr = stdout // FIXME: rcli can't differentiate stdout from stderr } - attachErr := container.Attach(cStdin, cStdout, cStderr) + + attachErr := container.Attach(cStdin, stdin, cStdout, cStderr) Debugf("Starting\n") if err := container.Start(); err != nil { return err diff --git a/commands_test.go b/commands_test.go index 0dffe3dc1..a68aea7a4 100644 --- a/commands_test.go +++ b/commands_test.go @@ -93,9 +93,7 @@ func TestRunExit(t *testing.T) { stdout, stdoutPipe := io.Pipe() c1 := make(chan struct{}) go func() { - if err := srv.CmdRun(stdin, stdoutPipe, "-i", GetTestImage(runtime).Id, "/bin/cat"); err != nil { - t.Error(err) - } + srv.CmdRun(stdin, stdoutPipe, "-i", GetTestImage(runtime).Id, "/bin/cat") close(c1) }() @@ -123,11 +121,8 @@ func TestRunExit(t *testing.T) { // Make sure that the client has been disconnected setTimeout(t, "The client should have been disconnected once the remote process exited.", 2*time.Second, func() { - if _, err := stdin.Read([]byte{}); err != nil { - if err != io.EOF { - t.Fatal(err) - } - } + // Expecting pipe i/o error, just check that read does not block + stdin.Read([]byte{}) }) // Cleanup pipes @@ -293,6 +288,7 @@ func TestAttachDisconnect(t *testing.T) { setTimeout(t, "Waiting for CmdAttach timed out", 2*time.Second, func() { <-c1 }) + // We closed stdin, expect /bin/cat to still be running // Wait a little bit to make sure container.monitor() did his thing err = container.WaitTimeout(500 * time.Millisecond) diff --git a/container.go b/container.go index adb160216..60c761ba8 100644 --- a/container.go +++ b/container.go @@ -257,9 +257,9 @@ func (container *Container) start() error { return container.cmd.Start() } -func (container *Container) Attach(stdin io.Reader, stdout io.Writer, stderr io.Writer) chan error { - var cStdout io.ReadCloser - var cStderr io.ReadCloser +func (container *Container) Attach(stdin io.ReadCloser, stdinCloser io.Closer, stdout io.Writer, stderr io.Writer) chan error { + var cStdout, cStderr io.ReadCloser + var nJobs int errors := make(chan error, 3) if stdin != nil && container.Config.OpenStdin { @@ -277,7 +277,8 @@ func (container *Container) Attach(stdin io.Reader, stdout io.Writer, stderr io. if err != nil { Debugf("[error] attach stdout: %s\n", err) } - errors <- err + // Discard error, expecting pipe error + errors <- nil }() } } @@ -290,6 +291,15 @@ func (container *Container) Attach(stdin io.Reader, stdout io.Writer, stderr io. go func() { Debugf("[start] attach stdout\n") defer Debugf("[end] attach stdout\n") + // If we are in StdinOnce mode, then close stdin + if container.Config.StdinOnce { + if stdin != nil { + defer stdin.Close() + } + if stdinCloser != nil { + defer stdinCloser.Close() + } + } _, err := io.Copy(stdout, cStdout) if err != nil { Debugf("[error] attach stdout: %s\n", err) @@ -307,6 +317,15 @@ func (container *Container) Attach(stdin io.Reader, stdout io.Writer, stderr io. go func() { Debugf("[start] attach stderr\n") defer Debugf("[end] attach stderr\n") + // If we are in StdinOnce mode, then close stdin + if container.Config.StdinOnce { + if stdin != nil { + defer stdin.Close() + } + if stdinCloser != nil { + defer stdinCloser.Close() + } + } _, err := io.Copy(stderr, cStderr) if err != nil { Debugf("[error] attach stderr: %s\n", err) From ad2bbe23be869e3a3287cedbf0408115e97a4a49 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 2 Apr 2013 12:19:01 -0700 Subject: [PATCH 3/4] Close the broadcaster once they are not needed anymore --- container.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/container.go b/container.go index 60c761ba8..4879ec3e9 100644 --- a/container.go +++ b/container.go @@ -269,13 +269,20 @@ func (container *Container) Attach(stdin io.ReadCloser, stdinCloser io.Closer, s } else { go func() { Debugf("[start] attach stdin\n") - defer Debugf("[end] attach stdin\n") + defer Debugf("[end] attach stdin\n") + // No matter what, when stdin is closed (io.Copy unblock), close stdout and stderr + if cStdout != nil { + defer cStdout.Close() + } + if cStderr != nil { + defer cStderr.Close() + } if container.Config.StdinOnce { defer cStdin.Close() } _, err := io.Copy(cStdin, stdin) if err != nil { - Debugf("[error] attach stdout: %s\n", err) + Debugf("[error] attach stdin: %s\n", err) } // Discard error, expecting pipe error errors <- nil From a19a9e3ca81880f030a7621a9e03ffb63c95b1f1 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 2 Apr 2013 12:21:35 -0700 Subject: [PATCH 4/4] Discarding errors in CmdRun --- commands.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/commands.go b/commands.go index ca1f7890c..0a2c7f47d 100644 --- a/commands.go +++ b/commands.go @@ -930,7 +930,9 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) fmt.Fprintln(stdout, container.ShortId()) } Debugf("Waiting for attach to return\n") - return <-attachErr + <-attachErr + // Expecting I/O pipe error, discarding + return nil } func NewServer() (*Server, error) {