From 947120f9694511b3293f98549a72d0d4d8329043 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Mon, 26 Mar 2018 14:56:37 +0200 Subject: [PATCH] Fix of race on access to send side congestion controller. Modifies RtpTransportControllerSend to store a raw pointer to send side congestion controller(SSCC). This avoids a race between destruction of the send_side_cc_ unique pointer and calling AvailableBandwidth on the SSCC instance from the OnNetworkChanged callback. Bug: None Change-Id: I11f414d7db48ab0b29a049b9488b073c1551113d Reviewed-on: https://webrtc-review.googlesource.com/64640 Commit-Queue: Sebastian Jansson Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#22604} --- call/rtp_transport_controller_send.cc | 3 ++- call/rtp_transport_controller_send.h | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index d471ea4b1f..a0c427a34a 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -62,6 +62,7 @@ RtpTransportControllerSend::RtpTransportControllerSend( &pacer_, bitrate_config, TaskQueueExperimentEnabled())) { + send_side_cc_ptr_ = send_side_cc_.get(); process_thread_->RegisterModule(&pacer_, RTC_FROM_HERE); process_thread_->RegisterModule(send_side_cc_.get(), RTC_FROM_HERE); process_thread_->Start(); @@ -85,7 +86,7 @@ void RtpTransportControllerSend::OnNetworkChanged(uint32_t bitrate_bps, msg.network_estimate.at_time = msg.at_time; msg.network_estimate.bwe_period = TimeDelta::ms(probing_interval_ms); uint32_t bandwidth_bps; - if (send_side_cc_->AvailableBandwidth(&bandwidth_bps)) + if (send_side_cc_ptr_->AvailableBandwidth(&bandwidth_bps)) msg.network_estimate.bandwidth = DataRate::bps(bandwidth_bps); msg.network_estimate.loss_rate_ratio = fraction_loss / 255.0; msg.network_estimate.round_trip_time = TimeDelta::ms(rtt_ms); diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index eff1e1603a..c7b9b037c9 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -91,6 +91,14 @@ class RtpTransportControllerSend final const std::unique_ptr process_thread_; rtc::CriticalSection observer_crit_; TargetTransferRateObserver* observer_ RTC_GUARDED_BY(observer_crit_); + // Caches send_side_cc_.get(), to avoid racing with destructor. + // Note that this is declared before send_side_cc_ to ensure that it is not + // invalidated until no more tasks can be running on the send_side_cc_ task + // queue. + // TODO(srte): Remove this when only the task queue based send side congestion + // controller is used and it is no longer accessed synchronously in the + // OnNetworkChanged callback. + SendSideCongestionControllerInterface* send_side_cc_ptr_; // Declared last since it will issue callbacks from a task queue. Declaring it // last ensures that it is destroyed first. const std::unique_ptr send_side_cc_;