Skip to content

Commit 3b5d316

Browse files
Zach KZach K
authored andcommitted
Fix packet buffer read index after buffer resize
Fix a bug that caused the packet circular buffer to have an incorrect read index after a buffer resize occurs when the read index is non-zero. When this occured, reading from a connection would erroneously block (packet(s) are in the buffer) and some buffered packets could be lost. Partially fixes #712. This specifically fixes the test failure mode where the test times out due to blocking on reading a packet even though a buffered packet exists.
1 parent 806ff2f commit 3b5d316

File tree

2 files changed

+45
-2
lines changed

2 files changed

+45
-2
lines changed

internal/net/buffer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ func (b *PacketBuffer) WriteTo(pkt []byte, addr net.Addr) (int, error) {
105105

106106
b.packets = newBuf
107107

108-
// Update write pointer to point to new location and mark buffer as not
109-
// full.
108+
// Update read/write pointers and mark buffer as not full.
109+
b.read = 0
110110
b.write = n
111111
b.full = false
112112
}

internal/net/buffer_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,49 @@ func TestShortBuffer(t *testing.T) {
137137
assert.NoError(t, buffer.Close())
138138
}
139139

140+
func TestResizeWraparound(t *testing.T) {
141+
buffer := NewPacketBuffer()
142+
addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:5684")
143+
assert.NoError(t, err)
144+
145+
n, err := buffer.WriteTo([]byte{1}, addr)
146+
assert.NoError(t, err)
147+
equalInt(t, 1, n)
148+
149+
// Trigger resize from 1 -> 2.
150+
n, err = buffer.WriteTo([]byte{2}, addr)
151+
assert.NoError(t, err)
152+
equalInt(t, 1, n)
153+
154+
// Read once to ensure the read index is non-zero when the next resize
155+
// happens.
156+
dst := make([]byte, 1)
157+
n, _, err = buffer.ReadFrom(dst)
158+
assert.NoError(t, err)
159+
equalInt(t, 1, n)
160+
equalBytes(t, []byte{1}, dst[:n])
161+
162+
// Trigger resize from 2 -> 4.
163+
n, err = buffer.WriteTo([]byte{3}, addr)
164+
assert.NoError(t, err)
165+
equalInt(t, 1, n)
166+
167+
// Write another packet after resizing to ensure we retain buffered packets
168+
// before and after the resize.
169+
n, err = buffer.WriteTo([]byte{4}, addr)
170+
assert.NoError(t, err)
171+
equalInt(t, 1, n)
172+
173+
// Read out all buffered packets. They should 1) all be readable from the
174+
// buffer and 2) be read in order.
175+
for i := 2; i < 5; i++ {
176+
n, _, err = buffer.ReadFrom(dst)
177+
assert.NoError(t, err)
178+
equalInt(t, 1, n)
179+
equalBytes(t, []byte{byte(i)}, dst[:n])
180+
}
181+
}
182+
140183
func TestWraparound(t *testing.T) {
141184
buffer := NewPacketBuffer()
142185
addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:5684")

0 commit comments

Comments
 (0)