From d5514013b6110850789d5397b9b972527e1641cd Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 14 Apr 2023 15:34:10 -0700 Subject: [PATCH] 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 Auto-Submit: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Reviewed-by: Orlando Labao Reviewed-by: Ian Lance Taylor TryBot-Bypass: Ian Lance Taylor Reviewed-by: Bryan Mills TryBot-Bypass: Ian Lance Taylor --- src/os/fifo_test.go | 96 ++++++++++++++++++++++++++++++++++ src/runtime/netpoll.go | 39 ++++++++++++-- src/runtime/netpoll_aix.go | 5 +- src/runtime/netpoll_epoll.go | 13 +++-- src/runtime/netpoll_kqueue.go | 31 +++++++++-- src/runtime/netpoll_solaris.go | 20 +++++-- src/runtime/netpoll_windows.go | 1 + 7 files changed, 189 insertions(+), 16 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) + } + } +} diff --git a/src/runtime/netpoll.go b/src/runtime/netpoll.go index b4eb7330c3..b1b3766e11 100644 --- a/src/runtime/netpoll.go +++ b/src/runtime/netpoll.go @@ -71,9 +71,10 @@ const pollBlockSize = 4 * 1024 // // No heap pointers. type pollDesc struct { - _ sys.NotInHeap - link *pollDesc // in pollcache, protected by pollcache.lock - fd uintptr // constant for pollDesc usage lifetime + _ sys.NotInHeap + link *pollDesc // in pollcache, protected by pollcache.lock + fd uintptr // constant for pollDesc usage lifetime + fdseq atomic.Uintptr // protects against stale pollDesc // atomicInfo holds bits from closing, rd, and wd, // which are only ever written while holding the lock, @@ -120,6 +121,12 @@ const ( pollEventErr pollExpiredReadDeadline pollExpiredWriteDeadline + pollFDSeq // 20 bit field, low 20 bits of fdseq field +) + +const ( + pollFDSeqBits = 20 // number of bits in pollFDSeq + pollFDSeqMask = 1<> pollFDSeq) & pollFDSeqMask + if seq != 0 && xSeq != mSeq { + return + } for (x&pollEventErr != 0) != b && !pd.atomicInfo.CompareAndSwap(x, x^pollEventErr) { x = pd.atomicInfo.Load() + xSeq := (x >> pollFDSeq) & pollFDSeqMask + if seq != 0 && xSeq != mSeq { + return + } } } @@ -226,8 +245,12 @@ func poll_runtime_pollOpen(fd uintptr) (*pollDesc, int) { throw("runtime: blocked read on free polldesc") } pd.fd = fd + if pd.fdseq.Load() == 0 { + // The value 0 is special in setEventErr, so don't use it. + pd.fdseq.Store(1) + } pd.closing = false - pd.setEventErr(false) + pd.setEventErr(false, 0) pd.rseq++ pd.rg.Store(pdNil) pd.rd = 0 @@ -264,6 +287,12 @@ func poll_runtime_pollClose(pd *pollDesc) { } func (c *pollCache) free(pd *pollDesc) { + // Increment the fdseq field, so that any currently + // running netpoll calls will not mark pd as ready. + fdseq := pd.fdseq.Load() + fdseq = (fdseq + 1) & (1<