Skip to content

Commit aa4d5e7

Browse files
committed
syscall,internal/poll: move pipe check from syscall.Seek to callers
On Windows, syscall.Seek is a thin wrapper over SetFilePointerEx [1], which does not work on pipes, although it doesn't return an error on that case. To avoid this undefined behavior, Seek defensively calls GetFileType and errors if the type is FILE_TYPE_PIPE. The problem with this approach is that Seek is a low level foundational function that can be called many times for the same file, and the additional cgo call (GetFileType) will artificially slow down seek operations. I've seen GetFileType to account for 10% of cpu time in seek-intensive workloads. A better approach, implemented in this CL, would be to move the check one level up, where many times the file type is already known so the GetFileType is unnecessary. The drawback is that syscall.Seek has had this behavior since pipes where first introduced to Windows in https://codereview.appspot.com/1715046 and someone could be relying on it. On the other hand, this behavior is not documented, so we couldn't be breaking any contract. [1] https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfilepointerex Change-Id: I7602182f9d08632e22a8a1635bc8ad9ad35a5056 Reviewed-on: https://go-review.googlesource.com/c/go/+/493626 Run-TryBot: Quim Muntal <[email protected]> Reviewed-by: Alex Brainman <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Bryan Mills <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 841e99e commit aa4d5e7

File tree

3 files changed

+14
-5
lines changed

3 files changed

+14
-5
lines changed

src/internal/poll/fd_windows.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,10 @@ func (fd *FD) readConsole(b []byte) (int, error) {
522522

523523
// Pread emulates the Unix pread system call.
524524
func (fd *FD) Pread(b []byte, off int64) (int, error) {
525+
if fd.kind == kindPipe {
526+
// Pread does not work with pipes
527+
return 0, syscall.ESPIPE
528+
}
525529
// Call incref, not readLock, because since pread specifies the
526530
// offset it is independent from other reads.
527531
if err := fd.incref(); err != nil {
@@ -744,6 +748,10 @@ func (fd *FD) writeConsole(b []byte) (int, error) {
744748

745749
// Pwrite emulates the Unix pwrite system call.
746750
func (fd *FD) Pwrite(buf []byte, off int64) (int, error) {
751+
if fd.kind == kindPipe {
752+
// Pwrite does not work with pipes
753+
return 0, syscall.ESPIPE
754+
}
747755
// Call incref, not writeLock, because since pwrite specifies the
748756
// offset it is independent from other writes.
749757
if err := fd.incref(); err != nil {
@@ -992,6 +1000,9 @@ func (fd *FD) Accept(sysSocket func() (syscall.Handle, error)) (syscall.Handle,
9921000

9931001
// Seek wraps syscall.Seek.
9941002
func (fd *FD) Seek(offset int64, whence int) (int64, error) {
1003+
if fd.kind == kindPipe {
1004+
return 0, syscall.ESPIPE
1005+
}
9951006
if err := fd.incref(); err != nil {
9961007
return 0, err
9971008
}

src/internal/poll/sendfile_windows.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ func SendFile(fd *FD, src syscall.Handle, n int64) (written int64, err error) {
1515
// TransmitFile does not work with pipes
1616
return 0, syscall.ESPIPE
1717
}
18+
if ft, _ := syscall.GetFileType(src); ft == syscall.FILE_TYPE_PIPE {
19+
return 0, syscall.ESPIPE
20+
}
1821

1922
if err := fd.writeLock(); err != nil {
2023
return 0, err

src/syscall/syscall_windows.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -498,11 +498,6 @@ func Seek(fd Handle, offset int64, whence int) (newoffset int64, err error) {
498498
case 2:
499499
w = FILE_END
500500
}
501-
// use GetFileType to check pipe, pipe can't do seek
502-
ft, _ := GetFileType(fd)
503-
if ft == FILE_TYPE_PIPE {
504-
return 0, ESPIPE
505-
}
506501
err = setFilePointerEx(fd, offset, &newoffset, w)
507502
return
508503
}

0 commit comments

Comments
 (0)