zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Add thread checker to PortAllocator and its subclasses and fix a bug"

Browse Source 
This reverts commit fc43d11717e16dd427ac84fee614e5511e43cefd.

Reason for revert: Crashes downstream tests

Original change's description:
> Add thread checker to PortAllocator and its subclasses and fix a bug
> causing memory contention by threads.
> 
> PortAllocator and its subclasses assume all of their methods except the
> constructor must be called on the same thread (the network thread in
> practice). This CL adds a thread checker to PortAllocator and its
> subclasses for thread safety, and fixes bugs of invoking some of their
> methods in PeerConnection on the signaling thread.
> 
> Bug: webrtc:9112
> Change-Id: I33ba9bae72ec09a45ec70435962f3f25cd31583c
> Reviewed-on: https://webrtc-review.googlesource.com/66945
> Commit-Queue: Qingsi Wang <qingsi@google.com>
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#22814}

TBR=deadbeef@webrtc.org,pthatcher@google.com,pthatcher@webrtc.org,qingsi@google.com,honghaiz@webrtc.org

Change-Id: I2db6561d5d6366d38caa58c3e719d0d48eda70c2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9112
Reviewed-on: https://webrtc-review.googlesource.com/69200
Reviewed-by: Patrik Höglund <phoglund@webrtc.org>
Commit-Queue: Patrik Höglund <phoglund@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22818}
This commit is contained in:
Patrik Höglund
2018-04-11 11:13:57 +00:00
committed by Commit Bot
parent 9df3cf3e8c
commit 3dc41069ef
11 changed files with 98 additions and 215 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
pc/peerconnection.cc
View File

@ -1980,6 +1980,11 @@ void PeerConnection::SetLocalDescription(
PostSetSessionDescriptionSuccess(observer);
// According to JSEP, after setLocalDescription, changing the candidate pool
// size is not allowed, and changing the set of ICE servers will not result
// in new candidates being gathered.
port_allocator_->FreezeCandidatePool();
// MaybeStartGathering needs to be called after posting
// MSG_SET_SESSIONDESCRIPTION_SUCCESS, so that we don't signal any candidates
// before signaling that SetLocalDescription completed.
@ -2816,9 +2821,6 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration,
RTCError* error) {
TRACE_EVENT0("webrtc", "PeerConnection::SetConfiguration");
// According to JSEP, after setLocalDescription, changing the candidate pool
// size is not allowed, and changing the set of ICE servers will not result
// in new candidates being gathered.
if (local_description() && configuration.ice_candidate_pool_size !=
configuration_.ice_candidate_pool_size) {
RTC_LOG(LS_ERROR) << "Can't change candidate pool size after calling "
@ -2976,12 +2978,23 @@ bool PeerConnection::RemoveIceCandidates(
void PeerConnection::RegisterUMAObserver(UMAObserver* observer) {
TRACE_EVENT0("webrtc", "PeerConnection::RegisterUmaObserver");
network_thread()->Invoke<void>(
RTC_FROM_HERE,
rtc::Bind(&PeerConnection::SetMetricObserver_n, this, observer));
uma_observer_ = observer;
if (transport_controller()) {
transport_controller()->SetMetricsObserver(uma_observer_);
}
for (auto transceiver : transceivers_) {
auto* channel = transceiver->internal()->channel();
if (channel) {
channel->SetMetricsObserver(uma_observer_);
}
}
// Send information about IPv4/IPv6 status.
if (uma_observer_) {
if (port_allocator_flags_ & cricket::PORTALLOCATOR_ENABLE_IPV6) {
port_allocator_->SetMetricsObserver(uma_observer_);
if (port_allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_IPV6) {
uma_observer_->IncrementEnumCounter(
kEnumCounterAddressFamily, kPeerConnection_IPv6,
kPeerConnectionAddressFamilyCounter_Max);
@ -3014,25 +3027,6 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) {
}
}
void PeerConnection::SetMetricObserver_n(UMAObserver* observer) {
RTC_DCHECK(network_thread()->IsCurrent());
uma_observer_ = observer;
if (transport_controller()) {
transport_controller()->SetMetricsObserver(uma_observer_);
}
for (auto transceiver : transceivers_) {
auto* channel = transceiver->internal()->channel();
if (channel) {
channel->SetMetricsObserver(uma_observer_);
}
}
if (uma_observer_) {
port_allocator_->SetMetricsObserver(uma_observer_);
}
}
RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) {
if (!worker_thread()->IsCurrent()) {
return worker_thread()->Invoke<RTCError>(
@ -4619,43 +4613,44 @@ bool PeerConnection::InitializePortAllocator_n(
}
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 portallocator_flags = port_allocator_->flags();
portallocator_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);
portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
} else if (webrtc::field_trial::FindFullName("WebRTC-IPv6Default")
.find("Disabled") == 0) {
port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
}
if (configuration.disable_ipv6_on_wifi) {
port_allocator_flags_ &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI);
portallocator_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;
portallocator_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;
portallocator_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;
portallocator_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(portallocator_flags);
// No step delay is used while allocating ports.
port_allocator_->set_step_delay(cricket::kMinimumStepDelay);
port_allocator_->set_candidate_filter(
@ -4681,12 +4676,6 @@ bool PeerConnection::ReconfigurePortAllocator_n(
rtc::Optional<int> stun_candidate_keepalive_interval) {
port_allocator_->set_candidate_filter(
ConvertIceTransportTypeToCandidateFilter(type));
// According to JSEP, after setLocalDescription, changing the candidate pool
// size is not allowed, and changing the set of ICE servers will not result
// in new candidates being gathered.
if (local_description()) {
port_allocator_->FreezeCandidatePool();
}
// Call this last since it may create pooled allocator sessions using the
// candidate filter set above.
return port_allocator_->SetConfiguration(
@ -5088,10 +5077,7 @@ rtc::Optional<std::string> PeerConnection::sctp_transport_name() const {
cricket::CandidateStatsList PeerConnection::GetPooledCandidateStats() const {
cricket::CandidateStatsList candidate_states_list;
network_thread()->Invoke<void>(
RTC_FROM_HERE,
rtc::Bind(&cricket::PortAllocator::GetCandidateStatsFromPooledSessions,
port_allocator_.get(), &candidate_states_list));
port_allocator_->GetCandidateStatsFromPooledSessions(&candidate_states_list);
return candidate_states_list;
}