diff --git a/contrib/cmd/recvtty/recvtty.go b/contrib/cmd/recvtty/recvtty.go index cf2fddf76..8fddd0269 100644 --- a/contrib/cmd/recvtty/recvtty.go +++ b/contrib/cmd/recvtty/recvtty.go @@ -106,9 +106,6 @@ func handleSingle(path string, noStdin bool) error { if err != nil { return err } - if err := console.ClearONLCR(c.Fd()); err != nil { - return err - } // Copy from our stdio to the master fd. var ( diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index bb8ff0b52..06684eb5b 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -186,6 +186,10 @@ func setupConsole(socket *os.File, config *initConfig, mount bool) error { if err != nil { return err } + err = console.ClearONLCR(pty.Fd()) + if err != nil { + return err + } // After we return from here, we don't need the console anymore. defer pty.Close() diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index ad2066ea9..dda716712 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -277,7 +277,12 @@ func TestExecInTTY(t *testing.T) { } err = container.Run(process) stdinR.Close() - defer stdinW.Close() + defer func() { + stdinW.Close() + if _, err := process.Wait(); err != nil { + t.Log(err) + } + }() ok(t, err) var stdout bytes.Buffer @@ -286,67 +291,55 @@ func TestExecInTTY(t *testing.T) { Args: []string{"ps"}, Env: standardEnvironment, } - parent, child, err := utils.NewSockPair("console") - if err != nil { + + // Repeat to increase chances to catch a race; see + // https://github.com/opencontainers/runc/issues/2425. + for i := 0; i < 300; i++ { + parent, child, err := utils.NewSockPair("console") + if err != nil { + ok(t, err) + } + ps.ConsoleSocket = child + + done := make(chan (error)) + go func() { + f, err := utils.RecvFd(parent) + if err != nil { + done <- fmt.Errorf("RecvFd: %w", err) + return + } + c, err := console.ConsoleFromFile(f) + if err != nil { + done <- fmt.Errorf("ConsoleFromFile: %w", err) + return + } + // An error from io.Copy is expected once the terminal + // is gone, so we deliberately ignore it. + _, _ = io.Copy(&stdout, c) + done <- nil + }() + + err = container.Run(ps) ok(t, err) - } - defer parent.Close() - defer child.Close() - ps.ConsoleSocket = child - type cdata struct { - c console.Console - err error - } - dc := make(chan *cdata, 1) - go func() { - f, err := utils.RecvFd(parent) - if err != nil { - dc <- &cdata{ - err: err, - } - return - } - c, err := console.ConsoleFromFile(f) - if err != nil { - dc <- &cdata{ - err: err, - } - return - } - console.ClearONLCR(c.Fd()) - dc <- &cdata{ - c: c, - } - }() - err = container.Run(ps) - ok(t, err) - data := <-dc - if data.err != nil { - ok(t, data.err) - } - console := data.c - copy := make(chan struct{}) - go func() { - io.Copy(&stdout, console) - close(copy) - }() - ok(t, err) - select { - case <-time.After(5 * time.Second): - t.Fatal("Waiting for copy timed out") - case <-copy: - } - waitProcess(ps, t) - stdinW.Close() - waitProcess(process, t) + select { + case <-time.After(5 * time.Second): + t.Fatal("Waiting for copy timed out") + case err := <-done: + ok(t, err) + } - out := stdout.String() - if !strings.Contains(out, "cat") || !strings.Contains(out, "ps") { - t.Fatalf("unexpected running process, output %q", out) - } - if strings.Contains(out, "\r") { - t.Fatalf("unexpected carriage-return in output %q", out) + waitProcess(ps, t) + parent.Close() + child.Close() + + out := stdout.String() + if !strings.Contains(out, "cat") || !strings.Contains(out, "ps") { + t.Fatalf("unexpected running process, output %q", out) + } + if strings.Contains(out, "\r") { + t.Fatalf("unexpected carriage-return in output %q", out) + } } } diff --git a/tty.go b/tty.go index 844d7f842..b15010202 100644 --- a/tty.go +++ b/tty.go @@ -112,10 +112,6 @@ func (t *tty) recvtty(process *libcontainer.Process, socket *os.File) (Err error if err != nil { return err } - err = console.ClearONLCR(cons.Fd()) - if err != nil { - return err - } epoller, err := console.NewEpoller() if err != nil { return err diff --git a/utils_linux.go b/utils_linux.go index f15537fa7..8241d4232 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -164,10 +164,7 @@ func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, det t.postStart = append(t.postStart, parent, child) t.consoleC = make(chan error, 1) go func() { - if err := t.recvtty(process, parent); err != nil { - t.consoleC <- err - } - t.consoleC <- nil + t.consoleC <- t.recvtty(process, parent) }() } else { // the caller of runc will handle receiving the console master