From 5b425cc3ab2c9ce4752a5baa9a52ab86bda96036 Mon Sep 17 00:00:00 2001 From: Dave Cheney Date: Fri, 30 Nov 2012 20:02:30 +1100 Subject: [PATCH] undo CL 6855110 / 869253ef7009 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 64bit atomics are broken on 32bit systems. This is issue 599. linux/arm builders all broke with this change, I am concerned that the other 32bit builders are silently impacted. ««« original CL description net: fix data races on deadline vars Fixes #4434. R=mikioh.mikioh, bradfitz, dvyukov, alex.brainman CC=golang-dev https://golang.org/cl/6855110 »»» R=rsc, mikioh.mikioh, dvyukov, minux.ma CC=golang-dev https://golang.org/cl/6852105 --- src/pkg/net/fd_posix_test.go | 57 ---------------- src/pkg/net/fd_unix.go | 114 +++++++++++++++----------------- src/pkg/net/fd_windows.go | 48 +++----------- src/pkg/net/sendfile_freebsd.go | 2 +- src/pkg/net/sendfile_linux.go | 2 +- src/pkg/net/sock_posix.go | 6 +- src/pkg/net/sockopt_posix.go | 21 ++++-- 7 files changed, 83 insertions(+), 167 deletions(-) delete mode 100644 src/pkg/net/fd_posix_test.go diff --git a/src/pkg/net/fd_posix_test.go b/src/pkg/net/fd_posix_test.go deleted file mode 100644 index 8be0335d61..0000000000 --- a/src/pkg/net/fd_posix_test.go +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2012 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 darwin freebsd linux netbsd openbsd windows - -package net - -import ( - "testing" - "time" -) - -var deadlineSetTimeTests = []struct { - input time.Time - expected int64 -}{ - {time.Time{}, 0}, - {time.Date(2009, 11, 10, 23, 00, 00, 00, time.UTC), 1257894000000000000}, // 2009-11-10 23:00:00 +0000 UTC -} - -func TestDeadlineSetTime(t *testing.T) { - for _, tt := range deadlineSetTimeTests { - var d deadline - d.setTime(tt.input) - actual := d.value() - expected := int64(0) - if !tt.input.IsZero() { - expected = tt.input.UnixNano() - } - if actual != expected { - t.Errorf("set/value failed: expected %v, actual %v", expected, actual) - } - } -} - -var deadlineExpiredTests = []struct { - deadline time.Time - expired bool -}{ - // note, times are relative to the start of the test run, not - // the start of TestDeadlineExpired - {time.Now().Add(5 * time.Minute), false}, - {time.Now().Add(-5 * time.Minute), true}, - {time.Time{}, false}, // no deadline set -} - -func TestDeadlineExpired(t *testing.T) { - for _, tt := range deadlineExpiredTests { - var d deadline - d.set(tt.deadline.UnixNano()) - expired := d.expired() - if expired != tt.expired { - t.Errorf("expire failed: expected %v, actual %v", tt.expired, expired) - } - } -} diff --git a/src/pkg/net/fd_unix.go b/src/pkg/net/fd_unix.go index 31709aead0..9326b6278a 100644 --- a/src/pkg/net/fd_unix.go +++ b/src/pkg/net/fd_unix.go @@ -11,7 +11,6 @@ import ( "os" "runtime" "sync" - "sync/atomic" "syscall" "time" ) @@ -38,11 +37,11 @@ type netFD struct { laddr Addr raddr Addr - // serialize access to Read and Write methods - rio, wio sync.Mutex - - // read and write deadlines - rdeadline, wdeadline deadline + // owned by client + rdeadline int64 + rio sync.Mutex + wdeadline int64 + wio sync.Mutex // owned by fd wait server ncr, ncw int @@ -51,31 +50,6 @@ type netFD struct { pollServer *pollServer } -// deadline is an atomically-accessed number of nanoseconds since 1970 -// or 0, if no deadline is set. -type deadline int64 - -func (d *deadline) expired() bool { - t := d.value() - return t > 0 && time.Now().UnixNano() >= t -} - -func (d *deadline) value() int64 { - return atomic.LoadInt64((*int64)(d)) -} - -func (d *deadline) set(v int64) { - atomic.StoreInt64((*int64)(d), v) -} - -func (d *deadline) setTime(t time.Time) { - if t.IsZero() { - d.set(0) - } else { - d.set(t.UnixNano()) - } -} - // A pollServer helps FDs determine when to retry a non-blocking // read or write after they get EAGAIN. When an FD needs to wait, // call s.WaitRead() or s.WaitWrite() to pass the request to the poll server. @@ -108,11 +82,11 @@ func (s *pollServer) AddFD(fd *netFD, mode int) error { key := intfd << 1 if mode == 'r' { fd.ncr++ - t = fd.rdeadline.value() + t = fd.rdeadline } else { fd.ncw++ key++ - t = fd.wdeadline.value() + t = fd.wdeadline } s.pending[key] = fd doWakeup := false @@ -179,8 +153,12 @@ func (s *pollServer) WakeFD(fd *netFD, mode int, err error) { } } +func (s *pollServer) Now() int64 { + return time.Now().UnixNano() +} + func (s *pollServer) CheckDeadlines() { - now := time.Now().UnixNano() + now := s.Now() // TODO(rsc): This will need to be handled more efficiently, // probably with a heap indexed by wakeup time. @@ -194,9 +172,9 @@ func (s *pollServer) CheckDeadlines() { mode = 'w' } if mode == 'r' { - t = fd.rdeadline.value() + t = fd.rdeadline } else { - t = fd.wdeadline.value() + t = fd.wdeadline } if t > 0 { if t <= now { @@ -220,15 +198,15 @@ func (s *pollServer) Run() { s.Lock() defer s.Unlock() for { - var timeout int64 // nsec to wait for or 0 for none - if s.deadline > 0 { - timeout = s.deadline - time.Now().UnixNano() - if timeout <= 0 { + var t = s.deadline + if t > 0 { + t = t - s.Now() + if t <= 0 { s.CheckDeadlines() continue } } - fd, mode, err := s.poll.WaitFD(s, timeout) + fd, mode, err := s.poll.WaitFD(s, t) if err != nil { print("pollServer WaitFD: ", err.Error(), "\n") return @@ -439,9 +417,11 @@ func (fd *netFD) Read(p []byte) (n int, err error) { } defer fd.decref() for { - if fd.rdeadline.expired() { - err = errTimeout - break + if fd.rdeadline > 0 { + if time.Now().UnixNano() >= fd.rdeadline { + err = errTimeout + break + } } n, err = syscall.Read(int(fd.sysfd), p) if err != nil { @@ -469,9 +449,11 @@ func (fd *netFD) ReadFrom(p []byte) (n int, sa syscall.Sockaddr, err error) { } defer fd.decref() for { - if fd.rdeadline.expired() { - err = errTimeout - break + if fd.rdeadline > 0 { + if time.Now().UnixNano() >= fd.rdeadline { + err = errTimeout + break + } } n, sa, err = syscall.Recvfrom(fd.sysfd, p, 0) if err != nil { @@ -499,13 +481,15 @@ func (fd *netFD) ReadMsg(p []byte, oob []byte) (n, oobn, flags int, sa syscall.S } defer fd.decref() for { - if fd.rdeadline.expired() { - err = errTimeout - break + if fd.rdeadline > 0 { + if time.Now().UnixNano() >= fd.rdeadline { + err = errTimeout + break + } } n, oobn, flags, sa, err = syscall.Recvmsg(fd.sysfd, p, oob, 0) if err != nil { - // TODO(dfc) should n and oobn be set to 0 + // TODO(dfc) should n and oobn be set to nil if err == syscall.EAGAIN { if err = fd.pollServer.WaitRead(fd); err == nil { continue @@ -528,17 +512,21 @@ func chkReadErr(n int, err error, fd *netFD) error { return err } -func (fd *netFD) Write(p []byte) (nn int, err error) { +func (fd *netFD) Write(p []byte) (int, error) { fd.wio.Lock() defer fd.wio.Unlock() if err := fd.incref(false); err != nil { return 0, err } defer fd.decref() + var err error + nn := 0 for { - if fd.wdeadline.expired() { - err = errTimeout - break + if fd.wdeadline > 0 { + if time.Now().UnixNano() >= fd.wdeadline { + err = errTimeout + break + } } var n int n, err = syscall.Write(int(fd.sysfd), p[nn:]) @@ -576,9 +564,11 @@ func (fd *netFD) WriteTo(p []byte, sa syscall.Sockaddr) (n int, err error) { } defer fd.decref() for { - if fd.wdeadline.expired() { - err = errTimeout - break + if fd.wdeadline > 0 { + if time.Now().UnixNano() >= fd.wdeadline { + err = errTimeout + break + } } err = syscall.Sendto(fd.sysfd, p, 0, sa) if err == syscall.EAGAIN { @@ -604,9 +594,11 @@ func (fd *netFD) WriteMsg(p []byte, oob []byte, sa syscall.Sockaddr) (n int, oob } defer fd.decref() for { - if fd.wdeadline.expired() { - err = errTimeout - break + if fd.wdeadline > 0 { + if time.Now().UnixNano() >= fd.wdeadline { + err = errTimeout + break + } } err = syscall.Sendmsg(fd.sysfd, p, oob, sa, 0) if err == syscall.EAGAIN { diff --git a/src/pkg/net/fd_windows.go b/src/pkg/net/fd_windows.go index 4cffcc8046..351f9271c7 100644 --- a/src/pkg/net/fd_windows.go +++ b/src/pkg/net/fd_windows.go @@ -285,39 +285,11 @@ type netFD struct { errnoc [2]chan error // read/write submit or cancel operation errors closec chan bool // used by Close to cancel pending IO - // serialize access to Read and Write methods - rio, wio sync.Mutex - - // read and write deadlines - rdeadline, wdeadline deadline -} - -// deadline is a number of nanoseconds since 1970 or 0, if no deadline is set. -// For compatability, deadline has the same method set as fd_unix.go, but -// does not use atomic operations as it is not known if data races exist on -// these values. -// TODO(dfc,brainman) when we get a windows race builder, revisit this. -type deadline int64 - -func (d *deadline) expired() bool { - t := d.value() - return t > 0 && time.Now().UnixNano() >= t -} - -func (d *deadline) value() int64 { - return int64(*d) -} - -func (d *deadline) set(v int64) { - *d = deadline(v) -} - -func (d *deadline) setTime(t time.Time) { - if t.IsZero() { - d.set(0) - } else { - d.set(t.UnixNano()) - } + // owned by client + rdeadline int64 + rio sync.Mutex + wdeadline int64 + wio sync.Mutex } func allocFD(fd syscall.Handle, family, sotype int, net string) *netFD { @@ -450,7 +422,7 @@ func (fd *netFD) Read(buf []byte) (int, error) { defer fd.rio.Unlock() var o readOp o.Init(fd, buf, 'r') - n, err := iosrv.ExecIO(&o, fd.rdeadline.value()) + n, err := iosrv.ExecIO(&o, fd.rdeadline) if err == nil && n == 0 { err = io.EOF } @@ -487,7 +459,7 @@ func (fd *netFD) ReadFrom(buf []byte) (n int, sa syscall.Sockaddr, err error) { var o readFromOp o.Init(fd, buf, 'r') o.rsan = int32(unsafe.Sizeof(o.rsa)) - n, err = iosrv.ExecIO(&o, fd.rdeadline.value()) + n, err = iosrv.ExecIO(&o, fd.rdeadline) if err != nil { return 0, nil, err } @@ -519,7 +491,7 @@ func (fd *netFD) Write(buf []byte) (int, error) { defer fd.wio.Unlock() var o writeOp o.Init(fd, buf, 'w') - return iosrv.ExecIO(&o, fd.wdeadline.value()) + return iosrv.ExecIO(&o, fd.wdeadline) } // WriteTo to network. @@ -551,7 +523,7 @@ func (fd *netFD) WriteTo(buf []byte, sa syscall.Sockaddr) (int, error) { var o writeToOp o.Init(fd, buf, 'w') o.sa = sa - return iosrv.ExecIO(&o, fd.wdeadline.value()) + return iosrv.ExecIO(&o, fd.wdeadline) } // Accept new network connections. @@ -600,7 +572,7 @@ func (fd *netFD) accept(toAddr func(syscall.Sockaddr) Addr) (*netFD, error) { var o acceptOp o.Init(fd, 'r') o.newsock = s - _, err = iosrv.ExecIO(&o, fd.rdeadline.value()) + _, err = iosrv.ExecIO(&o, fd.rdeadline) if err != nil { closesocket(s) return nil, err diff --git a/src/pkg/net/sendfile_freebsd.go b/src/pkg/net/sendfile_freebsd.go index 8008bc3b56..8500006104 100644 --- a/src/pkg/net/sendfile_freebsd.go +++ b/src/pkg/net/sendfile_freebsd.go @@ -82,7 +82,7 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) { if n == 0 && err1 == nil { break } - if err1 == syscall.EAGAIN { + if err1 == syscall.EAGAIN && c.wdeadline >= 0 { if err1 = c.pollServer.WaitWrite(c); err1 == nil { continue } diff --git a/src/pkg/net/sendfile_linux.go b/src/pkg/net/sendfile_linux.go index 3357e65386..5ee18f9ccc 100644 --- a/src/pkg/net/sendfile_linux.go +++ b/src/pkg/net/sendfile_linux.go @@ -58,7 +58,7 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) { if n == 0 && err1 == nil { break } - if err1 == syscall.EAGAIN { + if err1 == syscall.EAGAIN && c.wdeadline >= 0 { if err1 = c.pollServer.WaitWrite(c); err1 == nil { continue } diff --git a/src/pkg/net/sock_posix.go b/src/pkg/net/sock_posix.go index 2fd9053cc1..78417fd2ee 100644 --- a/src/pkg/net/sock_posix.go +++ b/src/pkg/net/sock_posix.go @@ -57,14 +57,16 @@ func socket(net string, f, t, p int, ipv6only bool, ulsa, ursa syscall.Sockaddr, } if ursa != nil { - fd.wdeadline.setTime(deadline) + if !deadline.IsZero() { + fd.wdeadline = deadline.UnixNano() + } if err = fd.connect(ursa); err != nil { closesocket(s) fd.Close() return nil, err } fd.isConnected = true - fd.wdeadline.set(0) + fd.wdeadline = 0 } lsa, _ := syscall.Getsockname(s) diff --git a/src/pkg/net/sockopt_posix.go b/src/pkg/net/sockopt_posix.go index fe371fe0ce..b139c42765 100644 --- a/src/pkg/net/sockopt_posix.go +++ b/src/pkg/net/sockopt_posix.go @@ -119,22 +119,29 @@ func setWriteBuffer(fd *netFD, bytes int) error { return os.NewSyscallError("setsockopt", syscall.SetsockoptInt(fd.sysfd, syscall.SOL_SOCKET, syscall.SO_SNDBUF, bytes)) } -// TODO(dfc) these unused error returns could be removed - func setReadDeadline(fd *netFD, t time.Time) error { - fd.rdeadline.setTime(t) + if t.IsZero() { + fd.rdeadline = 0 + } else { + fd.rdeadline = t.UnixNano() + } return nil } func setWriteDeadline(fd *netFD, t time.Time) error { - fd.wdeadline.setTime(t) + if t.IsZero() { + fd.wdeadline = 0 + } else { + fd.wdeadline = t.UnixNano() + } return nil } func setDeadline(fd *netFD, t time.Time) error { - setReadDeadline(fd, t) - setWriteDeadline(fd, t) - return nil + if err := setReadDeadline(fd, t); err != nil { + return err + } + return setWriteDeadline(fd, t) } func setKeepAlive(fd *netFD, keepalive bool) error { -- 2.48.1