From 274d3a06f7331740b849e20cff1d1c1ab84dd0e0 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 5 Oct 2022 15:26:43 -0400 Subject: [PATCH] os/exec: delete TestExtraFilesFDShuffle This test has been disabled for over nine years (since CL 12869049). Although it still compiles, it seems likely to have rotted since then, and if it was going to detect a real bug it also seems like that bug would have been encountered and reported by users since then (and would presumably have its own regression tests). To eliminate overhead from mainining it (or skipping over it while maintaining other tests), let's just delete it. Fixes #5780. Change-Id: I2a85cba20cba98a1dc6fc82336ae5e22d2242e99 Reviewed-on: https://go-review.googlesource.com/c/go/+/439197 Reviewed-by: Ian Lance Taylor Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot --- src/os/exec/exec_test.go | 125 +++------------------------------------ 1 file changed, 9 insertions(+), 116 deletions(-) diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 13715fecac..33f7022a6d 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -180,16 +180,15 @@ var exeOnce struct { var helperCommandUsed sync.Map var helperCommands = map[string]func(...string){ - "echo": cmdEcho, - "echoenv": cmdEchoEnv, - "cat": cmdCat, - "pipetest": cmdPipeTest, - "stdinClose": cmdStdinClose, - "exit": cmdExit, - "describefiles": cmdDescribeFiles, - "extraFilesAndPipes": cmdExtraFilesAndPipes, - "stderrfail": cmdStderrFail, - "yes": cmdYes, + "echo": cmdEcho, + "echoenv": cmdEchoEnv, + "cat": cmdCat, + "pipetest": cmdPipeTest, + "stdinClose": cmdStdinClose, + "exit": cmdExit, + "describefiles": cmdDescribeFiles, + "stderrfail": cmdStderrFail, + "yes": cmdYes, } func cmdEcho(args ...string) { @@ -272,25 +271,6 @@ func cmdDescribeFiles(args ...string) { } } -func cmdExtraFilesAndPipes(args ...string) { - n, _ := strconv.Atoi(args[0]) - pipes := make([]*os.File, n) - for i := 0; i < n; i++ { - pipes[i] = os.NewFile(uintptr(3+i), strconv.Itoa(i)) - } - response := "" - for i, r := range pipes { - buf := make([]byte, 10) - n, err := r.Read(buf) - if err != nil { - fmt.Fprintf(os.Stderr, "Child: read error: %v on pipe %d\n", err, i) - os.Exit(1) - } - response = response + string(buf[:n]) - } - fmt.Fprintf(os.Stderr, "child: %s", response) -} - func cmdStderrFail(...string) { fmt.Fprintf(os.Stderr, "some stderr text\n") os.Exit(1) @@ -629,93 +609,6 @@ func TestPipeLookPathLeak(t *testing.T) { } } -func TestExtraFilesFDShuffle(t *testing.T) { - maySkipHelperCommand("extraFilesAndPipes") - testenv.SkipFlaky(t, 5780) - switch runtime.GOOS { - case "windows": - t.Skip("no operating system support; skipping") - } - - // syscall.StartProcess maps all the FDs passed to it in - // ProcAttr.Files (the concatenation of stdin,stdout,stderr and - // ExtraFiles) into consecutive FDs in the child, that is: - // Files{11, 12, 6, 7, 9, 3} should result in the file - // represented by FD 11 in the parent being made available as 0 - // in the child, 12 as 1, etc. - // - // We want to test that FDs in the child do not get overwritten - // by one another as this shuffle occurs. The original implementation - // was buggy in that in some data dependent cases it would overwrite - // stderr in the child with one of the ExtraFile members. - // Testing for this case is difficult because it relies on using - // the same FD values as that case. In particular, an FD of 3 - // must be at an index of 4 or higher in ProcAttr.Files and - // the FD of the write end of the Stderr pipe (as obtained by - // StderrPipe()) must be the same as the size of ProcAttr.Files; - // therefore we test that the read end of this pipe (which is what - // is returned to the parent by StderrPipe() being one less than - // the size of ProcAttr.Files, i.e. 3+len(cmd.ExtraFiles). - // - // Moving this test case around within the overall tests may - // affect the FDs obtained and hence the checks to catch these cases. - npipes := 2 - c := helperCommand(t, "extraFilesAndPipes", strconv.Itoa(npipes+1)) - rd, wr, _ := os.Pipe() - defer rd.Close() - if rd.Fd() != 3 { - t.Errorf("bad test value for test pipe: fd %d", rd.Fd()) - } - stderr, _ := c.StderrPipe() - wr.WriteString("_LAST") - wr.Close() - - pipes := make([]struct { - r, w *os.File - }, npipes) - data := []string{"a", "b"} - - for i := 0; i < npipes; i++ { - r, w, err := os.Pipe() - if err != nil { - t.Fatalf("unexpected error creating pipe: %s", err) - } - pipes[i].r = r - pipes[i].w = w - w.WriteString(data[i]) - c.ExtraFiles = append(c.ExtraFiles, pipes[i].r) - defer func() { - r.Close() - w.Close() - }() - } - // Put fd 3 at the end. - c.ExtraFiles = append(c.ExtraFiles, rd) - - stderrFd := int(stderr.(*os.File).Fd()) - if stderrFd != ((len(c.ExtraFiles) + 3) - 1) { - t.Errorf("bad test value for stderr pipe") - } - - expected := "child: " + strings.Join(data, "") + "_LAST" - - err := c.Start() - if err != nil { - t.Fatalf("Run: %v", err) - } - - buf := make([]byte, 512) - n, err := stderr.Read(buf) - if err != nil { - t.Errorf("Read: %s", err) - } else { - if m := string(buf[:n]); m != expected { - t.Errorf("Read: '%s' not '%s'", m, expected) - } - } - c.Wait() -} - func TestExtraFiles(t *testing.T) { if haveUnexpectedFDs { // The point of this test is to make sure that any -- 2.50.0