Fix deadlock in video_receiver.cc.
In webrtc::vcm::VideoReceiver::ResetDecoder(), the lock order is:
1. take _receiveCritSect,
2. take process_crit_sect_
This conflicts with the follow code path:
1. webrtc::vcm::VideoReceiver::Process(), take process_crit_sect_
call -> webrtc::vcm::VideoReceiver::NackList(),
2. with nackStats=kNackKeyFrameRequest, take _receiveCritSect
BUG=2861
TEST=trybots
R=sprang@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/7749004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@5456 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/video_coding/main/source/video_receiver.cc b/webrtc/modules/video_coding/main/source/video_receiver.cc
index 68668ea..bd99311 100644
--- a/webrtc/modules/video_coding/main/source/video_receiver.cc
+++ b/webrtc/modules/video_coding/main/source/video_receiver.cc
@@ -121,8 +121,12 @@
// Key frame requests
if (_keyRequestTimer.TimeUntilProcess() == 0) {
_keyRequestTimer.Processed();
- CriticalSectionScoped cs(process_crit_sect_.get());
- if (_scheduleKeyRequest && _frameTypeCallback != NULL) {
+ bool request_key_frame = false;
+ {
+ CriticalSectionScoped cs(process_crit_sect_.get());
+ request_key_frame = _scheduleKeyRequest && _frameTypeCallback != NULL;
+ }
+ if (request_key_frame) {
const int32_t ret = RequestKeyFrame();
if (ret != VCM_OK && returnValue == VCM_OK) {
returnValue = ret;
@@ -135,16 +139,24 @@
// disabled when NACK is off.
if (_retransmissionTimer.TimeUntilProcess() == 0) {
_retransmissionTimer.Processed();
- CriticalSectionScoped cs(process_crit_sect_.get());
- if (_packetRequestCallback != NULL) {
- uint16_t length = max_nack_list_size_;
+ bool callback_registered = false;
+ uint16_t length;
+ {
+ CriticalSectionScoped cs(process_crit_sect_.get());
+ length = max_nack_list_size_;
+ callback_registered = _packetRequestCallback != NULL;
+ }
+ if (callback_registered && length > 0) {
std::vector<uint16_t> nackList(length);
const int32_t ret = NackList(&nackList[0], &length);
if (ret != VCM_OK && returnValue == VCM_OK) {
returnValue = ret;
}
- if (length > 0) {
- _packetRequestCallback->ResendPackets(&nackList[0], length);
+ if (ret == VCM_OK && length > 0) {
+ CriticalSectionScoped cs(process_crit_sect_.get());
+ if (_packetRequestCallback != NULL) {
+ _packetRequestCallback->ResendPackets(&nackList[0], length);
+ }
}
}
}
@@ -452,7 +464,7 @@
int32_t VideoReceiver::RequestKeyFrame() {
TRACE_EVENT0("webrtc", "RequestKeyFrame");
- CriticalSectionScoped cs(process_crit_sect_.get());
+ CriticalSectionScoped process_cs(process_crit_sect_.get());
if (_frameTypeCallback != NULL) {
const int32_t ret = _frameTypeCallback->RequestKeyFrame();
if (ret < 0) {
@@ -547,6 +559,7 @@
int32_t ret = _decoder->Decode(frame, clock_->TimeInMilliseconds());
// Check for failed decoding, run frame type request callback if needed.
+ bool request_key_frame = false;
if (ret < 0) {
if (ret == VCM_ERROR_REQUEST_SLI) {
return RequestSliceLossIndication(
@@ -557,52 +570,59 @@
VCMId(_id),
"Failed to decode frame %u, requesting key frame",
frame.TimeStamp());
- ret = RequestKeyFrame();
+ request_key_frame = true;
}
} else if (ret == VCM_REQUEST_SLI) {
ret = RequestSliceLossIndication(
_decodedFrameCallback.LastReceivedPictureID() + 1);
}
if (!frame.Complete() || frame.MissingFrame()) {
- CriticalSectionScoped cs(process_crit_sect_.get());
switch (_keyRequestMode) {
case kKeyOnKeyLoss: {
if (frame.FrameType() == kVideoFrameKey) {
- _scheduleKeyRequest = true;
- return VCM_OK;
+ request_key_frame = true;
+ ret = VCM_OK;
}
break;
}
case kKeyOnLoss: {
- _scheduleKeyRequest = true;
- return VCM_OK;
+ request_key_frame = true;
+ ret = VCM_OK;
}
default:
break;
}
}
+ if (request_key_frame) {
+ CriticalSectionScoped cs(process_crit_sect_.get());
+ _scheduleKeyRequest = true;
+ }
TRACE_EVENT_ASYNC_END0("webrtc", "Video", frame.TimeStamp());
return ret;
}
// Reset the decoder state
int32_t VideoReceiver::ResetDecoder() {
- CriticalSectionScoped cs(_receiveCritSect);
- if (_decoder != NULL) {
- _receiver.Initialize();
- _timing.Reset();
- {
- CriticalSectionScoped cs(process_crit_sect_.get());
- _scheduleKeyRequest = false;
+ bool reset_key_request = false;
+ {
+ CriticalSectionScoped cs(_receiveCritSect);
+ if (_decoder != NULL) {
+ _receiver.Initialize();
+ _timing.Reset();
+ reset_key_request = true;
+ _decoder->Reset();
}
- _decoder->Reset();
+ if (_dualReceiver.State() != kPassive) {
+ _dualReceiver.Initialize();
+ }
+ if (_dualDecoder != NULL) {
+ _codecDataBase.ReleaseDecoder(_dualDecoder);
+ _dualDecoder = NULL;
+ }
}
- if (_dualReceiver.State() != kPassive) {
- _dualReceiver.Initialize();
- }
- if (_dualDecoder != NULL) {
- _codecDataBase.ReleaseDecoder(_dualDecoder);
- _dualDecoder = NULL;
+ if (reset_key_request) {
+ CriticalSectionScoped cs(process_crit_sect_.get());
+ _scheduleKeyRequest = false;
}
return VCM_OK;
}
@@ -720,7 +740,6 @@
return VCM_MEMORY;
}
case kNackKeyFrameRequest: {
- CriticalSectionScoped cs(_receiveCritSect);
WEBRTC_TRACE(webrtc::kTraceWarning,
webrtc::kTraceVideoCoding,
VCMId(_id),
