Ensure when a conneciton is closed, only one connection is removed

peer.go

@@ -400,23 +400,21 @@ func (p *Peer) AddOutboundConnection(c *Connection) error {
 	return nil
 }
 
-// removeClosed will check remove the changed connection if it exists
-// in the specified connections list.
-func (p *Peer) removeClosed(connsPtr *[]*Connection, changed *Connection) (updated bool, found bool) {
+// removeConnection will check remove the connection if it exists on connsPtr
+// and returns whether it removed the connection.
+func (p *Peer) removeConnection(connsPtr *[]*Connection, changed *Connection) bool {
 	conns := *connsPtr
-	newConns := conns[:0]
-	for _, c := range conns {
+	for i, c := range conns {
 		if c == changed {
-			found = true
+			// Remove the connection by moving to the end and slicing the list.
+			last := len(conns) - 1
+			conns[i], conns[last] = conns[last], conns[i]
+			*connsPtr = conns[:last]
+			return true
 		}
-
-		updated = true
-	}
-	if updated {
-		*connsPtr = newConns
 	}
 
-	return updated, found
+	return false
 }
 
 // connectionStateChanged is called when one of the peers' connections states changes.
@@ -426,13 +424,13 @@ func (p *Peer) connectionCloseStateChange(changed *Connection) {
 	}
 
 	p.Lock()
-	updated, found := p.removeClosed(&p.inboundConnections, changed)
+	found := p.removeConnection(&p.inboundConnections, changed)
 	if !found {
-		updated, found = p.removeClosed(&p.outboundConnections, changed)
+		found = p.removeConnection(&p.outboundConnections, changed)
 	}
 	p.Unlock()
 
-	if updated {
+	if found {
 		p.onClosedConnRemoved(p)
 	}
 }

peer_test.go

@@ -136,6 +136,31 @@ func TestGetPeerAvoidPrevSelected(t *testing.T) {
 	}
 }
 
+func TestPeerRemoveClosedConnection(t *testing.T) {
+	ctx, cancel := NewContext(time.Second)
+	defer cancel()
+
+	WithVerifiedServer(t, nil, func(ch *Channel, hostPort string) {
+		client := testutils.NewClient(t, nil)
+		defer client.Close()
+
+		p := client.Peers().Add(hostPort)
+
+		c1, err := p.Connect(ctx)
+		require.NoError(t, err, "Failed to connect")
+		c2, err := p.Connect(ctx)
+		require.NoError(t, err, "Failed to connect")
+
+		require.NoError(t, c1.Close(), "Failed to close first connection")
+		_, outConns := p.NumConnections()
+		assert.Equal(t, 1, outConns, "Expected 1 remaining outgoing connection")
+
+		c, err := p.GetConnection(ctx)
+		require.NoError(t, err, "GetConnection failed")
+		assert.Equal(t, c2, c, "Expected second active connection")
+	})
+}
+
 func TestInboundEphemeralPeerRemoved(t *testing.T) {
 	ctx, cancel := NewContext(time.Second)
 	defer cancel()