Skip to content

Commit

Permalink
fixed panic bug on chace eviction. added scenario to test. (#2125)
Browse files Browse the repository at this point in the history
## Motivation
When udp message from a new peer would arrive and the udp session state cache was full we try to evict one session. if one of the sessions in the state is older than 24hours we remove it first. this code path was never tested and had called `Close` on the connection after removing it from the map.

Closes #
<!-- `Closes #XXXX, closes #XXXX, ...` links mentioned issues to this PR and automatically closes them when this it's merged -->

## Changes

added test and fixed.

## Test Plan
<!-- Please specify how these changes were tested 
(e.g. unit tests, manual testing, etc.) -->

## TODO
<!-- This section should be removed when all items are complete -->
- [x] Explain motivation or link existing issue(s)
- [x] Test changes and document test plan
- [x] Update documentation as needed
  • Loading branch information
y0sher authored and noamnelke committed Aug 25, 2020
1 parent 4ae9c69 commit 0cbb228
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
2 changes: 1 addition & 1 deletion p2p/net/udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,8 @@ func (n *UDPNet) addConn(addr net.Addr, ucw udpConn) {
lastk = k
if time.Since(c.Created()) > maxUDPLife {
delete(n.incomingConn, k)
c.Close()
evicted = true
n.incomingConn[k].Close()
break
}

Expand Down
18 changes: 16 additions & 2 deletions p2p/net/udp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func TestUDPNet_Cache(t *testing.T) {
}})
require.Len(t, n.incomingConn, 1)

for i := 1; i < maxUDPConn; i++ {
for i := 1; i < maxUDPConn-1; i++ {
addrx := testUDPAddr()
_, ok := n.incomingConn[addrx.String()]
for ok {
Expand All @@ -234,17 +234,31 @@ func TestUDPNet_Cache(t *testing.T) {
}})
}

require.Len(t, n.incomingConn, maxUDPConn)
require.Len(t, n.incomingConn, maxUDPConn-1)

addrx := testUDPAddr()
_, ok := n.incomingConn[addrx.String()]
for ok {
addrx = testUDPAddr()
_, ok = n.incomingConn[addrx.String()]
}

n.addConn(addrx, &udpConnMock{CreatedFunc: func() time.Time {
return time.Now().Add(-maxUDPLife - 1*time.Second)
}})

require.Len(t, n.incomingConn, maxUDPConn)

addrx2 := testUDPAddr()
_, ok2 := n.incomingConn[addrx2.String()]
for ok2 {
addrx2 = testUDPAddr()
_, ok = n.incomingConn[addrx2.String()]
}
n.addConn(addrx2, &udpConnMock{CreatedFunc: func() time.Time {
return time.Now()
}})

require.Len(t, n.incomingConn, maxUDPConn)

i := 0
Expand Down

0 comments on commit 0cbb228

Please sign in to comment.