Add thread safety annotations for some more PeerConnection members
Plus all the annotations that were necessary to make things compile
again.
port_allocator_flags_ was accessed on both the signaling and the
network thread, but I was able to replace it with a return value.
Bug: webrtc:9987
Change-Id: Iab977a49d6588ce2240487475ec3588ae579caa1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/128772
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27254}diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 15bea89..3faacc9 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -907,8 +907,10 @@
transport_controller_.reset();
// port_allocator_ lives on the network thread and should be destroyed there.
- network_thread()->Invoke<void>(RTC_FROM_HERE,
- [this] { port_allocator_.reset(); });
+ network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
+ RTC_DCHECK_RUN_ON(network_thread());
+ port_allocator_.reset();
+ });
// call_ and event_log_ must be destroyed on the worker thread.
worker_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
RTC_DCHECK_RUN_ON(worker_thread());
@@ -977,12 +979,12 @@
// The port allocator lives on the network thread and should be initialized
// there.
- if (!network_thread()->Invoke<bool>(
+ const auto pa_result =
+ network_thread()->Invoke<InitializePortAllocatorResult>(
RTC_FROM_HERE,
rtc::Bind(&PeerConnection::InitializePortAllocator_n, this,
- stun_servers, turn_servers, configuration))) {
- return false;
- }
+ stun_servers, turn_servers, configuration));
+
// If initialization was successful, note if STUN or TURN servers
// were supplied.
if (!stun_servers.empty()) {
@@ -994,7 +996,7 @@
// Send information about IPv4/IPv6 status.
PeerConnectionAddressFamilyCounter address_family;
- if (port_allocator_flags_ & cricket::PORTALLOCATOR_ENABLE_IPV6) {
+ if (pa_result.enable_ipv6) {
address_family = kPeerConnection_IPv6;
} else {
address_family = kPeerConnection_IPv4;
@@ -5290,48 +5292,51 @@
return nullptr;
}
-bool PeerConnection::InitializePortAllocator_n(
+PeerConnection::InitializePortAllocatorResult
+PeerConnection::InitializePortAllocator_n(
const cricket::ServerAddresses& stun_servers,
const std::vector<cricket::RelayServerConfig>& turn_servers,
const RTCConfiguration& configuration) {
+ RTC_DCHECK_RUN_ON(network_thread());
+
port_allocator_->Initialize();
// To handle both internal and externally created port allocator, we will
// enable BUNDLE here.
- port_allocator_flags_ = port_allocator_->flags();
- port_allocator_flags_ |= cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
- cricket::PORTALLOCATOR_ENABLE_IPV6 |
- cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI;
+ int port_allocator_flags = port_allocator_->flags();
+ port_allocator_flags |= cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+ cricket::PORTALLOCATOR_ENABLE_IPV6 |
+ cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI;
// If the disable-IPv6 flag was specified, we'll not override it
// by experiment.
if (configuration.disable_ipv6) {
- port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
+ port_allocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
} else if (webrtc::field_trial::FindFullName("WebRTC-IPv6Default")
.find("Disabled") == 0) {
- port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
+ port_allocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
}
if (configuration.disable_ipv6_on_wifi) {
- port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI);
+ port_allocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI);
RTC_LOG(LS_INFO) << "IPv6 candidates on Wi-Fi are disabled.";
}
if (configuration.tcp_candidate_policy == kTcpCandidatePolicyDisabled) {
- port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_TCP;
+ port_allocator_flags |= cricket::PORTALLOCATOR_DISABLE_TCP;
RTC_LOG(LS_INFO) << "TCP candidates are disabled.";
}
if (configuration.candidate_network_policy ==
kCandidateNetworkPolicyLowCost) {
- port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS;
+ port_allocator_flags |= cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS;
RTC_LOG(LS_INFO) << "Do not gather candidates on high-cost networks";
}
if (configuration.disable_link_local_networks) {
- port_allocator_flags_ |= cricket::PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS;
+ port_allocator_flags |= cricket::PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS;
RTC_LOG(LS_INFO) << "Disable candidates on link-local network interfaces.";
}
- port_allocator_->set_flags(port_allocator_flags_);
+ port_allocator_->set_flags(port_allocator_flags);
// No step delay is used while allocating ports.
port_allocator_->set_step_delay(cricket::kMinimumStepDelay);
port_allocator_->set_candidate_filter(
@@ -5349,7 +5354,10 @@
configuration.ice_candidate_pool_size, configuration.prune_turn_ports,
configuration.turn_customizer,
configuration.stun_candidate_keepalive_interval);
- return true;
+
+ InitializePortAllocatorResult res;
+ res.enable_ipv6 = port_allocator_flags & cricket::PORTALLOCATOR_ENABLE_IPV6;
+ return res;
}
bool PeerConnection::ReconfigurePortAllocator_n(
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 20b66ac..c0d3e47 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -749,7 +749,10 @@
DataChannel* FindDataChannelBySid(int sid) const;
// Called when first configuring the port allocator.
- bool InitializePortAllocator_n(
+ struct InitializePortAllocatorResult {
+ bool enable_ipv6;
+ };
+ InitializePortAllocatorResult InitializePortAllocator_n(
const cricket::ServerAddresses& stun_servers,
const std::vector<cricket::RelayServerConfig>& turn_servers,
const RTCConfiguration& configuration);
@@ -1095,14 +1098,18 @@
// TODO(zstein): |async_resolver_factory_| can currently be nullptr if it
// is not injected. It should be required once chromium supplies it.
- std::unique_ptr<AsyncResolverFactory> async_resolver_factory_;
- std::unique_ptr<cricket::PortAllocator> port_allocator_;
- std::unique_ptr<rtc::SSLCertificateVerifier> tls_cert_verifier_;
- int port_allocator_flags_ = 0;
+ std::unique_ptr<AsyncResolverFactory> async_resolver_factory_
+ RTC_GUARDED_BY(signaling_thread());
+ std::unique_ptr<cricket::PortAllocator>
+ port_allocator_; // TODO(bugs.webrtc.org/9987): Accessed on both
+ // signaling and network thread.
+ std::unique_ptr<rtc::SSLCertificateVerifier>
+ tls_cert_verifier_; // TODO(bugs.webrtc.org/9987): Accessed on both
+ // signaling and network thread.
// One PeerConnection has only one RTCP CNAME.
// https://tools.ietf.org/html/draft-ietf-rtcweb-rtp-usage-26#section-4.9
- std::string rtcp_cname_;
+ const std::string rtcp_cname_;
// Streams added via AddStream.
rtc::scoped_refptr<StreamCollection> local_streams_;
