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 <srte@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22604}
This commit is contained in:
Sebastian Jansson
2018-03-26 14:56:37 +02:00
committed by Commit Bot
parent 5f22365dd7
commit 947120f969
2 changed files with 10 additions and 1 deletions

View File

@ -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);

View File

@ -91,6 +91,14 @@ class RtpTransportControllerSend final
const std::unique_ptr<ProcessThread> 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<SendSideCongestionControllerInterface> send_side_cc_;