[Connection] Replace local index with a copy of the candidate.
This is to avoid using an index into a vector that's owned by Port.
Bug: none
Change-Id: Ifc67fcc24bcb04e55c7b963de6d29bb9541c1495
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/263643
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37015}
diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc
index b0097b5..876deb1 100644
--- a/p2p/base/connection.cc
+++ b/p2p/base/connection.cc
@@ -291,7 +291,7 @@
: network_thread_(port->thread()),
id_(rtc::CreateRandomId()),
port_(std::move(port)),
- local_candidate_index_(index),
+ local_candidate_(port_->Candidates()[index]),
remote_candidate_(remote_candidate),
recv_rate_tracker_(100, 10u),
send_rate_tracker_(100, 10u),
@@ -328,8 +328,7 @@
const Candidate& Connection::local_candidate() const {
RTC_DCHECK_RUN_ON(network_thread_);
- RTC_DCHECK(local_candidate_index_ < port_->Candidates().size());
- return port_->Candidates()[local_candidate_index_];
+ return local_candidate_;
}
const Candidate& Connection::remote_candidate() const {
@@ -345,6 +344,9 @@
}
uint64_t Connection::priority() const {
+ if (!port_)
+ return 0;
+
uint64_t priority = 0;
// RFC 5245 - 5.7.2. Computing Pair Priority and Ordering Pairs
// Let G be the priority for the candidate provided by the controlling
@@ -968,6 +970,15 @@
}
}
+void Connection::UpdateLocalIceParameters(int component,
+ absl::string_view username_fragment,
+ absl::string_view password) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ local_candidate_.set_component(component);
+ local_candidate_.set_username(username_fragment);
+ local_candidate_.set_password(password);
+}
+
int64_t Connection::last_ping_sent() const {
RTC_DCHECK_RUN_ON(network_thread_);
return last_ping_sent_;
@@ -1216,23 +1227,23 @@
ss << "Conn[" << ToDebugId();
if (!port_) {
- // No content name for pending delete, so temporarily substitute the name
- // with a hash (rhyming with trash) and don't include any information about
- // the network or candidates, state that belongs to a potentially deleted
- // `port_`.
- ss << ":#:";
+ // No content or network names for pending delete. Temporarily substitute
+ // the names with a hash (rhyming with trash).
+ ss << ":#:#:";
} else {
- const Candidate& local = local_candidate();
- const Candidate& remote = remote_candidate();
ss << ":" << port_->content_name() << ":" << port_->Network()->ToString()
- << ":" << local.id() << ":" << local.component() << ":"
- << local.generation() << ":" << local.type() << ":" << local.protocol()
- << ":" << local.address().ToSensitiveString() << "->" << remote.id()
- << ":" << remote.component() << ":" << remote.priority() << ":"
- << remote.type() << ":" << remote.protocol() << ":"
- << remote.address().ToSensitiveString() << "|";
+ << ":";
}
+ const Candidate& local = local_candidate();
+ const Candidate& remote = remote_candidate();
+ ss << local.id() << ":" << local.component() << ":" << local.generation()
+ << ":" << local.type() << ":" << local.protocol() << ":"
+ << local.address().ToSensitiveString() << "->" << remote.id() << ":"
+ << remote.component() << ":" << remote.priority() << ":" << remote.type()
+ << ":" << remote.protocol() << ":" << remote.address().ToSensitiveString()
+ << "|";
+
ss << CONNECT_STATE_ABBREV[connected_] << RECEIVE_STATE_ABBREV[receiving_]
<< WRITE_STATE_ABBREV[write_state_] << ICESTATE[static_cast<int>(state_)]
<< "|" << SELECTED_STATE_ABBREV[selected_] << "|" << remote_nomination_
@@ -1504,12 +1515,12 @@
return;
}
- for (size_t i = 0; i < port_->Candidates().size(); ++i) {
- if (port_->Candidates()[i].address() == addr->GetAddress()) {
- if (local_candidate_index_ != i) {
+ for (const Candidate& candidate : port_->Candidates()) {
+ if (candidate.address() == addr->GetAddress()) {
+ if (local_candidate_ != candidate) {
RTC_LOG(LS_INFO) << ToString()
<< ": Updating local candidate type to srflx.";
- local_candidate_index_ = i;
+ local_candidate_ = candidate;
// SignalStateChange to force a re-sort in P2PTransportChannel as this
// Connection's local candidate has changed.
SignalStateChange(this);
@@ -1533,19 +1544,20 @@
std::string id = rtc::CreateRandomString(8);
// Create a peer-reflexive candidate based on the local candidate.
- Candidate new_local_candidate(local_candidate());
- new_local_candidate.set_id(id);
- new_local_candidate.set_type(PRFLX_PORT_TYPE);
- new_local_candidate.set_address(addr->GetAddress());
- new_local_candidate.set_priority(priority);
- new_local_candidate.set_related_address(local_candidate().address());
- new_local_candidate.set_foundation(Port::ComputeFoundation(
- PRFLX_PORT_TYPE, local_candidate().protocol(),
- local_candidate().relay_protocol(), local_candidate().address()));
+ local_candidate_.set_id(id);
+ local_candidate_.set_type(PRFLX_PORT_TYPE);
+ // Set the related address and foundation attributes before changing the
+ // address.
+ local_candidate_.set_related_address(local_candidate_.address());
+ local_candidate_.set_foundation(Port::ComputeFoundation(
+ PRFLX_PORT_TYPE, local_candidate_.protocol(),
+ local_candidate_.relay_protocol(), local_candidate_.address()));
+ local_candidate_.set_priority(priority);
+ local_candidate_.set_address(addr->GetAddress());
// Change the local candidate of this Connection to the new prflx candidate.
RTC_LOG(LS_INFO) << ToString() << ": Updating local candidate type to prflx.";
- local_candidate_index_ = port_->AddPrflxCandidate(new_local_candidate);
+ port_->AddPrflxCandidate(local_candidate_);
// SignalStateChange to force a re-sort in P2PTransportChannel as this
// Connection's local candidate has changed.
@@ -1580,6 +1592,20 @@
return true;
}
+void Connection::SetLocalCandidateNetworkCost(uint16_t cost) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+
+ if (cost == local_candidate_.network_cost())
+ return;
+
+ local_candidate_.set_network_cost(cost);
+
+ // Network cost change will affect the connection selection criteria.
+ // Signal the connection state change to force a re-sort in
+ // P2PTransportChannel.
+ SignalStateChange(this);
+}
+
// RTC_RUN_ON(network_thread_).
bool Connection::ShouldSendGoogPing(const StunMessage* message) {
if (remote_support_goog_ping_ == true && cached_stun_binding_ &&
diff --git a/p2p/base/connection.h b/p2p/base/connection.h
index 3e8cffb..f0850ab 100644
--- a/p2p/base/connection.h
+++ b/p2p/base/connection.h
@@ -15,6 +15,7 @@
#include <string>
#include <vector>
+#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "api/candidate.h"
#include "api/transport/stun.h"
@@ -202,6 +203,10 @@
// the current time, which is compared against various timeouts.
void UpdateState(int64_t now);
+ void UpdateLocalIceParameters(int component,
+ absl::string_view username_fragment,
+ absl::string_view password);
+
// Called when this connection should try checking writability again.
int64_t last_ping_sent() const;
void Ping(int64_t now);
@@ -284,6 +289,9 @@
// Check if we sent `val` pings without receving a response.
bool TooManyOutstandingPings(const absl::optional<int>& val) const;
+ // Called by Port when the network cost changes.
+ void SetLocalCandidateNetworkCost(uint16_t cost);
+
void SetIceFieldTrials(const IceFieldTrials* field_trials);
const rtc::EventBasedExponentialMovingAverage& GetRttEstimate() const {
return rtt_estimate_;
@@ -357,7 +365,7 @@
webrtc::TaskQueueBase* const network_thread_;
const uint32_t id_;
rtc::WeakPtr<Port> port_;
- size_t local_candidate_index_ RTC_GUARDED_BY(network_thread_);
+ Candidate local_candidate_ RTC_GUARDED_BY(network_thread_);
Candidate remote_candidate_;
ConnectionInfo stats_;
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 59c344b..0d76614 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -239,16 +239,23 @@
}
void Port::SetIceParameters(int component,
- const std::string& username_fragment,
- const std::string& password) {
+ absl::string_view username_fragment,
+ absl::string_view password) {
+ RTC_DCHECK_RUN_ON(thread_);
component_ = component;
- ice_username_fragment_ = username_fragment;
- password_ = password;
+ ice_username_fragment_ = std::string(username_fragment);
+ password_ = std::string(password);
for (Candidate& c : candidates_) {
c.set_component(component);
c.set_username(username_fragment);
c.set_password(password);
}
+
+ // In case any connections exist make sure we update them too.
+ for (auto& [unused, connection] : connections_) {
+ connection->UpdateLocalIceParameters(component, username_fragment,
+ password);
+ }
}
const std::vector<Candidate>& Port::Candidates() const {
@@ -274,6 +281,7 @@
uint32_t relay_preference,
const std::string& url,
bool is_final) {
+ RTC_DCHECK_RUN_ON(thread_);
if (protocol == TCP_PROTOCOL_NAME && type == LOCAL_PORT_TYPE) {
RTC_DCHECK(!tcptype.empty());
}
@@ -325,6 +333,7 @@
copy.set_address(hostname_address);
copy.set_related_address(rtc::SocketAddress());
if (weak_ptr != nullptr) {
+ RTC_DCHECK_RUN_ON(weak_ptr->thread_);
weak_ptr->set_mdns_name_registration_status(
MdnsNameRegistrationStatus::kCompleted);
weak_ptr->FinishAddingAddress(copy, is_final);
@@ -429,9 +438,9 @@
}
}
-size_t Port::AddPrflxCandidate(const Candidate& local) {
+void Port::AddPrflxCandidate(const Candidate& local) {
+ RTC_DCHECK_RUN_ON(thread_);
candidates_.push_back(local);
- return (candidates_.size() - 1);
}
bool Port::GetStunMessage(const char* data,
@@ -891,6 +900,7 @@
// TODO(honghaiz): Make the network cost configurable from user setting.
void Port::UpdateNetworkCost() {
+ RTC_DCHECK_RUN_ON(thread_);
uint16_t new_cost = network_->GetCost(*field_trials_);
if (network_cost_ == new_cost) {
return;
@@ -901,16 +911,11 @@
<< ". Number of connections created: "
<< connections_.size();
network_cost_ = new_cost;
- for (cricket::Candidate& candidate : candidates_) {
+ for (cricket::Candidate& candidate : candidates_)
candidate.set_network_cost(network_cost_);
- }
- // Network cost change will affect the connection selection criteria.
- // Signal the connection state change on each connection to force a
- // re-sort in P2PTransportChannel.
- for (const auto& kv : connections_) {
- Connection* conn = kv.second;
- conn->SignalStateChange(conn);
- }
+
+ for (auto& [unused, connection] : connections_)
+ connection->SetLocalCandidateNetworkCost(network_cost_);
}
void Port::EnablePortPackets() {
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 65560fe..dd7b3e3 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -265,8 +265,8 @@
// PortAllocatorSession, and is now being assigned to an ICE transport.
// Updates the information for candidates as well.
void SetIceParameters(int component,
- const std::string& username_fragment,
- const std::string& password);
+ absl::string_view username_fragment,
+ absl::string_view password);
// Fired when candidates are discovered by the port. When all candidates
// are discovered that belong to port SignalAddressReady is fired.
@@ -367,8 +367,7 @@
void OnReadyToSend();
// Called when the Connection discovers a local peer reflexive candidate.
- // Returns the index of the new local candidate.
- size_t AddPrflxCandidate(const Candidate& local);
+ void AddPrflxCandidate(const Candidate& local);
int16_t network_cost() const { return network_cost_; }
@@ -406,7 +405,8 @@
const std::string& url,
bool is_final);
- void FinishAddingAddress(const Candidate& c, bool is_final);
+ void FinishAddingAddress(const Candidate& c, bool is_final)
+ RTC_RUN_ON(thread_);
virtual void PostAddAddress(bool is_final);
@@ -481,7 +481,7 @@
// username_fragment().
std::string ice_username_fragment_;
std::string password_;
- std::vector<Candidate> candidates_;
+ std::vector<Candidate> candidates_ RTC_GUARDED_BY(thread_);
AddressMap connections_;
int timeout_delay_;
bool enable_port_packets_;
@@ -508,7 +508,7 @@
bool MaybeObfuscateAddress(Candidate* c,
const std::string& type,
- bool is_final);
+ bool is_final) RTC_RUN_ON(thread_);
friend class Connection;
webrtc::CallbackList<PortInterface*> port_destroyed_callback_list_;
