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.
Pion ICE
A Go implementation of ICE
Roadmap
The library is used as a part of our WebRTC implementation. Please refer to that roadmap to track our major milestones.
Community
Pion has an active community on the Discord.
Follow the Pion Bluesky or Pion Twitter for project updates and important WebRTC news.
We are always looking to support your projects. Please reach out if you have something to build! If you need commercial support or don't want to use public methods you can contact us at team@pion.ly
Contributing
Check out the contributing wiki to join the group of amazing people making this project possible
License
MIT License - see LICENSE for full text