]> Cypherpunks repositories - gostls13.git/commitdiff
runtime, internal/poll, net: report event scanning error on read event
authorMikio Hara <mikioh.public.networking@gmail.com>
Sun, 10 Mar 2019 05:30:06 +0000 (14:30 +0900)
committerMikio Hara <mikioh.public.networking@gmail.com>
Wed, 13 Mar 2019 08:53:02 +0000 (08:53 +0000)
This change makes it possible the runtime-integrated network poller and
APIs in the package internal/poll to report an event scanning error on a
read event.

The latest Go releases open up the way of the manipulation of the poller
for users. On the other hand, it starts misleading users into believing
that the poller accepts any user-configured file or socket perfectly
because of not reporting any error on event scanning, as mentioned in
issue 30426. The initial implementation of the poller was designed for
just well-configured, validated sockets produced by the package net.
However, the assumption is now obsolete.

Fixes #30624.

Benchmark results on linux/amd64:

benchmark                              old ns/op     new ns/op     delta
BenchmarkTCP4OneShot-4                 24649         23979         -2.72%
BenchmarkTCP4OneShotTimeout-4          25742         24411         -5.17%
BenchmarkTCP4Persistent-4              5139          5222          +1.62%
BenchmarkTCP4PersistentTimeout-4       4919          4892          -0.55%
BenchmarkTCP6OneShot-4                 21182         20767         -1.96%
BenchmarkTCP6OneShotTimeout-4          23364         22305         -4.53%
BenchmarkTCP6Persistent-4              4351          4366          +0.34%
BenchmarkTCP6PersistentTimeout-4       4227          4255          +0.66%
BenchmarkTCP4ConcurrentReadWrite-4     2309          1839          -20.36%
BenchmarkTCP6ConcurrentReadWrite-4     2180          1791          -17.84%

benchmark                              old allocs     new allocs   delta
BenchmarkTCP4OneShot-4                 26             26           +0.00%
BenchmarkTCP4OneShotTimeout-4          26             26           +0.00%
BenchmarkTCP4Persistent-4              0              0            +0.00%
BenchmarkTCP4PersistentTimeout-4       0              0            +0.00%
BenchmarkTCP6OneShot-4                 26             26           +0.00%
BenchmarkTCP6OneShotTimeout-4          26             26           +0.00%
BenchmarkTCP6Persistent-4              0              0            +0.00%
BenchmarkTCP6PersistentTimeout-4       0              0            +0.00%
BenchmarkTCP4ConcurrentReadWrite-4     0              0            +0.00%
BenchmarkTCP6ConcurrentReadWrite-4     0              0            +0.00%

benchmark                              old bytes     new bytes     delta
BenchmarkTCP4OneShot-4                 2000          2000          +0.00%
BenchmarkTCP4OneShotTimeout-4          2000          2000          +0.00%
BenchmarkTCP4Persistent-4              0             0             +0.00%
BenchmarkTCP4PersistentTimeout-4       0             0             +0.00%
BenchmarkTCP6OneShot-4                 2144          2144          +0.00%
BenchmarkTCP6OneShotTimeout-4          2144          2145          +0.05%
BenchmarkTCP6Persistent-4              0             0             +0.00%
BenchmarkTCP6PersistentTimeout-4       0             0             +0.00%
BenchmarkTCP4ConcurrentReadWrite-4     0             0             +0.00%
BenchmarkTCP6ConcurrentReadWrite-4     0             0             +0.00%

Change-Id: Iab60e504dff5639e688dc5420d852f336508c0af
Reviewed-on: https://go-review.googlesource.com/c/go/+/166497
Run-TryBot: Mikio Hara <mikioh.public.networking@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/internal/poll/error_linux_test.go [new file with mode: 0644]
src/internal/poll/error_stub_test.go [new file with mode: 0644]
src/internal/poll/error_test.go [new file with mode: 0644]
src/internal/poll/fd.go
src/internal/poll/fd_poll_runtime.go
src/net/error_test.go
src/runtime/netpoll.go
src/runtime/netpoll_aix.go
src/runtime/netpoll_epoll.go
src/runtime/netpoll_kqueue.go
src/runtime/netpoll_solaris.go

diff --git a/src/internal/poll/error_linux_test.go b/src/internal/poll/error_linux_test.go
new file mode 100644 (file)
index 0000000..059fb8e
--- /dev/null
@@ -0,0 +1,31 @@
+// Copyright 2019 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 poll_test
+
+import (
+       "errors"
+       "internal/poll"
+       "os"
+       "syscall"
+)
+
+func badStateFile() (*os.File, error) {
+       if os.Getuid() != 0 {
+               return nil, errors.New("must be root")
+       }
+       // Using OpenFile for a device file is an easy way to make a
+       // file attached to the runtime-integrated network poller and
+       // configured in halfway.
+       return os.OpenFile("/dev/net/tun", os.O_RDWR, 0)
+}
+
+func isBadStateFileError(err error) (string, bool) {
+       switch err {
+       case poll.ErrNotPollable, syscall.EBADFD:
+               return "", true
+       default:
+               return "not pollable or file in bad state error", false
+       }
+}
diff --git a/src/internal/poll/error_stub_test.go b/src/internal/poll/error_stub_test.go
new file mode 100644 (file)
index 0000000..c40ffcd
--- /dev/null
@@ -0,0 +1,21 @@
+// Copyright 2019 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.
+
+// +build !linux
+
+package poll_test
+
+import (
+       "errors"
+       "os"
+       "runtime"
+)
+
+func badStateFile() (*os.File, error) {
+       return nil, errors.New("not supported on " + runtime.GOOS)
+}
+
+func isBadStateFileError(err error) (string, bool) {
+       return "", false
+}
diff --git a/src/internal/poll/error_test.go b/src/internal/poll/error_test.go
new file mode 100644 (file)
index 0000000..89c6e38
--- /dev/null
@@ -0,0 +1,44 @@
+// Copyright 2019 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 poll_test
+
+import (
+       "fmt"
+       "net"
+       "os"
+       "testing"
+)
+
+func TestReadError(t *testing.T) {
+       t.Run("ErrNotPollable", func(t *testing.T) {
+               f, err := badStateFile()
+               if err != nil {
+                       t.Skip(err)
+               }
+               defer f.Close()
+               var b [1]byte
+               _, err = f.Read(b[:])
+               if perr := parseReadError(err, isBadStateFileError); perr != nil {
+                       t.Fatal(perr)
+               }
+       })
+}
+
+func parseReadError(nestedErr error, verify func(error) (string, bool)) error {
+       err := nestedErr
+       if nerr, ok := err.(*net.OpError); ok {
+               err = nerr.Err
+       }
+       if nerr, ok := err.(*os.PathError); ok {
+               err = nerr.Err
+       }
+       if nerr, ok := err.(*os.SyscallError); ok {
+               err = nerr.Err
+       }
+       if s, ok := verify(err); !ok {
+               return fmt.Errorf("got %v; want %s", nestedErr, s)
+       }
+       return nil
+}
index 2567746106f7b0961a51d487168c6e52c17d3f5e..2ab86f23143208c5469713dcffbb66ed644407e9 100644 (file)
@@ -44,6 +44,10 @@ func (e *TimeoutError) Error() string   { return "i/o timeout" }
 func (e *TimeoutError) Timeout() bool   { return true }
 func (e *TimeoutError) Temporary() bool { return true }
 
+// ErrNotPollable is returned when the file or socket is not suitable
+// for event notification.
+var ErrNotPollable = errors.New("not pollable")
+
 // consume removes data from a slice of byte slices, for writev.
 func consume(v *[][]byte, n int64) {
        for len(*v) > 0 {
index 2932615d851df00ac4d2c313859342466ed5b7bb..d32f4a0ddd8921a3f3b0a42930b5adc223d7f33f 100644 (file)
@@ -115,6 +115,8 @@ func convertErr(res int, isFile bool) error {
                return errClosing(isFile)
        case 2:
                return ErrTimeout
+       case 3:
+               return ErrNotPollable
        }
        println("unreachable: ", res)
        panic("unreachable")
index 2819986c0cd7cefb20a759050afb57666370a771..b0622d7fd5b02704f89a4f9dcd8bfab5e5865147 100644 (file)
@@ -436,7 +436,7 @@ second:
                goto third
        }
        switch nestedErr {
-       case poll.ErrNetClosing, poll.ErrTimeout:
+       case poll.ErrNetClosing, poll.ErrTimeout, poll.ErrNotPollable:
                return nil
        }
        return fmt.Errorf("unexpected type on 2nd nested level: %T", nestedErr)
@@ -627,7 +627,7 @@ second:
                goto third
        }
        switch nestedErr {
-       case poll.ErrNetClosing, poll.ErrTimeout:
+       case poll.ErrNetClosing, poll.ErrTimeout, poll.ErrNotPollable:
                return nil
        }
        return fmt.Errorf("unexpected type on 2nd nested level: %T", nestedErr)
index 71ca993cc05654129e245f993b757eb21fd71e49..0de67c63e75ae4fa6641a52f7a3ce99ed6482c38 100644 (file)
@@ -49,13 +49,14 @@ type pollDesc struct {
        // The lock protects pollOpen, pollSetDeadline, pollUnblock and deadlineimpl operations.
        // This fully covers seq, rt and wt variables. fd is constant throughout the PollDesc lifetime.
        // pollReset, pollWait, pollWaitCanceled and runtime·netpollready (IO readiness notification)
-       // proceed w/o taking the lock. So closing, rg, rd, wg and wd are manipulated
+       // proceed w/o taking the lock. So closing, everr, rg, rd, wg and wd are manipulated
        // in a lock-free way by all operations.
        // NOTE(dvyukov): the following code uses uintptr to store *g (rg/wg),
        // that will blow up when GC starts moving objects.
        lock    mutex // protects the following fields
        fd      uintptr
        closing bool
+       everr   bool    // marks event scanning error happened
        user    uint32  // user settable cookie
        rseq    uintptr // protects from stale read timers
        rg      uintptr // pdReady, pdWait, G waiting for read or nil
@@ -120,6 +121,7 @@ func poll_runtime_pollOpen(fd uintptr) (*pollDesc, int) {
        }
        pd.fd = fd
        pd.closing = false
+       pd.everr = false
        pd.rseq++
        pd.rg = 0
        pd.rd = 0
@@ -335,10 +337,16 @@ func netpollready(toRun *gList, pd *pollDesc, mode int32) {
 
 func netpollcheckerr(pd *pollDesc, mode int32) int {
        if pd.closing {
-               return 1 // errClosing
+               return 1 // ErrFileClosing or ErrNetClosing
        }
        if (mode == 'r' && pd.rd < 0) || (mode == 'w' && pd.wd < 0) {
-               return 2 // errTimeout
+               return 2 // ErrTimeout
+       }
+       // Report an event scanning error only on a read event.
+       // An error on a write event will be captured in a subsequent
+       // write call that is able to report a more specific error.
+       if mode == 'r' && pd.everr {
+               return 3 // ErrNotPollable
        }
        return 0
 }
index 1e886dae949e965a36d796a84d2210945ef919aa..b4d7de8c2af53cd4288ef388b02b144b1764b327 100644 (file)
@@ -232,6 +232,10 @@ retry:
                        if pollVerbose {
                                println("*** netpollready i=", i, "revents=", pfd.revents, "events=", pfd.events, "pd=", pds[i])
                        }
+                       pds[i].everr = false
+                       if pfd.revents&_POLLERR != 0 {
+                               pds[i].everr = true
+                       }
                        netpollready(&toRun, pds[i], mode)
                        n--
                }
index f764d6ff7c886021e51288cef844a5ba718266e4..7dc8301acd87544339ad76c8ed66e647f594e8d5 100644 (file)
@@ -91,7 +91,10 @@ retry:
                }
                if mode != 0 {
                        pd := *(**pollDesc)(unsafe.Pointer(&ev.data))
-
+                       pd.everr = false
+                       if ev.events&_EPOLLERR != 0 {
+                               pd.everr = true
+                       }
                        netpollready(&toRun, pd, mode)
                }
        }
index fdaa1cd80debd23dad16865646b2fbd4fbe58731..1de484978a87b76256282501aae2c534360f22b6 100644 (file)
@@ -102,7 +102,12 @@ retry:
                        mode += 'w'
                }
                if mode != 0 {
-                       netpollready(&toRun, (*pollDesc)(unsafe.Pointer(ev.udata)), mode)
+                       pd := (*pollDesc)(unsafe.Pointer(ev.udata))
+                       pd.everr = false
+                       if ev.flags&_EV_ERROR != 0 {
+                               pd.everr = true
+                       }
+                       netpollready(&toRun, pd, mode)
                }
        }
        if block && toRun.empty() {
index 6bd484afaaec8dca228f3476769652b636f3e5c2..7ae8a2aba10174a3ef97d7acf76a33fe04fcbed0 100644 (file)
@@ -233,6 +233,10 @@ retry:
                }
 
                if mode != 0 {
+                       pd.everr = false
+                       if ev.portev_events&_POLLERR != 0 {
+                               pd.everr = true
+                       }
                        netpollready(&toRun, pd, mode)
                }
        }