Remove deadlock in WebRtcVideoEngine2.
Acquiring stream_lock_ in WebRtcVideoChannel2 in a callback from Call forms a lock-order inversion between process-thread locks and libjingle locks, manifesting as CPU adaptation requests blocking on stream creation that is blocked on the CPU adaptation request finishing. R=asapersson@webrtc.org, mflodman@webrtc.org BUG=4535,chromium:475065 Review URL: https://webrtc-codereview.appspot.com/50679004 Cr-Commit-Position: refs/heads/master@{#8985}
This commit is contained in:
parent
6fc2d2f487
commit
e7b221f476
@ -633,6 +633,7 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
|
|||||||
external_decoder_factory_(external_decoder_factory) {
|
external_decoder_factory_(external_decoder_factory) {
|
||||||
SetDefaultOptions();
|
SetDefaultOptions();
|
||||||
options_.SetAll(options);
|
options_.SetAll(options);
|
||||||
|
options_.cpu_overuse_detection.Get(&signal_cpu_adaptation_);
|
||||||
webrtc::Call::Config config(this);
|
webrtc::Call::Config config(this);
|
||||||
config.overuse_callback = this;
|
config.overuse_callback = this;
|
||||||
if (voice_engine != NULL) {
|
if (voice_engine != NULL) {
|
||||||
@ -1144,6 +1145,7 @@ bool WebRtcVideoChannel2::SetCapturer(uint32 ssrc, VideoCapturer* capturer) {
|
|||||||
LOG(LS_INFO) << "SetCapturer: " << ssrc << " -> "
|
LOG(LS_INFO) << "SetCapturer: " << ssrc << " -> "
|
||||||
<< (capturer != NULL ? "(capturer)" : "NULL");
|
<< (capturer != NULL ? "(capturer)" : "NULL");
|
||||||
assert(ssrc != 0);
|
assert(ssrc != 0);
|
||||||
|
{
|
||||||
rtc::CritScope stream_lock(&stream_crit_);
|
rtc::CritScope stream_lock(&stream_crit_);
|
||||||
if (send_streams_.find(ssrc) == send_streams_.end()) {
|
if (send_streams_.find(ssrc) == send_streams_.end()) {
|
||||||
LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc;
|
LOG(LS_ERROR) << "No sending stream on ssrc " << ssrc;
|
||||||
@ -1152,12 +1154,17 @@ bool WebRtcVideoChannel2::SetCapturer(uint32 ssrc, VideoCapturer* capturer) {
|
|||||||
if (!send_streams_[ssrc]->SetCapturer(capturer)) {
|
if (!send_streams_[ssrc]->SetCapturer(capturer)) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (capturer) {
|
if (capturer) {
|
||||||
capturer->SetApplyRotation(
|
capturer->SetApplyRotation(
|
||||||
!FindHeaderExtension(send_rtp_extensions_,
|
!FindHeaderExtension(send_rtp_extensions_,
|
||||||
kRtpVideoRotationHeaderExtension));
|
kRtpVideoRotationHeaderExtension));
|
||||||
}
|
}
|
||||||
|
{
|
||||||
|
rtc::CritScope lock(&capturer_crit_);
|
||||||
|
capturers_[ssrc] = capturer;
|
||||||
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1333,6 +1340,10 @@ bool WebRtcVideoChannel2::SetOptions(const VideoOptions& options) {
|
|||||||
// No new options to set.
|
// No new options to set.
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
{
|
||||||
|
rtc::CritScope lock(&capturer_crit_);
|
||||||
|
options_.cpu_overuse_detection.Get(&signal_cpu_adaptation_);
|
||||||
|
}
|
||||||
rtc::DiffServCodePoint dscp = options_.dscp.GetWithDefaultIfUnset(false)
|
rtc::DiffServCodePoint dscp = options_.dscp.GetWithDefaultIfUnset(false)
|
||||||
? rtc::DSCP_AF41
|
? rtc::DSCP_AF41
|
||||||
: rtc::DSCP_DEFAULT;
|
: rtc::DSCP_DEFAULT;
|
||||||
@ -1372,15 +1383,19 @@ void WebRtcVideoChannel2::OnMessage(rtc::Message* msg) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void WebRtcVideoChannel2::OnLoadUpdate(Load load) {
|
void WebRtcVideoChannel2::OnLoadUpdate(Load load) {
|
||||||
rtc::CritScope stream_lock(&stream_crit_);
|
// OnLoadUpdate can not take any locks that are held while creating streams
|
||||||
for (std::map<uint32, WebRtcVideoSendStream*>::iterator it =
|
// etc. Doing so establishes lock-order inversions between the webrtc process
|
||||||
send_streams_.begin();
|
// thread on stream creation and locks such as stream_crit_ while calling out.
|
||||||
it != send_streams_.end();
|
rtc::CritScope stream_lock(&capturer_crit_);
|
||||||
++it) {
|
if (!signal_cpu_adaptation_)
|
||||||
it->second->OnCpuResolutionRequest(load == kOveruse
|
return;
|
||||||
? CoordinatedVideoAdapter::DOWNGRADE
|
for (auto& kv : capturers_) {
|
||||||
|
if (kv.second != nullptr && kv.second->video_adapter() != nullptr) {
|
||||||
|
kv.second->video_adapter()->OnCpuResolutionRequest(
|
||||||
|
load == kOveruse ? CoordinatedVideoAdapter::DOWNGRADE
|
||||||
: CoordinatedVideoAdapter::UPGRADE);
|
: CoordinatedVideoAdapter::UPGRADE);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
bool WebRtcVideoChannel2::SendRtp(const uint8_t* data, size_t len) {
|
bool WebRtcVideoChannel2::SendRtp(const uint8_t* data, size_t len) {
|
||||||
@ -1962,18 +1977,6 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetMaxBitrateBps(
|
|||||||
last_dimensions_.width = 0;
|
last_dimensions_.width = 0;
|
||||||
SetDimensions(width, last_dimensions_.height, last_dimensions_.is_screencast);
|
SetDimensions(width, last_dimensions_.height, last_dimensions_.is_screencast);
|
||||||
}
|
}
|
||||||
void WebRtcVideoChannel2::WebRtcVideoSendStream::OnCpuResolutionRequest(
|
|
||||||
CoordinatedVideoAdapter::AdaptRequest adapt_request) {
|
|
||||||
rtc::CritScope cs(&lock_);
|
|
||||||
bool adapt_cpu;
|
|
||||||
parameters_.options.cpu_overuse_detection.Get(&adapt_cpu);
|
|
||||||
if (!adapt_cpu)
|
|
||||||
return;
|
|
||||||
if (capturer_ == NULL || capturer_->video_adapter() == NULL)
|
|
||||||
return;
|
|
||||||
|
|
||||||
capturer_->video_adapter()->OnCpuResolutionRequest(adapt_request);
|
|
||||||
}
|
|
||||||
|
|
||||||
void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() {
|
void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() {
|
||||||
if (stream_ != NULL) {
|
if (stream_ != NULL) {
|
||||||
|
@ -295,9 +295,6 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
|
|||||||
|
|
||||||
void SetMaxBitrateBps(int max_bitrate_bps);
|
void SetMaxBitrateBps(int max_bitrate_bps);
|
||||||
|
|
||||||
void OnCpuResolutionRequest(
|
|
||||||
CoordinatedVideoAdapter::AdaptRequest adapt_request);
|
|
||||||
|
|
||||||
private:
|
private:
|
||||||
// Parameters needed to reconstruct the underlying stream.
|
// Parameters needed to reconstruct the underlying stream.
|
||||||
// webrtc::VideoSendStream doesn't support setting a lot of options on the
|
// webrtc::VideoSendStream doesn't support setting a lot of options on the
|
||||||
@ -497,6 +494,13 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
|
|||||||
DefaultUnsignalledSsrcHandler default_unsignalled_ssrc_handler_;
|
DefaultUnsignalledSsrcHandler default_unsignalled_ssrc_handler_;
|
||||||
UnsignalledSsrcHandler* const unsignalled_ssrc_handler_;
|
UnsignalledSsrcHandler* const unsignalled_ssrc_handler_;
|
||||||
|
|
||||||
|
// Separate list of set capturers used to signal CPU adaptation. These should
|
||||||
|
// not be locked while calling methods that take other locks to prevent
|
||||||
|
// lock-order inversions.
|
||||||
|
rtc::CriticalSection capturer_crit_;
|
||||||
|
bool signal_cpu_adaptation_ GUARDED_BY(capturer_crit_);
|
||||||
|
std::map<uint32, VideoCapturer*> capturers_ GUARDED_BY(capturer_crit_);
|
||||||
|
|
||||||
rtc::CriticalSection stream_crit_;
|
rtc::CriticalSection stream_crit_;
|
||||||
// Using primary-ssrc (first ssrc) as key.
|
// Using primary-ssrc (first ssrc) as key.
|
||||||
std::map<uint32, WebRtcVideoSendStream*> send_streams_
|
std::map<uint32, WebRtcVideoSendStream*> send_streams_
|
||||||
|
Loading…
x
Reference in New Issue
Block a user