net: sendFile can't handle os.fileWithoutWriteTo

[philwo]

Go version

go version go1.22.2 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/philwo/Library/Caches/go-build'
GOENV='/Users/philwo/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/philwo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/philwo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.2/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/philwo/src/philtools/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/ft/3rpc4dkj09b3lw6pbm_j0j7w005v1x/T/go-build2443549281=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

1. Open a regular file f for reading.
2. Connect to a TCP socket s.
3. Copy f into s by calling io.Copy(s, f)

Example: https://go.dev/play/p/xJbcSMVNLqy (deadlocks when run on Go Playground due to go.dev/issue/48394)

What did you see happen?

io.Copy does not use sendfile on macOS to copy f into s, but instead falls back to using a buffer due to a regression caused by a side-effect of go.dev/cl/472475.

Stepping through the code with a debugger, I think this is what happens:

io.Copy implements three different strategies to copy src to `dst:

1. Call src.WriteTo(dst) if src implements the WriterTo interface.
2. Call dst.ReadFrom(src) if dst implements the ReaderFrom interface.
3. Use a buffer to handle the copying ourselves.

In the first iteration, this goes through os.File's WriteTo method, which calls into zero_copy_stub.go's writeTo, which is just a stub and thus can't do anything. It thus falls back to calling io.Copy again with itself wrapped into fileWithoutWriteTo, preventing it from taking the first branch.

In this second iteration, it thus calls ReadFrom on s and passes it f wrapped into fileWithoutWriteTo. tcpsock_posix.go attempts to use sendFile now, passing f into its r io.Reader parameter. sendFile needs to ensure that the passed Reader supports Stat, Seek and SyscallConn, and thus attempts a type assertion to *os.File.

However, this fails, because r isn't a File, it's a fileWithoutWriteTo.

Prior to go.dev/cl/472475 this all happened to work, because we never took the first branch in io.Copy, because os.File didn't implement io.WriterTo, so we passed the unmodified arg to the socket's ReadFrom.

What did you expect to see?

io.Copy should use sendfile on macOS to copy f into s, like it did prior to go.dev/cl/472475 (and still does on Linux, because os/zero_copy_linux.go handles it there and is not affected by the regression).

[gopherbot]

Change https://go.dev/cl/581035 mentions this issue: net: fix sendfile regression with io.Copy on macOS

[odeke-em]

Thank you for this report and catching this regression @philwo! Indeed restricting the reader to be an *os.File deprived other use cases IMHO. One way to ensure that the fix is for ever caught and doesn't trivially regress is by using a test such as one that I have crafted down below inside a new file net/sendfile_unix_alt_test.go

// Copyright 2024 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.

//go:build (darwin && !ios) || dragonfly || freebsd || solaris

package net

import (
	"errors"
	"io"
	"io/fs"
	"syscall"
	"testing"
	"time"
)

type sendFileMocker struct {
	cursor  int64
	content []byte

	seekInvoked        bool
	syscallConnInvoked bool
}

var _ fs.File = (*sendFileMocker)(nil)
var _ syscall.Conn = (*sendFileMocker)(nil)

func TestSendFileWorksOnNonOsFilesToo(t *testing.T) {
	ln, err := Listen("tcp", ":0")
	if err != nil {
		t.Fatal(err)
	}
	defer ln.Close()

	go func() {
		conn, err := ln.Accept()
		if err != nil {
			panic(err)
		}
		_, _ = io.Copy(io.Discard, conn)
	}()

	c, err := Dial("tcp", ln.Addr().String())
	if err != nil {
		t.Fatal(err)
	}
	defer c.Close()

	sfm := &sendFileMocker{content: []byte("abcdefghijklmno")}

	tcpConn := c.(*TCPConn)
	_, err, _ = sendFile(tcpConn.fd, sfm)
	if err == nil || err.Error() != "purposefully not available" {
		t.Fatal("Purposefully expected .SyscallConn to return an error")
	}

	// After this invocation, sfm's variables must be set to true.
	if !sfm.seekInvoked {
		t.Error(".Seek was not invoked!")
	}
	if !sfm.syscallConnInvoked {
		t.Error(".SyscallConn was not invoked!")
	}
}

func (sfm *sendFileMocker) SyscallConn() (syscall.RawConn, error) {
	sfm.syscallConnInvoked = true
	return nil, errors.New("purposefully not available")
}

func (sfm *sendFileMocker) Seek(offset int64, whence int) (int64, error) {
	sfm.seekInvoked = true
	if whence == io.SeekEnd {
		offset = -offset
	} else if whence == io.SeekStart {
		sfm.cursor = 0
	}
	sfm.cursor += offset
	return sfm.cursor, nil
}

func (sfm *sendFileMocker) Size() int64 {
	return int64(len(sfm.content))
}

func (sfm *sendFileMocker) Close() error {
	return nil
}

func (sfm *sendFileMocker) Stat() (fs.FileInfo, error) {
	return &mockFileInfo{sfm.Size()}, nil
}

type mockFileInfo struct{ size int64 }

var _ fs.FileInfo = (*mockFileInfo)(nil)

func (mfi *mockFileInfo) Name() string       { return "in-memory" }
func (mfi *mockFileInfo) Size() int64        { return mfi.size }
func (mfi *mockFileInfo) IsDir() bool        { return false }
func (mfi *mockFileInfo) Sys() any           { return nil }
func (mfi *mockFileInfo) Mode() fs.FileMode  { return 0 }
func (mfi *mockFileInfo) ModTime() time.Time { return time.Now() }

func (sfm *sendFileMocker) Read(b []byte) (int, error) {
	n := copy(b, sfm.content[sfm.cursor:])
	return n, nil
}

[gopherbot]

Change https://go.dev/cl/581778 mentions this issue: net, os, internal/poll: test for use of sendfile