Skip to content

Commit a5aa9c7

Browse files
matzfneild
matzf
authored and
neild
committed
internal/socket: handle reordering in TestUDP/Messages
TestUDP/Messages and TestUDP/Messages-dialed occasionally failed because the expected messages were not received in a single RecvMsgs call, or because the messages were received out of order. Assuming that both messages are returned immediately from a single RecvMsgs call was a flawed expectation. Fixed by repeatedly invoking RecvMsgs until all expected messages have been received. While it certainly seems unusual that packets are reordered on a loopback device, it does appear to happen occasionally (on linux-mips). Fixed by sizing receive buffers such that messages in any order can be received correctly, and by allowing either order for the reassembled message. Combine "Messages" and "Messages-dialed" subtests with a simple table-driven test, to avoid the repetition. The same "Message" and "Message-dialed". Finally, make the test failure messages slightly more useful. Fixes golang/go#49385 Change-Id: I04463c6ffdf4865d2ccfb8662ab4660bda3b3cbf GitHub-Last-Rev: d9df27b GitHub-Pull-Request: #119 Reviewed-on: https://go-review.googlesource.com/c/net/+/368094 Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-by: Damien Neil <[email protected]> Trust: Bryan C. Mills <[email protected]> Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Go Bot <[email protected]>
1 parent d83791d commit a5aa9c7

File tree

1 file changed

+84
-104
lines changed

1 file changed

+84
-104
lines changed

‎internal/socket/socket_test.go

Lines changed: 84 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -181,84 +181,99 @@ func TestUDP(t *testing.T) {
181181
t.Fatal(err)
182182
}
183183

184-
t.Run("Message", func(t *testing.T) {
185-
data := []byte("HELLO-R-U-THERE")
186-
wm := socket.Message{
187-
Buffers: bytes.SplitAfter(data, []byte("-")),
188-
Addr: c.LocalAddr(),
189-
}
190-
if err := cc.SendMsg(&wm, 0); err != nil {
191-
t.Fatal(err)
192-
}
193-
b := make([]byte, 32)
194-
rm := socket.Message{
195-
Buffers: [][]byte{b[:1], b[1:3], b[3:7], b[7:11], b[11:]},
196-
}
197-
if err := cc.RecvMsg(&rm, 0); err != nil {
198-
t.Fatal(err)
199-
}
200-
if !bytes.Equal(b[:rm.N], data) {
201-
t.Fatalf("got %#v; want %#v", b[:rm.N], data)
202-
}
203-
})
204-
t.Run("Message-dialed", func(t *testing.T) {
205-
data := []byte("HELLO-R-U-THERE")
206-
wm := socket.Message{
207-
Buffers: bytes.SplitAfter(data, []byte("-")),
208-
Addr: nil,
209-
}
210-
if err := ccDialed.SendMsg(&wm, 0); err != nil {
211-
t.Fatal(err)
212-
}
213-
b := make([]byte, 32)
214-
rm := socket.Message{
215-
Buffers: [][]byte{b[:1], b[1:3], b[3:7], b[7:11], b[11:]},
216-
}
217-
if err := cc.RecvMsg(&rm, 0); err != nil {
218-
t.Fatal(err)
219-
}
220-
if !bytes.Equal(b[:rm.N], data) {
221-
t.Fatalf("got %#v; want %#v", b[:rm.N], data)
222-
}
223-
})
224-
225-
switch runtime.GOOS {
226-
case "android", "linux":
227-
t.Run("Messages", func(t *testing.T) {
228-
data := []byte("HELLO-R-U-THERE")
229-
wmbs := bytes.SplitAfter(data, []byte("-"))
230-
wms := []socket.Message{
231-
{Buffers: wmbs[:1], Addr: c.LocalAddr()},
232-
{Buffers: wmbs[1:], Addr: c.LocalAddr()},
184+
const data = "HELLO-R-U-THERE"
185+
messageTests := []struct {
186+
name string
187+
conn *socket.Conn
188+
dest net.Addr
189+
}{
190+
{
191+
name: "Message",
192+
conn: cc,
193+
dest: c.LocalAddr(),
194+
},
195+
{
196+
name: "Message-dialed",
197+
conn: ccDialed,
198+
dest: nil,
199+
},
200+
}
201+
for _, tt := range messageTests {
202+
t.Run(tt.name, func(t *testing.T) {
203+
wm := socket.Message{
204+
Buffers: bytes.SplitAfter([]byte(data), []byte("-")),
205+
Addr: tt.dest,
233206
}
234-
n, err := cc.SendMsgs(wms, 0)
235-
if err != nil {
207+
if err := tt.conn.SendMsg(&wm, 0); err != nil {
236208
t.Fatal(err)
237209
}
238-
if n != len(wms) {
239-
t.Fatalf("got %d; want %d", n, len(wms))
240-
}
241210
b := make([]byte, 32)
242-
rmbs := [][][]byte{{b[:len(wmbs[0])]}, {b[len(wmbs[0]):]}}
243-
rms := []socket.Message{
244-
{Buffers: rmbs[0]},
245-
{Buffers: rmbs[1]},
211+
rm := socket.Message{
212+
Buffers: [][]byte{b[:1], b[1:3], b[3:7], b[7:11], b[11:]},
246213
}
247-
n, err = cc.RecvMsgs(rms, 0)
248-
if err != nil {
214+
if err := cc.RecvMsg(&rm, 0); err != nil {
249215
t.Fatal(err)
250216
}
251-
if n != len(rms) {
252-
t.Fatalf("got %d; want %d", n, len(rms))
253-
}
254-
nn := 0
255-
for i := 0; i < n; i++ {
256-
nn += rms[i].N
257-
}
258-
if !bytes.Equal(b[:nn], data) {
259-
t.Fatalf("got %#v; want %#v", b[:nn], data)
217+
received := string(b[:rm.N])
218+
if received != data {
219+
t.Fatalf("Roundtrip SendMsg/RecvMsg got %q; want %q", received, data)
260220
}
261221
})
222+
}
223+
224+
switch runtime.GOOS {
225+
case "android", "linux":
226+
messagesTests := []struct {
227+
name string
228+
conn *socket.Conn
229+
dest net.Addr
230+
}{
231+
{
232+
name: "Messages",
233+
conn: cc,
234+
dest: c.LocalAddr(),
235+
},
236+
{
237+
name: "Messages-dialed",
238+
conn: ccDialed,
239+
dest: nil,
240+
},
241+
}
242+
for _, tt := range messagesTests {
243+
t.Run(tt.name, func(t *testing.T) {
244+
wmbs := bytes.SplitAfter([]byte(data), []byte("-"))
245+
wms := []socket.Message{
246+
{Buffers: wmbs[:1], Addr: tt.dest},
247+
{Buffers: wmbs[1:], Addr: tt.dest},
248+
}
249+
n, err := tt.conn.SendMsgs(wms, 0)
250+
if err != nil {
251+
t.Fatal(err)
252+
}
253+
if n != len(wms) {
254+
t.Fatalf("SendMsgs(%#v) != %d; want %d", wms, n, len(wms))
255+
}
256+
rmbs := [][]byte{make([]byte, 32), make([]byte, 32)}
257+
rms := []socket.Message{
258+
{Buffers: [][]byte{rmbs[0]}},
259+
{Buffers: [][]byte{rmbs[1][:1], rmbs[1][1:3], rmbs[1][3:7], rmbs[1][7:11], rmbs[1][11:]}},
260+
}
261+
nrecv := 0
262+
for nrecv < len(rms) {
263+
n, err := cc.RecvMsgs(rms[nrecv:], 0)
264+
if err != nil {
265+
t.Fatal(err)
266+
}
267+
nrecv += n
268+
}
269+
received0, received1 := string(rmbs[0][:rms[0].N]), string(rmbs[1][:rms[1].N])
270+
assembled := received0 + received1
271+
assembledReordered := received1 + received0
272+
if assembled != data && assembledReordered != data {
273+
t.Fatalf("Roundtrip SendMsgs/RecvMsgs got %q / %q; want %q", assembled, assembledReordered, data)
274+
}
275+
})
276+
}
262277
t.Run("Messages-undialed-no-dst", func(t *testing.T) {
263278
// sending without destination address should fail.
264279
// This checks that the internally recycled buffers are reset correctly.
@@ -273,41 +288,6 @@ func TestUDP(t *testing.T) {
273288
t.Fatal("expected error, destination address required")
274289
}
275290
})
276-
t.Run("Messages-dialed", func(t *testing.T) {
277-
data := []byte("HELLO-R-U-THERE")
278-
wmbs := bytes.SplitAfter(data, []byte("-"))
279-
wms := []socket.Message{
280-
{Buffers: wmbs[:1], Addr: nil},
281-
{Buffers: wmbs[1:], Addr: nil},
282-
}
283-
n, err := ccDialed.SendMsgs(wms, 0)
284-
if err != nil {
285-
t.Fatal(err)
286-
}
287-
if n != len(wms) {
288-
t.Fatalf("got %d; want %d", n, len(wms))
289-
}
290-
b := make([]byte, 32)
291-
rmbs := [][][]byte{{b[:len(wmbs[0])]}, {b[len(wmbs[0]):]}}
292-
rms := []socket.Message{
293-
{Buffers: rmbs[0]},
294-
{Buffers: rmbs[1]},
295-
}
296-
n, err = cc.RecvMsgs(rms, 0)
297-
if err != nil {
298-
t.Fatal(err)
299-
}
300-
if n != len(rms) {
301-
t.Fatalf("got %d; want %d", n, len(rms))
302-
}
303-
nn := 0
304-
for i := 0; i < n; i++ {
305-
nn += rms[i].N
306-
}
307-
if !bytes.Equal(b[:nn], data) {
308-
t.Fatalf("got %#v; want %#v", b[:nn], data)
309-
}
310-
})
311291
}
312292

313293
// The behavior of transmission for zero byte paylaod depends

0 commit comments

Comments
 (0)