From 56ad133512b4f05c071ec79bc4cf9ccb227567c1 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 2 Nov 2022 09:09:24 -0400 Subject: [PATCH] os/exec: allow open descriptors to be closed during TestPipeLookPathLeak In https://build.golang.org/log/d2eb315305bf3d513c490e7f85d56e9a016aacd2, we observe a failure in TestPipeLookPathLeak due to an additional descriptor (7) that was open at the start of the test being closed while the test executes. I haven't dug much into the failure, but it seems plausible to me that the descriptor may have been opened by libc for some reason, and may have been closed due to some sort of idle timeout or the completion of a background initialization routine. Since the test is looking for a leak, and closing an existing descriptor does not indicate a leak, let's not fail the test if an existing descriptor is unexpectedly closed. Updates #5071. Change-Id: I03973ddff6592c454cfcc790d6e56accd051dd52 Reviewed-on: https://go-review.googlesource.com/c/go/+/447235 Reviewed-by: Ian Lance Taylor Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot --- src/os/exec/exec_test.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 3c1fffd951c..7f1f99330dd 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -26,7 +26,6 @@ import ( "os/exec/internal/fdtest" "os/signal" "path/filepath" - "reflect" "runtime" "runtime/debug" "strconv" @@ -641,7 +640,11 @@ func TestPipeLookPathLeak(t *testing.T) { return fds } - want := openFDs() + old := map[uintptr]bool{} + for _, fd := range openFDs() { + old[fd] = true + } + for i := 0; i < 6; i++ { cmd := exec.Command("something-that-does-not-exist-executable") cmd.StdoutPipe() @@ -651,9 +654,16 @@ func TestPipeLookPathLeak(t *testing.T) { t.Fatal("unexpected success") } } - got := openFDs() - if !reflect.DeepEqual(got, want) { - t.Errorf("set of open file descriptors changed: got %v, want %v", got, want) + + // Since this test is not running in parallel, we don't expect any new file + // descriptors to be opened while it runs. However, if there are additional + // FDs present at the start of the test (for example, opened by libc), those + // may be closed due to a timeout of some sort. Allow those to go away, but + // check that no new FDs are added. + for _, fd := range openFDs() { + if !old[fd] { + t.Errorf("leaked file descriptor %v", fd) + } } }