From: qmuntal Date: Tue, 9 May 2023 14:16:42 +0000 (+0200) Subject: syscall,internal/poll: move pipe check from syscall.Seek to callers X-Git-Tag: go1.21rc1~622 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=aa4d5e739f32397969fd5c33cbc95d316686039f;p=gostls13.git 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 Reviewed-by: Alex Brainman TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills Reviewed-by: Ian Lance Taylor --- diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go index f863ecb998..9df39edced 100644 --- a/src/internal/poll/fd_windows.go +++ b/src/internal/poll/fd_windows.go @@ -522,6 +522,10 @@ func (fd *FD) readConsole(b []byte) (int, error) { // Pread emulates the Unix pread system call. func (fd *FD) Pread(b []byte, off int64) (int, error) { + if fd.kind == kindPipe { + // Pread does not work with pipes + return 0, syscall.ESPIPE + } // Call incref, not readLock, because since pread specifies the // offset it is independent from other reads. if err := fd.incref(); err != nil { @@ -744,6 +748,10 @@ func (fd *FD) writeConsole(b []byte) (int, error) { // Pwrite emulates the Unix pwrite system call. func (fd *FD) Pwrite(buf []byte, off int64) (int, error) { + if fd.kind == kindPipe { + // Pwrite does not work with pipes + return 0, syscall.ESPIPE + } // Call incref, not writeLock, because since pwrite specifies the // offset it is independent from other writes. if err := fd.incref(); err != nil { @@ -992,6 +1000,9 @@ func (fd *FD) Accept(sysSocket func() (syscall.Handle, error)) (syscall.Handle, // Seek wraps syscall.Seek. func (fd *FD) Seek(offset int64, whence int) (int64, error) { + if fd.kind == kindPipe { + return 0, syscall.ESPIPE + } if err := fd.incref(); err != nil { return 0, err } diff --git a/src/internal/poll/sendfile_windows.go b/src/internal/poll/sendfile_windows.go index 50c3ee86c0..8c3353bc6f 100644 --- a/src/internal/poll/sendfile_windows.go +++ b/src/internal/poll/sendfile_windows.go @@ -15,6 +15,9 @@ func SendFile(fd *FD, src syscall.Handle, n int64) (written int64, err error) { // TransmitFile does not work with pipes return 0, syscall.ESPIPE } + if ft, _ := syscall.GetFileType(src); ft == syscall.FILE_TYPE_PIPE { + return 0, syscall.ESPIPE + } if err := fd.writeLock(); err != nil { return 0, err diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go index 8687d1cc21..cf6049a2f2 100644 --- a/src/syscall/syscall_windows.go +++ b/src/syscall/syscall_windows.go @@ -498,11 +498,6 @@ func Seek(fd Handle, offset int64, whence int) (newoffset int64, err error) { case 2: w = FILE_END } - // use GetFileType to check pipe, pipe can't do seek - ft, _ := GetFileType(fd) - if ft == FILE_TYPE_PIPE { - return 0, ESPIPE - } err = setFilePointerEx(fd, offset, &newoffset, w) return }