Skip to content

Add gather generations and tag candidates - #945

Open
maum-meliora wants to merge 4 commits into
pion:mainfrom
maum-meliora:gather-generation-extension
Open

Add gather generations and tag candidates#945
maum-meliora wants to merge 4 commits into
pion:mainfrom
maum-meliora:gather-generation-extension

Conversation

@maum-meliora

@maum-meliora maum-meliora commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Follows up on #874 by @boushley, carrying the same goal forward with a restart-scoped implementation aligned with libwebrtc.

Tag every gathered candidate with a generation extension holding the
candidate's gather generation id — the attribute libwebrtc-based
browsers emit so peers can tell candidates of a restarted gather cycle
apart from the previous cycle's.

Two commits:

  1. Encapsulate ICE generation state in one type. Local credentials
    and gathering state were separate Agent fields that a restart
    updated piecemeal — an in-flight gather cycle could read a
    credential the restart had already rewritten, a data race. The new
    iceGeneration type bundles the generation id, credentials, and
    gathering state; Restart swaps the active generation on the event
    loop, and a gather cycle reads credentials only from the generation
    it captured at start, immutable for the life of the cycle.
  2. Add the generation candidate extension. A superseded cycle
    tags its own generation, not the agent's current generation. Only
    an ICE restart advances the generation — a continual-gathering
    re-gather reuses its cycle's generation, and the initial gather is
    generation 0 — matching libwebrtc.

Behavior notes

  • Marshaled candidates (and downstream SDP a=candidate lines) gain
    one additive generation N token; peers that follow RFC 5245
    extension parsing ignore unknown extensions.
  • Close joins a gather cycle that a restart superseded when no new
    gather followed; restart-then-regather leaves the previous cycle to
    context cancellation, unchanged from current behavior.

Testing

  • Unit tests: a superseded cycle tags its own generation and ufrag; a
    canceled cycle's state write is dropped with no late
    end-of-candidates; Close joins a superseded gather. A vnet test
    asserts generation 0, then generation 1 after a restart.
  • Full suite green under -race; golangci-lint clean. End-to-end
    against pion/webrtc main (local replace): SDP candidate lines carry
    generation 0, then generation 1 after an ICE restart.

Review focus

  • Add gathering generation extension to candidates #874 bumped the generation on continual-gathering network changes;
    this PR advances the generation only on ICE restart, matching
    libwebrtc (a generation is an allocator session, created only on
    credential change — p2p/base/p2p_transport_channel.cc). For the
    flapping scenario discussed in Add gathering generation extension to candidates #874, libwebrtc prunes failed-network
    ports and relies on connectivity checks rather than generation
    filtering; equivalent cleanup is out of scope for this PR.

An ICE generation's local credentials and gathering state were
held as separate Agent fields, so a restart updated them piecemeal.
An in-flight gather cycle could then read a credential the restart
had already rewritten — a data race — or write gathering state the
restart had just reset.

Introduce iceGeneration, which bundles the generation id, local
credentials, and the gathering state.

- Restart replaces the active *iceGeneration on the event loop
  instead of updating fields piecemeal.
- A gather cycle captures the generation it started under and
  reads that generation's credentials, immutable for the life of
  the cycle, so off-loop gather goroutines never touch shared
  credential fields — removing the data race on restart.
- The gather goroutine's cancel/done handles stay on the Agent
  rather than the generation, so Close still joins a cycle that a
  restart superseded before a new gather began.
Tag every gathered candidate with a "generation" extension holding
the candidate's gather generation id — the attribute libwebrtc-based
browsers emit so peers can tell candidates of a restarted gather
cycle apart from the previous cycle's.

- A cycle superseded by a restart still tags its own generation,
  not the agent's current generation.
- Only an ICE restart advances the generation — a
  continual-gathering re-gather reuses its cycle's generation —
  and the initial gather is generation 0, matching libwebrtc.
- The marshaled candidate string gains one additive token; peers
  that follow RFC 5245 extension parsing ignore unknown extensions.

Continues pion#874 by @boushley.
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.35632% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.21%. Comparing base (c649265) to head (2ebf7d3).

Files with missing lines Patch % Lines
gather.go 79.54% 5 Missing and 4 partials ⚠️
agent.go 94.73% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #945      +/-   ##
==========================================
- Coverage   88.41%   88.21%   -0.20%     
==========================================
  Files          46       46              
  Lines        6291     6278      -13     
==========================================
- Hits         5562     5538      -24     
- Misses        497      504       +7     
- Partials      232      236       +4     
Flag Coverage Δ
go 88.21% <87.35%> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant