os: fix race in ReadAt/WriteAt on Windows

R=bradfitzgo, rsc, peterGo
CC=golang-dev
https://golang.org/cl/4441051

5 files changed, 147 insertions, 46 deletions

diff --git a/src/pkg/os/file.go b/src/pkg/os/file.go
index 643b225ce7..dff8fa862c 100644
--- a/src/pkg/os/file.go
+++ b/src/pkg/os/file.go
@@ -8,6 +8,7 @@ package os
 
 import (
 	"runtime"
+	"sync"
 	"syscall"
 )
 
@@ -15,8 +16,9 @@ import (
 type File struct {
 	fd      int
 	name    string
-	dirinfo *dirInfo // nil unless directory being read
-	nepipe  int      // number of consecutive EPIPE in Write
+	dirinfo *dirInfo   // nil unless directory being read
+	nepipe  int        // number of consecutive EPIPE in Write
+	l       sync.Mutex // used to implement windows pread/pwrite
 }
 
 // Fd returns the integer Unix file descriptor referencing the open file.
@@ -30,7 +32,7 @@ func NewFile(fd int, name string) *File {
 	if fd < 0 {
 		return nil
 	}
-	f := &File{fd, name, nil, 0}
+	f := &File{fd: fd, name: name}
 	runtime.SetFinalizer(f, (*File).Close)
 	return f
 }
@@ -85,7 +87,7 @@ func (file *File) Read(b []byte) (n int, err Error) {
 	if file == nil {
 		return 0, EINVAL
 	}
-	n, e := syscall.Read(file.fd, b)
+	n, e := file.read(b)
 	if n < 0 {
 		n = 0
 	}
@@ -107,7 +109,7 @@ func (file *File) ReadAt(b []byte, off int64) (n int, err Error) {
 		return 0, EINVAL
 	}
 	for len(b) > 0 {
-		m, e := syscall.Pread(file.fd, b, off)
+		m, e := file.pread(b, off)
 		if m == 0 && !iserror(e) {
 			return n, EOF
 		}
@@ -129,7 +131,7 @@ func (file *File) Write(b []byte) (n int, err Error) {
 	if file == nil {
 		return 0, EINVAL
 	}
-	n, e := syscall.Write(file.fd, b)
+	n, e := file.write(b)
 	if n < 0 {
 		n = 0
 	}
@@ -150,7 +152,7 @@ func (file *File) WriteAt(b []byte, off int64) (n int, err Error) {
 		return 0, EINVAL
 	}
 	for len(b) > 0 {
-		m, e := syscall.Pwrite(file.fd, b, off)
+		m, e := file.pwrite(b, off)
 		if iserror(e) {
 			err = &PathError{"write", file.name, Errno(e)}
 			break
@@ -167,7 +169,7 @@ func (file *File) WriteAt(b []byte, off int64) (n int, err Error) {
 // relative to the current offset, and 2 means relative to the end.
 // It returns the new offset and an Error, if any.
 func (file *File) Seek(offset int64, whence int) (ret int64, err Error) {
-	r, e := syscall.Seek(file.fd, offset, whence)
+	r, e := file.seek(offset, whence)
 	if !iserror(e) && file.dirinfo != nil && r != 0 {
 		e = syscall.EISDIR
 	}
diff --git a/src/pkg/os/file_plan9.go b/src/pkg/os/file_plan9.go
index c8d0efba40..7b473f8022 100644
--- a/src/pkg/os/file_plan9.go
+++ b/src/pkg/os/file_plan9.go
@@ -117,6 +117,39 @@ func (f *File) Sync() (err Error) {
 	return nil
 }
 
+// read reads up to len(b) bytes from the File.
+// It returns the number of bytes read and an error, if any.
+func (f *File) read(b []byte) (n int, err syscall.Error) {
+	return syscall.Read(f.fd, b)
+}
+
+// pread reads len(b) bytes from the File starting at byte offset off.
+// It returns the number of bytes read and the error, if any.
+// EOF is signaled by a zero count with err set to nil.
+func (f *File) pread(b []byte, off int64) (n int, err syscall.Error) {
+	return syscall.Pread(f.fd, b, off)
+}
+
+// write writes len(b) bytes to the File.
+// It returns the number of bytes written and an error, if any.
+func (f *File) write(b []byte) (n int, err syscall.Error) {
+	return syscall.Write(f.fd, b)
+}
+
+// pwrite writes len(b) bytes to the File starting at byte offset off.
+// It returns the number of bytes written and an error, if any.
+func (f *File) pwrite(b []byte, off int64) (n int, err syscall.Error) {
+	return syscall.Pwrite(f.fd, b, off)
+}
+
+// seek sets the offset for the next Read or Write on file to offset, interpreted
+// according to whence: 0 means relative to the origin of the file, 1 means
+// relative to the current offset, and 2 means relative to the end.
+// It returns the new offset and an error, if any.
+func (f *File) seek(offset int64, whence int) (ret int64, err syscall.Error) {
+	return syscall.Seek(f.fd, offset, whence)
+}
+
 // Truncate changes the size of the named file.
 // If the file is a symbolic link, it changes the size of the link's target.
 func Truncate(name string, size int64) Error {
diff --git a/src/pkg/os/file_unix.go b/src/pkg/os/file_unix.go
index f2b94f4c2d..2fb28df655 100644
--- a/src/pkg/os/file_unix.go
+++ b/src/pkg/os/file_unix.go
@@ -96,6 +96,39 @@ func (file *File) Readdir(count int) (fi []FileInfo, err Error) {
 	return
 }
 
+// read reads up to len(b) bytes from the File.
+// It returns the number of bytes read and an error, if any.
+func (f *File) read(b []byte) (n int, err int) {
+	return syscall.Read(f.fd, b)
+}
+
+// pread reads len(b) bytes from the File starting at byte offset off.
+// It returns the number of bytes read and the error, if any.
+// EOF is signaled by a zero count with err set to 0.
+func (f *File) pread(b []byte, off int64) (n int, err int) {
+	return syscall.Pread(f.fd, b, off)
+}
+
+// write writes len(b) bytes to the File.
+// It returns the number of bytes written and an error, if any.
+func (f *File) write(b []byte) (n int, err int) {
+	return syscall.Write(f.fd, b)
+}
+
+// pwrite writes len(b) bytes to the File starting at byte offset off.
+// It returns the number of bytes written and an error, if any.
+func (f *File) pwrite(b []byte, off int64) (n int, err int) {
+	return syscall.Pwrite(f.fd, b, off)
+}
+
+// seek sets the offset for the next Read or Write on file to offset, interpreted
+// according to whence: 0 means relative to the origin of the file, 1 means
+// relative to the current offset, and 2 means relative to the end.
+// It returns the new offset and an error, if any.
+func (f *File) seek(offset int64, whence int) (ret int64, err int) {
+	return syscall.Seek(f.fd, offset, whence)
+}
+
 // Truncate changes the size of the named file.
 // If the file is a symbolic link, it changes the size of the link's target.
 func Truncate(name string, size int64) Error {
diff --git a/src/pkg/os/file_windows.go b/src/pkg/os/file_windows.go
index 862baf6b91..95f60b7351 100644
--- a/src/pkg/os/file_windows.go
+++ b/src/pkg/os/file_windows.go
@@ -165,6 +165,77 @@ func (file *File) Readdir(count int) (fi []FileInfo, err Error) {
 	return fi, nil
 }
 
+// read reads up to len(b) bytes from the File.
+// It returns the number of bytes read and an error, if any.
+func (f *File) read(b []byte) (n int, err int) {
+	f.l.Lock()
+	defer f.l.Unlock()
+	return syscall.Read(f.fd, b)
+}
+
+// pread reads len(b) bytes from the File starting at byte offset off.
+// It returns the number of bytes read and the error, if any.
+// EOF is signaled by a zero count with err set to 0.
+func (f *File) pread(b []byte, off int64) (n int, err int) {
+	f.l.Lock()
+	defer f.l.Unlock()
+	curoffset, e := syscall.Seek(f.fd, 0, 1)
+	if e != 0 {
+		return 0, e
+	}
+	defer syscall.Seek(f.fd, curoffset, 0)
+	o := syscall.Overlapped{
+		OffsetHigh: uint32(off >> 32),
+		Offset:     uint32(off),
+	}
+	var done uint32
+	e = syscall.ReadFile(int32(f.fd), b, &done, &o)
+	if e != 0 {
+		return 0, e
+	}
+	return int(done), 0
+}
+
+// write writes len(b) bytes to the File.
+// It returns the number of bytes written and an error, if any.
+func (f *File) write(b []byte) (n int, err int) {
+	f.l.Lock()
+	defer f.l.Unlock()
+	return syscall.Write(f.fd, b)
+}
+
+// pwrite writes len(b) bytes to the File starting at byte offset off.
+// It returns the number of bytes written and an error, if any.
+func (f *File) pwrite(b []byte, off int64) (n int, err int) {
+	f.l.Lock()
+	defer f.l.Unlock()
+	curoffset, e := syscall.Seek(f.fd, 0, 1)
+	if e != 0 {
+		return 0, e
+	}
+	defer syscall.Seek(f.fd, curoffset, 0)
+	o := syscall.Overlapped{
+		OffsetHigh: uint32(off >> 32),
+		Offset:     uint32(off),
+	}
+	var done uint32
+	e = syscall.WriteFile(int32(f.fd), b, &done, &o)
+	if e != 0 {
+		return 0, e
+	}
+	return int(done), 0
+}
+
+// seek sets the offset for the next Read or Write on file to offset, interpreted
+// according to whence: 0 means relative to the origin of the file, 1 means
+// relative to the current offset, and 2 means relative to the end.
+// It returns the new offset and an error, if any.
+func (f *File) seek(offset int64, whence int) (ret int64, err int) {
+	f.l.Lock()
+	defer f.l.Unlock()
+	return syscall.Seek(f.fd, offset, whence)
+}
+
 // Truncate changes the size of the named file.
 // If the file is a symbolic link, it changes the size of the link's target.
 func Truncate(name string, size int64) Error {
diff --git a/src/pkg/syscall/syscall_windows.go b/src/pkg/syscall/syscall_windows.go
index e01310deff..1fbb3ccbf4 100644
--- a/src/pkg/syscall/syscall_windows.go
+++ b/src/pkg/syscall/syscall_windows.go
@@ -250,27 +250,6 @@ func Read(fd int, p []byte) (n int, errno int) {
 	return int(done), 0
 }
 
-// TODO(brainman): ReadFile/WriteFile change file offset, therefore
-// i use Seek here to preserve semantics of unix pread/pwrite,
-// not sure if I should do that
-
-func Pread(fd int, p []byte, offset int64) (n int, errno int) {
-	curoffset, e := Seek(fd, 0, 1)
-	if e != 0 {
-		return 0, e
-	}
-	defer Seek(fd, curoffset, 0)
-	var o Overlapped
-	o.OffsetHigh = uint32(offset >> 32)
-	o.Offset = uint32(offset)
-	var done uint32
-	e = ReadFile(int32(fd), p, &done, &o)
-	if e != 0 {
-		return 0, e
-	}
-	return int(done), 0
-}
-
 func Write(fd int, p []byte) (n int, errno int) {
 	var done uint32
 	e := WriteFile(int32(fd), p, &done, nil)
@@ -280,23 +259,6 @@ func Write(fd int, p []byte) (n int, errno int) {
 	return int(done), 0
 }
 
-func Pwrite(fd int, p []byte, offset int64) (n int, errno int) {
-	curoffset, e := Seek(fd, 0, 1)
-	if e != 0 {
-		return 0, e
-	}
-	defer Seek(fd, curoffset, 0)
-	var o Overlapped
-	o.OffsetHigh = uint32(offset >> 32)
-	o.Offset = uint32(offset)
-	var done uint32
-	e = WriteFile(int32(fd), p, &done, &o)
-	if e != 0 {
-		return 0, e
-	}
-	return int(done), 0
-}
-
 func Seek(fd int, offset int64, whence int) (newoffset int64, errno int) {
 	var w uint32
 	switch whence {