mirror of
https://github.com/pion/ice.git
synced 2026-04-22 16:17:11 +08:00
sirzooro
ceccc15191
This PR fixes two race conditions introduced by peer-reflexive candidate replacement and addresses a connectivity regression where an agent could transition from Connected to Disconnected/Failed shortly after replacement. ## Problems 1. Data race in candidate pair access: - `replaceRedundantPeerReflexiveCandidates` updated `pair.Remote` in place. - At the same time, hot-path reads (for example `CandidatePair.Write`) could read `pair.Remote` without synchronization. 2. Data race in remote candidate cache map: - `candidateBase.handleInboundPacket` (non-STUN path) read/wrote `remoteCandidateCaches` from recv loop goroutines. - Concurrent writes also happened from agent-loop code (`replaceRemoteCandidateCacheValues`). 3. Connectivity regression after replacement: - When replacing a selected pair's remote candidate (prflx -> signaled), the new candidate could have zero `LastReceived`/`LastSent`. - `validateSelectedPair` uses `selectedPair.Remote.LastReceived()`, which could cause quick `Connected -> Disconnected -> Failed` transitions. ## Root Cause Replacement logic preserved pair priority but mutated shared objects in place and did not preserve candidate activity timestamps across candidate object replacement. ## Fixes ### 1) Replace candidate pair objects instead of mutating `pair.Remote` - Added helper `replacePairRemote(pair, remote)` to clone pair state into a new `CandidatePair`. - Preserved pair fields and runtime stats: - `id`, role, state, nomination flags, binding counters. - RTT fields, packet/byte counters, request/response counters. - timestamp `atomic.Value` fields. - Replaced references in: - `a.checklist[i]` - `a.pairsByID[pair.id]` - If old pair was selected, published replacement via `a.setSelectedPair(replacement)`. ### 2) Serialize remote cache access on agent loop - Updated non-STUN path in `candidateBase.handleInboundPacket`: - Cache validation, remote candidate lookup, `seen(false)`, and cache insert now run inside `agent.loop.Run(...)`. - This removes concurrent map read/write between recv loop and agent-loop updates. ### 3) Preserve candidate activity on replacement - Added `copyCandidateActivity(dst, src)` to transfer: - `LastReceived` - `LastSent` - Applied before replacing references so selected-pair liveness checks remain stable.
82 lines
2.0 KiB
Go
82 lines
2.0 KiB
Go
// SPDX-FileCopyrightText: 2026 The Pion community <https://pion.ly>
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
package ice
|
|
|
|
import (
|
|
"net/netip"
|
|
"strings"
|
|
)
|
|
|
|
// CandidateHost is a candidate of type host.
|
|
type CandidateHost struct {
|
|
candidateBase
|
|
|
|
network string
|
|
}
|
|
|
|
// CandidateHostConfig is the config required to create a new CandidateHost.
|
|
type CandidateHostConfig struct {
|
|
CandidateID string
|
|
Network string
|
|
Address string
|
|
Port int
|
|
Component uint16
|
|
Priority uint32
|
|
Foundation string
|
|
TCPType TCPType
|
|
IsLocationTracked bool
|
|
}
|
|
|
|
// NewCandidateHost creates a new host candidate.
|
|
func NewCandidateHost(config *CandidateHostConfig) (*CandidateHost, error) {
|
|
candidateID := config.CandidateID
|
|
|
|
if candidateID == "" {
|
|
candidateID = globalCandidateIDGenerator.Generate()
|
|
}
|
|
|
|
candidateHost := &CandidateHost{
|
|
candidateBase: candidateBase{
|
|
id: candidateID,
|
|
address: config.Address,
|
|
candidateType: CandidateTypeHost,
|
|
component: config.Component,
|
|
port: config.Port,
|
|
tcpType: config.TCPType,
|
|
foundationOverride: config.Foundation,
|
|
priorityOverride: config.Priority,
|
|
isLocationTracked: config.IsLocationTracked,
|
|
},
|
|
network: config.Network,
|
|
}
|
|
|
|
if !strings.HasSuffix(config.Address, ".local") && !strings.HasSuffix(config.Address, ".invalid") {
|
|
ipAddr, err := netip.ParseAddr(config.Address)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
if err := candidateHost.setIPAddr(ipAddr); err != nil {
|
|
return nil, err
|
|
}
|
|
} else {
|
|
// Until mDNS candidate is resolved assume it is UDPv4
|
|
candidateHost.candidateBase.networkType = NetworkTypeUDP4
|
|
}
|
|
|
|
return candidateHost, nil
|
|
}
|
|
|
|
func (c *CandidateHost) setIPAddr(addr netip.Addr) error {
|
|
networkType, err := determineNetworkType(c.network, addr)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
c.candidateBase.networkType = networkType
|
|
c.candidateBase.resolvedAddr = createAddr(networkType, addr, c.port)
|
|
|
|
return nil
|
|
}
|