From eaa8419a72215b53576ab5d2def399f9503d1f58 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 15 Jun 2023 17:41:19 -0700 Subject: [PATCH] runtime: decrement netpollWaiters in netpollunblock We used to decrement it in netpollgoready, but that missed the common case of a descriptor becoming ready due to I/O. All calls to netpollgoready go through netpollunblock, so this shouldn't miss any decrements we missed before. Fixes #60782 Change-Id: Ideefefa1ac96ca38e09fe2dd5d595c5dd7883237 Reviewed-on: https://go-review.googlesource.com/c/go/+/503923 TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Run-TryBot: Ian Lance Taylor Reviewed-by: Michael Knyszek Auto-Submit: Ian Lance Taylor --- src/runtime/crash_test.go | 9 +++ src/runtime/netpoll.go | 5 +- src/runtime/testdata/testprognet/waiters.go | 68 +++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 src/runtime/testdata/testprognet/waiters.go diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go index 8f11333b462..5eccf86e1a7 100644 --- a/src/runtime/crash_test.go +++ b/src/runtime/crash_test.go @@ -869,3 +869,12 @@ func TestPanicOnUnsafeSlice(t *testing.T) { t.Errorf("output does not contain %q:\n%s", want, output) } } + +func TestNetpollWaiters(t *testing.T) { + t.Parallel() + output := runTestProg(t, "testprognet", "NetpollWaiters") + want := "OK\n" + if output != want { + t.Fatalf("output is not %q\n%s", want, output) + } +} diff --git a/src/runtime/netpoll.go b/src/runtime/netpoll.go index 9b54e8e21f2..6877b2c350f 100644 --- a/src/runtime/netpoll.go +++ b/src/runtime/netpoll.go @@ -526,7 +526,6 @@ func netpollblockcommit(gp *g, gpp unsafe.Pointer) bool { } func netpollgoready(gp *g, traceskip int) { - netpollWaiters.Add(-1) goready(gp, traceskip+1) } @@ -587,13 +586,15 @@ func netpollunblock(pd *pollDesc, mode int32, ioready bool) *g { // will check for timeout/cancel before waiting. return nil } - var new uintptr + new := pdNil if ioready { new = pdReady } if gpp.CompareAndSwap(old, new) { if old == pdWait { old = pdNil + } else if old != pdNil { + netpollWaiters.Add(-1) } return (*g)(unsafe.Pointer(old)) } diff --git a/src/runtime/testdata/testprognet/waiters.go b/src/runtime/testdata/testprognet/waiters.go new file mode 100644 index 00000000000..6c8db1f14ef --- /dev/null +++ b/src/runtime/testdata/testprognet/waiters.go @@ -0,0 +1,68 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "fmt" + "io" + "log" + "net" + "runtime/internal/atomic" + "sync" + "time" + _ "unsafe" // for go:linkname +) + +// The bug is that netpollWaiters increases monotonically. +// This doesn't cause a problem until it overflows. +// Use linkname to see the value. +//go:linkname netpollWaiters runtime.netpollWaiters +var netpollWaiters atomic.Uint32 + +func init() { + register("NetpollWaiters", NetpollWaiters) +} + +func NetpollWaiters() { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + log.Fatal(err) + } + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + conn, err := listener.Accept() + if err != nil { + log.Fatal(err) + } + defer conn.Close() + if _, err := io.Copy(io.Discard, conn); err != nil { + log.Fatal(err) + } + }() + + wg.Add(1) + go func() { + defer wg.Done() + conn, err := net.Dial("tcp", listener.Addr().String()) + if err != nil { + log.Fatal(err) + } + defer conn.Close() + for i := 0; i < 10; i++ { + fmt.Fprintf(conn, "%d\n", i) + time.Sleep(time.Millisecond) + } + }() + + wg.Wait() + if v := netpollWaiters.Load(); v != 0 { + log.Fatalf("current waiters %v", v) + } + + fmt.Println("OK") +}