Skip to content

Commit

Permalink
internal/socket: handle reordering in TestUDP/Messages
Browse files Browse the repository at this point in the history 
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#49382
  • Loading branch information
@matzf
matzf committed Dec 1, 2021
1 parent d83791d commit a30eeaf
Showing 1 changed file with 84 additions and 104 deletions.
188 changes: 84 additions & 104 deletions internal/socket/socket_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,84 +181,99 @@ func TestUDP(t *testing.T) {
t.Fatal(err)
}

t.Run("Message", func(t *testing.T) {
data := []byte("HELLO-R-U-THERE")
wm := socket.Message{
Buffers: bytes.SplitAfter(data, []byte("-")),
Addr: c.LocalAddr(),
}
if err := cc.SendMsg(&wm, 0); err != nil {
t.Fatal(err)
}
b := make([]byte, 32)
rm := socket.Message{
Buffers: [][]byte{b[:1], b[1:3], b[3:7], b[7:11], b[11:]},
}
if err := cc.RecvMsg(&rm, 0); err != nil {
t.Fatal(err)
}
if !bytes.Equal(b[:rm.N], data) {
t.Fatalf("got %#v; want %#v", b[:rm.N], data)
}
})
t.Run("Message-dialed", func(t *testing.T) {
data := []byte("HELLO-R-U-THERE")
wm := socket.Message{
Buffers: bytes.SplitAfter(data, []byte("-")),
Addr: nil,
}
if err := ccDialed.SendMsg(&wm, 0); err != nil {
t.Fatal(err)
}
b := make([]byte, 32)
rm := socket.Message{
Buffers: [][]byte{b[:1], b[1:3], b[3:7], b[7:11], b[11:]},
}
if err := cc.RecvMsg(&rm, 0); err != nil {
t.Fatal(err)
}
if !bytes.Equal(b[:rm.N], data) {
t.Fatalf("got %#v; want %#v", b[:rm.N], data)
}
})

switch runtime.GOOS {
case "android", "linux":
t.Run("Messages", func(t *testing.T) {
data := []byte("HELLO-R-U-THERE")
wmbs := bytes.SplitAfter(data, []byte("-"))
wms := []socket.Message{
{Buffers: wmbs[:1], Addr: c.LocalAddr()},
{Buffers: wmbs[1:], Addr: c.LocalAddr()},
const data = "HELLO-R-U-THERE"
messageTests := []struct {
name string
conn *socket.Conn
dest net.Addr
}{
{
name: "Message",
conn: cc,
dest: c.LocalAddr(),
},
{
name: "Message-dialed",
conn: ccDialed,
dest: nil,
},
}
for _, tt := range messageTests {
t.Run(tt.name, func(t *testing.T) {
wm := socket.Message{
Buffers: bytes.SplitAfter([]byte(data), []byte("-")),
Addr: tt.dest,
}
n, err := cc.SendMsgs(wms, 0)
if err != nil {
if err := tt.conn.SendMsg(&wm, 0); err != nil {
t.Fatal(err)
}
if n != len(wms) {
t.Fatalf("got %d; want %d", n, len(wms))
}
b := make([]byte, 32)
rmbs := [][][]byte{{b[:len(wmbs[0])]}, {b[len(wmbs[0]):]}}
rms := []socket.Message{
{Buffers: rmbs[0]},
{Buffers: rmbs[1]},
rm := socket.Message{
Buffers: [][]byte{b[:1], b[1:3], b[3:7], b[7:11], b[11:]},
}
n, err = cc.RecvMsgs(rms, 0)
if err != nil {
if err := cc.RecvMsg(&rm, 0); err != nil {
t.Fatal(err)
}
if n != len(rms) {
t.Fatalf("got %d; want %d", n, len(rms))
}
nn := 0
for i := 0; i < n; i++ {
nn += rms[i].N
}
if !bytes.Equal(b[:nn], data) {
t.Fatalf("got %#v; want %#v", b[:nn], data)
received := string(b[:rm.N])
if received != data {
t.Fatalf("Roundtrip SendMsg/RecvMsg got %q; want %q", received, data)
}
})
}

switch runtime.GOOS {
case "android", "linux":
messagesTests := []struct {
name string
conn *socket.Conn
dest net.Addr
}{
{
name: "Messages",
conn: cc,
dest: c.LocalAddr(),
},
{
name: "Messages-dialed",
conn: ccDialed,
dest: nil,
},
}
for _, tt := range messagesTests {
t.Run(tt.name, func(t *testing.T) {
wmbs := bytes.SplitAfter([]byte(data), []byte("-"))
wms := []socket.Message{
{Buffers: wmbs[:1], Addr: tt.dest},
{Buffers: wmbs[1:], Addr: tt.dest},
}
n, err := tt.conn.SendMsgs(wms, 0)
if err != nil {
t.Fatal(err)
}
if n != len(wms) {
t.Fatalf("SendMsgs(%#v) != %d; want %d", wms, n, len(wms))
}
rmbs := [][]byte{make([]byte, 32), make([]byte, 32)}
rms := []socket.Message{
{Buffers: [][]byte{rmbs[0]}},
{Buffers: [][]byte{rmbs[1][:1], rmbs[1][1:3], rmbs[1][3:7], rmbs[1][7:11], rmbs[1][11:]}},
}
nrecv := 0
for nrecv < len(rms) {
n, err := cc.RecvMsgs(rms[nrecv:], 0)
if err != nil {
t.Fatal(err)
}
nrecv += n
}
received0, received1 := string(rmbs[0][:rms[0].N]), string(rmbs[1][:rms[1].N])
assembled := received0 + received1
assembledReordered := received1 + received0
if assembled != data && assembledReordered != data {
t.Fatalf("Roundtrip SendMsgs/RecvMsgs got %q / %q; want %q", assembled, assembledReordered, data)
}
})
}
t.Run("Messages-undialed-no-dst", func(t *testing.T) {
// sending without destination address should fail.
// This checks that the internally recycled buffers are reset correctly.
Expand All @@ -273,41 +288,6 @@ func TestUDP(t *testing.T) {
t.Fatal("expected error, destination address required")
}
})
t.Run("Messages-dialed", func(t *testing.T) {
data := []byte("HELLO-R-U-THERE")
wmbs := bytes.SplitAfter(data, []byte("-"))
wms := []socket.Message{
{Buffers: wmbs[:1], Addr: nil},
{Buffers: wmbs[1:], Addr: nil},
}
n, err := ccDialed.SendMsgs(wms, 0)
if err != nil {
t.Fatal(err)
}
if n != len(wms) {
t.Fatalf("got %d; want %d", n, len(wms))
}
b := make([]byte, 32)
rmbs := [][][]byte{{b[:len(wmbs[0])]}, {b[len(wmbs[0]):]}}
rms := []socket.Message{
{Buffers: rmbs[0]},
{Buffers: rmbs[1]},
}
n, err = cc.RecvMsgs(rms, 0)
if err != nil {
t.Fatal(err)
}
if n != len(rms) {
t.Fatalf("got %d; want %d", n, len(rms))
}
nn := 0
for i := 0; i < n; i++ {
nn += rms[i].N
}
if !bytes.Equal(b[:nn], data) {
t.Fatalf("got %#v; want %#v", b[:nn], data)
}
})
}

// The behavior of transmission for zero byte paylaod depends
Expand Down

0 comments on commit a30eeaf

Please sign in to comment.