Skip to content

Add nice_agent_clear_stream_local_addresses#18

Draft
Copilot wants to merge 1 commit into
masterfrom
copilot/add-clear-stream-local-addresses-function
Draft

Add nice_agent_clear_stream_local_addresses#18
Copilot wants to merge 1 commit into
masterfrom
copilot/add-clear-stream-local-addresses-function

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 14, 2026

Exposes a way to drop the configured local addresses on a stream so callers can replace them, complementing the existing nice_agent_add_stream_local_address.

  • agent/agent.h: declare nice_agent_clear_stream_local_addresses(NiceAgent *, guint stream_id).
  • agent/agent.c: implementation takes the agent lock, looks up the stream, frees each NiceAddress in stream->local_addresses, frees the list, and nulls it out — mirroring the cleanup already performed in stream_free(). Returns FALSE for an invalid stream_id.
  • nice/libnice.sym: export the new symbol.
nice_agent_add_stream_local_address (agent, stream_id, &addr_old);
/* ...later, replace the set... */
nice_agent_clear_stream_local_addresses (agent, stream_id);
nice_agent_add_stream_local_address (agent, stream_id, &addr_new);

No tests added: the existing suite has no coverage of the per-stream local-address API (only the agent-wide nice_agent_add_local_address is used in tests/), so integration coverage is expected to live in the consuming layer.

Agent-Logs-Url: https://github.com/pexip/libnice/sessions/285af649-49e4-46aa-b4da-604a0fdfd6bf

Co-authored-by: bitflows <19825042+bitflows@users.noreply.github.com>
@bitflows
Copy link
Copy Markdown
Contributor

@copilot does libnice allow adding additional local_addresses while its running, and will a new entry be picked up immediately, and new clients gathered?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 15, 2026

@copilot does libnice allow adding additional local_addresses while its running, and will a new entry be picked up immediately, and new clients gathered?

You can add them at any time, but they are not picked up automatically — they only take effect on the next nice_agent_gather_candidates() call.

Looking at agent/agent.c:

  • nice_agent_add_local_address() / nice_agent_add_stream_local_address() simply take the agent lock and append the NiceAddress to agent->local_addresses or stream->local_addresses. No signal is emitted, no socket is created, and no host candidate is generated at that point.
  • The list is only consulted inside nice_agent_gather_candidates() (around line 1551), which snapshots stream->local_addresses (falling back to agent->local_addresses, then to nice_interfaces_get_local_ips()) once at the top, and creates UDP/TCP host candidates from that snapshot.

So in practice:

  1. Adding an address mid-run is safe (it's under the agent lock).
  2. No new host candidates are produced and nothing is signalled to the peer until you re-gather.
  3. To pick up the new address you need to call nice_agent_gather_candidates(agent, stream_id) again (or restart the stream). Note the TODO at ~line 1581 — stale candidates from removed interfaces aren't pruned, so re-gathering only adds candidates on existing streams; an ICE restart is cleaner if you need a fresh set.
  4. An address added after gathering has started but before it finishes won't be included in that run either, because of that one-shot snapshot.

The typical pattern with the new nice_agent_clear_stream_local_addresses from this PR is:

nice_agent_clear_stream_local_addresses(agent, stream_id);
nice_agent_add_stream_local_address(agent, stream_id, &new_addr);
nice_agent_gather_candidates(agent, stream_id);

If you want "added → immediately gathered" without an explicit re-gather call, that behaviour doesn't exist in libnice today and would need a new code path.

@bitflows
Copy link
Copy Markdown
Contributor

@copilot In pulse_network_monitor.c, I've spotted a bug. We get events from glib_network_monitor, and act on them. But, we seems to get an IPv4 address first, then if there is a IPv6 dhcp, it will take a few rounds before it finally has the "final" IPv6 address.
Based on these events, we do different things to the system, like send an SDP update or reset curl. Ideally, we do not want to force such things until we have the "final" ipv4 and ipv6 addresses ready, so we don't fire multiple times. The logic is in pulse_network_monitor_handle_connectivity_loss_unlocked().
Can you add some extra logic there, so we register the address change, but always wait at least 1000ms (configurable value), to wait for further changes (resets on every new address change), before calling any of the resetters (curl, event subscribers and sdp update).

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 15, 2026

@copilot In pulse_network_monitor.c, I've spotted a bug. We get events from glib_network_monitor, and act on them. But, we seems to get an IPv4 address first, then if there is a IPv6 d...

That file isn't part of libnice — pulse_network_monitor.c / pulse_network_monitor_handle_connectivity_loss_unlocked() don't exist in this repository (I searched the tree and there are no matches). It sounds like it lives in a downstream consumer of libnice, so the change needs to be made there, not in this PR.

That said, here's the shape of what you're describing in case it's useful when you raise it in the right repo:

  • Add a guint debounce_timeout_ms field (default 1000) and a guint pending_source_id to the monitor struct, configurable via a setter or property.
  • In pulse_network_monitor_handle_connectivity_loss_unlocked(), instead of invoking the resetters (curl reset, event subscribers, SDP update) directly:
    1. Record/merge the new address state into a pending struct.
    2. If pending_source_id != 0, g_source_remove() it (resets the timer on every new event).
    3. pending_source_id = g_timeout_add(debounce_timeout_ms, on_debounce_elapsed, self);
  • In on_debounce_elapsed(): take the same lock, clear pending_source_id, then call the resetters once with the latest snapshot. Return G_SOURCE_REMOVE.
  • On monitor dispose: if pending_source_id != 0, g_source_remove() it to avoid firing after free.

If you can point me at the correct repo/PR I'm happy to do the implementation there.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants