syscall: fix finalizer fd close bugs in TestFcntlFlock and TestPassFD

Currently, the syscall test suite takes very little time to run. It
stands to reason that pretty much every time, zero GCs execute.

With CL 309869, this changes because the minimum heap size is lowered,
triggering two bugs in the test suite.

One bug is in TestFcntlFlock, where a raw FD is wrapped in an os.File
whose last reference is passed into a Cmd. That FD is then closed by a
defer syscall.Close, instead of the os.File's Close, so the finalizer
may fire *after* that FD has already been reused by another test.

The second bug is in the child helper process of TestPassFD, where
there's a small window in which a temp file's FD is encoded for an
out-of-band unix domain socket message to the parent, but not yet sent.
The point of encoding is also the last reference that FD's os.File, so a
finalizer may run at any time. While it's safe for the finalizer to run
after the FD is sent, if it runs before, the send will fail, since unix
domain sockets require that any sent FDs are valid.

Change-Id: I2d1bd7e6db6efcc6763273217fd85cb5b9764274
Reviewed-on: https://go-review.googlesource.com/c/go/+/360575
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/syscall/syscall_unix_test.go b/src/syscall/syscall_unix_test.go
index 7cc78c2147bb1a20a06e701c987429f1cf64d50e..e4af0ba4a595f67bd975ded647205310fc228bdf 100644 (file)
--- a/src/syscall/syscall_unix_test.go
+++ b/src/syscall/syscall_unix_test.go
@@ -84,16 +84,24 @@ func TestFcntlFlock(t *testing.T) {
                if err != nil {
                        t.Fatalf("Open failed: %v", err)
                }
-               defer syscall.Close(fd)
-               if err := syscall.Ftruncate(fd, 1<<20); err != nil {
+               // f takes ownership of fd, and will close it.
+               //
+               // N.B. This defer is also necessary to keep f alive
+               // while we use its fd, preventing its finalizer from
+               // executing.
+               f := os.NewFile(uintptr(fd), name)
+               defer f.Close()
+
+               if err := syscall.Ftruncate(int(f.Fd()), 1<<20); err != nil {
                        t.Fatalf("Ftruncate(1<<20) failed: %v", err)
                }
-               if err := syscall.FcntlFlock(uintptr(fd), syscall.F_SETLK, &flock); err != nil {
+               if err := syscall.FcntlFlock(f.Fd(), syscall.F_SETLK, &flock); err != nil {
                        t.Fatalf("FcntlFlock(F_SETLK) failed: %v", err)
                }
+
                cmd := exec.Command(os.Args[0], "-test.run=^TestFcntlFlock$")
                cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
-               cmd.ExtraFiles = []*os.File{os.NewFile(uintptr(fd), name)}
+               cmd.ExtraFiles = []*os.File{f}
                out, err := cmd.CombinedOutput()
                if len(out) > 0 || err != nil {
                        t.Fatalf("child process: %q, %v", out, err)
@@ -251,6 +259,10 @@ func passFDChild() {
                fmt.Printf("TempFile: %v", err)
                return
        }
+       // N.B. This defer is also necessary to keep f alive
+       // while we use its fd, preventing its finalizer from
+       // executing.
+       defer f.Close()
 
        f.Write([]byte("Hello from child process!\n"))
        f.Seek(0, io.SeekStart)