runtime: add and use pollDesc fd sequence field

It is possible for a netpoll file to be closed and for the pollDesc
to be reused while a netpoll is running. This normally only causes
spurious wakeups, but if there is an error on the old file then the
new file can be incorrectly marked as having an error.

Fix this problem on most systems by introducing an fd sequence field
and using that as a tag in a taggedPointer. The taggedPointer is
stored in epoll or kqueue or whatever is being used. If the taggedPointer
returned by the kernel has a tag that does not match the fd
sequence field, the notification is for a closed file, and we
can ignore it. We check the tag stored in the pollDesc, and we also
check the tag stored in the pollDesc.atomicInfo.

This approach does not work on 32-bit systems where the kernel
only provides a 32-bit field to hold a user value. On those systems
we continue to use the older method without the sequence protection.
This is not ideal, but it is not an issue on Linux because the kernel
provides a 64-bit field, and it is not an issue on Windows because
there are no poller errors on Windows. It is potentially an issue
on *BSD systems, but on those systems we already call fstat in newFile
in os/file_unix.go to avoid adding non-pollable files to kqueue.
So we currently don't know of any cases that will fail.

Fixes #59545

Change-Id: I9a61e20dc39b4266a7a2978fc16446567fe683ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/484837
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Orlando Labao <orlando.labao43@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org>

1 files changed, 96 insertions, 0 deletions

diff --git a/src/os/fifo_test.go b/src/os/fifo_test.go
index 2f0e06bc52..7a6acce1af 100644
--- a/src/os/fifo_test.go
+++ b/src/os/fifo_test.go
@@ -7,8 +7,13 @@
 package os_test
 
 import (
+	"errors"
+	"internal/testenv"
+	"io/fs"
 	"os"
 	"path/filepath"
+	"strconv"
+	"sync"
 	"syscall"
 	"testing"
 )
@@ -59,3 +64,94 @@ func TestFifoEOF(t *testing.T) {
 
 	testPipeEOF(t, r, w)
 }
+
+// Issue #59545.
+func TestNonPollable(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test with tight loops in short mode")
+	}
+
+	// We need to open a non-pollable file.
+	// This is almost certainly Linux-specific,
+	// but if other systems have non-pollable files,
+	// we can add them here.
+	const nonPollable = "/dev/net/tun"
+
+	f, err := os.OpenFile(nonPollable, os.O_RDWR, 0)
+	if err != nil {
+		if errors.Is(err, fs.ErrExist) || errors.Is(err, fs.ErrPermission) || testenv.SyscallIsNotSupported(err) {
+			t.Skipf("can't open %q: %v", nonPollable, err)
+		}
+		t.Fatal(err)
+	}
+	f.Close()
+
+	// On a Linux laptop, before the problem was fixed,
+	// this test failed about 50% of the time with this
+	// number of iterations.
+	// It takes about 1/2 second when it passes.
+	const attempts = 20000
+
+	start := make(chan bool)
+	var wg sync.WaitGroup
+	wg.Add(1)
+	defer wg.Wait()
+	go func() {
+		defer wg.Done()
+		close(start)
+		for i := 0; i < attempts; i++ {
+			f, err := os.OpenFile(nonPollable, os.O_RDWR, 0)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			if err := f.Close(); err != nil {
+				t.Error(err)
+				return
+			}
+		}
+	}()
+
+	dir := t.TempDir()
+	<-start
+	for i := 0; i < attempts; i++ {
+		name := filepath.Join(dir, strconv.Itoa(i))
+		if err := syscall.Mkfifo(name, 0o600); err != nil {
+			t.Fatal(err)
+		}
+		// The problem only occurs if we use O_NONBLOCK here.
+		rd, err := os.OpenFile(name, os.O_RDONLY|syscall.O_NONBLOCK, 0o600)
+		if err != nil {
+			t.Fatal(err)
+		}
+		wr, err := os.OpenFile(name, os.O_WRONLY|syscall.O_NONBLOCK, 0o600)
+		if err != nil {
+			t.Fatal(err)
+		}
+		const msg = "message"
+		if _, err := wr.Write([]byte(msg)); err != nil {
+			if errors.Is(err, syscall.EAGAIN) || errors.Is(err, syscall.ENOBUFS) {
+				t.Logf("ignoring write error %v", err)
+				rd.Close()
+				wr.Close()
+				continue
+			}
+			t.Fatalf("write to fifo %d failed: %v", i, err)
+		}
+		if _, err := rd.Read(make([]byte, len(msg))); err != nil {
+			if errors.Is(err, syscall.EAGAIN) || errors.Is(err, syscall.ENOBUFS) {
+				t.Logf("ignoring read error %v", err)
+				rd.Close()
+				wr.Close()
+				continue
+			}
+			t.Fatalf("read from fifo %d failed; %v", i, err)
+		}
+		if err := rd.Close(); err != nil {
+			t.Fatal(err)
+		}
+		if err := wr.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}
+}