Skip to content

Commit

Permalink
runtime, internal/poll, net: report event scanning error on read event
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mikioh committed Mar 13, 2019
1 parent a891f2e commit a5fdd58
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 7 deletions.
31 changes: 31 additions & 0 deletions src/internal/poll/error_linux_test.go
Original file line number Diff line number Diff line change
@@ -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
}
}
21 changes: 21 additions & 0 deletions src/internal/poll/error_stub_test.go
Original file line number Diff line number Diff line change
@@ -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
}
44 changes: 44 additions & 0 deletions src/internal/poll/error_test.go
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 4 additions & 0 deletions src/internal/poll/fd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/internal/poll/fd_poll_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions src/net/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 11 additions & 3 deletions src/runtime/netpoll.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/netpoll_aix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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--
}
Expand Down
5 changes: 4 additions & 1 deletion src/runtime/netpoll_epoll.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/runtime/netpoll_kqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/netpoll_solaris.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ retry:
}

if mode != 0 {
pd.everr = false
if ev.portev_events&_POLLERR != 0 {
pd.everr = true
}
netpollready(&toRun, pd, mode)
}
}
Expand Down

0 comments on commit a5fdd58

Please sign in to comment.