Skip to content

Commit bd388c0

Browse files
neildneild
committed
internal/poll: keep copying after successful Sendfile return on BSD
The BSD implementation of poll.SendFile incorrectly halted copying after succesfully writing one full chunk of data. Adjust the copy loop to match the Linux and Solaris implementations. In testing, empirically macOS appears to sometimes return EAGAIN from sendfile after successfully copying a full chunk. Add a check to all implementations to return nil after successfully copying all data if the last sendfile call returns EAGAIN. For #70000 Change-Id: I57ba649491fc078c7330310b23e1cfd85135c8ff Reviewed-on: https://go-review.googlesource.com/c/go/+/622235 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 28b09d5 commit bd388c0

5 files changed

Lines changed: 171 additions & 49 deletions

File tree

‎src/internal/poll/sendfile_bsd.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,25 @@ func SendFile(dstFD *FD, src int, pos, remain int64) (written int64, err error,
4141
pos += int64(n)
4242
written += int64(n)
4343
remain -= int64(n)
44+
continue
45+
} else if err != syscall.EAGAIN && err != syscall.EINTR {
46+
// This includes syscall.ENOSYS (no kernel
47+
// support) and syscall.EINVAL (fd types which
48+
// don't implement sendfile), and other errors.
49+
// We should end the loop when there is no error
50+
// returned from sendfile(2) or it is not a retryable error.
51+
break
4452
}
4553
if err == syscall.EINTR {
4654
continue
4755
}
48-
// This includes syscall.ENOSYS (no kernel
49-
// support) and syscall.EINVAL (fd types which
50-
// don't implement sendfile), and other errors.
51-
// We should end the loop when there is no error
52-
// returned from sendfile(2) or it is not a retryable error.
53-
if err != syscall.EAGAIN {
54-
break
55-
}
5656
if err = dstFD.pd.waitWrite(dstFD.isFile); err != nil {
5757
break
5858
}
5959
}
60+
if err == syscall.EAGAIN {
61+
err = nil
62+
}
6063
handled = written != 0 || (err != syscall.ENOSYS && err != syscall.EINVAL)
6164
return
6265
}

‎src/internal/poll/sendfile_linux.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ func SendFile(dstFD *FD, src int, remain int64) (written int64, err error, handl
5353
break
5454
}
5555
}
56+
if err == syscall.EAGAIN {
57+
err = nil
58+
}
5659
handled = written != 0 || (err != syscall.ENOSYS && err != syscall.EINVAL)
5760
return
5861
}

‎src/internal/poll/sendfile_solaris.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ func SendFile(dstFD *FD, src int, pos, remain int64) (written int64, err error,
7171
break
7272
}
7373
}
74+
if err == syscall.EAGAIN {
75+
err = nil
76+
}
7477
handled = written != 0 || (err != syscall.ENOSYS && err != syscall.EINVAL)
7578
return
7679
}

‎src/os/copy_test.go‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package os_test
6+
7+
import (
8+
"bytes"
9+
"errors"
10+
"io"
11+
"math/rand/v2"
12+
"net"
13+
"os"
14+
"runtime"
15+
"sync"
16+
"testing"
17+
18+
"golang.org/x/net/nettest"
19+
)
20+
21+
// Exercise sendfile/splice fast paths with a moderately large file.
22+
//
23+
// https://go.dev/issue/70000
24+
25+
func TestLargeCopyViaNetwork(t *testing.T) {
26+
const size = 10 * 1024 * 1024
27+
dir := t.TempDir()
28+
29+
src, err := os.Create(dir + "/src")
30+
if err != nil {
31+
t.Fatal(err)
32+
}
33+
defer src.Close()
34+
if _, err := io.CopyN(src, newRandReader(), size); err != nil {
35+
t.Fatal(err)
36+
}
37+
if _, err := src.Seek(0, 0); err != nil {
38+
t.Fatal(err)
39+
}
40+
41+
dst, err := os.Create(dir + "/dst")
42+
if err != nil {
43+
t.Fatal(err)
44+
}
45+
defer dst.Close()
46+
47+
client, server := createSocketPair(t, "tcp")
48+
var wg sync.WaitGroup
49+
wg.Add(2)
50+
go func() {
51+
defer wg.Done()
52+
if n, err := io.Copy(dst, server); n != size || err != nil {
53+
t.Errorf("copy to destination = %v, %v; want %v, nil", n, err, size)
54+
}
55+
}()
56+
go func() {
57+
defer wg.Done()
58+
defer client.Close()
59+
if n, err := io.Copy(client, src); n != size || err != nil {
60+
t.Errorf("copy from source = %v, %v; want %v, nil", n, err, size)
61+
}
62+
}()
63+
wg.Wait()
64+
65+
if _, err := dst.Seek(0, 0); err != nil {
66+
t.Fatal(err)
67+
}
68+
if err := compareReaders(dst, io.LimitReader(newRandReader(), size)); err != nil {
69+
t.Fatal(err)
70+
}
71+
}
72+
73+
func compareReaders(a, b io.Reader) error {
74+
bufa := make([]byte, 4096)
75+
bufb := make([]byte, 4096)
76+
for {
77+
na, erra := io.ReadFull(a, bufa)
78+
if erra != nil && erra != io.EOF {
79+
return erra
80+
}
81+
nb, errb := io.ReadFull(b, bufb)
82+
if errb != nil && errb != io.EOF {
83+
return errb
84+
}
85+
if !bytes.Equal(bufa[:na], bufb[:nb]) {
86+
return errors.New("contents mismatch")
87+
}
88+
if erra == io.EOF && errb == io.EOF {
89+
break
90+
}
91+
}
92+
return nil
93+
}
94+
95+
type randReader struct {
96+
rand *rand.Rand
97+
}
98+
99+
func newRandReader() *randReader {
100+
return &randReader{rand.New(rand.NewPCG(0, 0))}
101+
}
102+
103+
func (r *randReader) Read(p []byte) (int, error) {
104+
var v uint64
105+
var n int
106+
for i := range p {
107+
if n == 0 {
108+
v = r.rand.Uint64()
109+
n = 8
110+
}
111+
p[i] = byte(v & 0xff)
112+
v >>= 8
113+
n--
114+
}
115+
return len(p), nil
116+
}
117+
118+
func createSocketPair(t *testing.T, proto string) (client, server net.Conn) {
119+
t.Helper()
120+
if !nettest.TestableNetwork(proto) {
121+
t.Skipf("%s does not support %q", runtime.GOOS, proto)
122+
}
123+
124+
ln, err := nettest.NewLocalListener(proto)
125+
if err != nil {
126+
t.Fatalf("NewLocalListener error: %v", err)
127+
}
128+
t.Cleanup(func() {
129+
if ln != nil {
130+
ln.Close()
131+
}
132+
if client != nil {
133+
client.Close()
134+
}
135+
if server != nil {
136+
server.Close()
137+
}
138+
})
139+
ch := make(chan struct{})
140+
go func() {
141+
var err error
142+
server, err = ln.Accept()
143+
if err != nil {
144+
t.Errorf("Accept new connection error: %v", err)
145+
}
146+
ch <- struct{}{}
147+
}()
148+
client, err = net.Dial(proto, ln.Addr().String())
149+
<-ch
150+
if err != nil {
151+
t.Fatalf("Dial new connection error: %v", err)
152+
}
153+
return client, server
154+
}

‎src/os/readfrom_linux_test.go‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,11 @@ import (
1414
"net"
1515
. "os"
1616
"path/filepath"
17-
"runtime"
1817
"strconv"
1918
"sync"
2019
"syscall"
2120
"testing"
2221
"time"
23-
24-
"golang.org/x/net/nettest"
2522
)
2623

2724
func TestSpliceFile(t *testing.T) {
@@ -479,41 +476,3 @@ func testGetPollFDAndNetwork(t *testing.T, proto string) {
479476
t.Fatalf("server Control error: %v", err)
480477
}
481478
}
482-
483-
func createSocketPair(t *testing.T, proto string) (client, server net.Conn) {
484-
t.Helper()
485-
if !nettest.TestableNetwork(proto) {
486-
t.Skipf("%s does not support %q", runtime.GOOS, proto)
487-
}
488-
489-
ln, err := nettest.NewLocalListener(proto)
490-
if err != nil {
491-
t.Fatalf("NewLocalListener error: %v", err)
492-
}
493-
t.Cleanup(func() {
494-
if ln != nil {
495-
ln.Close()
496-
}
497-
if client != nil {
498-
client.Close()
499-
}
500-
if server != nil {
501-
server.Close()
502-
}
503-
})
504-
ch := make(chan struct{})
505-
go func() {
506-
var err error
507-
server, err = ln.Accept()
508-
if err != nil {
509-
t.Errorf("Accept new connection error: %v", err)
510-
}
511-
ch <- struct{}{}
512-
}()
513-
client, err = net.Dial(proto, ln.Addr().String())
514-
<-ch
515-
if err != nil {
516-
t.Fatalf("Dial new connection error: %v", err)
517-
}
518-
return client, server
519-
}

0 commit comments

Comments
 (0)