From bb8ada686e9becb9cea6db79fa691ee5aacfde73 Mon Sep 17 00:00:00 2001 From: "henrika@webrtc.org" Date: Thu, 4 Apr 2013 08:39:09 +0000 Subject: [PATCH] TSan v2 reports data races in WebRTCAudioDeviceTest.FullDuplexAudioWithAGC BUG=226044 TEST=content_unittests in Chrome with TSan v2 enabled Review URL: https://webrtc-codereview.appspot.com/1201010 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3760 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../main/source/audio_coding_module_impl.cc | 28 ++++++++++++------- webrtc/voice_engine/transmit_mixer.cc | 17 +++++++++-- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/webrtc/modules/audio_coding/main/source/audio_coding_module_impl.cc b/webrtc/modules/audio_coding/main/source/audio_coding_module_impl.cc index ba7bde1254..27cce93bca 100644 --- a/webrtc/modules/audio_coding/main/source/audio_coding_module_impl.cc +++ b/webrtc/modules/audio_coding/main/source/audio_coding_module_impl.cc @@ -2053,8 +2053,11 @@ WebRtc_Word32 AudioCodingModuleImpl::IncomingPacket( return 0; } } else { - if (track_neteq_buffer_) - num_bytes_accumulated_ += payload_length; + { + CriticalSectionScoped lock(acm_crit_sect_); + if (track_neteq_buffer_) + num_bytes_accumulated_ += payload_length; + } return neteq_.RecIn(incoming_payload, payload_length, rtp_header); } } @@ -2136,9 +2139,12 @@ WebRtc_Word32 AudioCodingModuleImpl::SetMinimumPlayoutDelay( "Delay must be in the range of 0-10000 milliseconds."); return -1; } - // Don't let the extra delay modified while accumulating buffers in NetEq. - if (track_neteq_buffer_ && first_payload_received_) - return 0; + { + CriticalSectionScoped lock(acm_crit_sect_); + // Don't let the extra delay modified while accumulating buffers in NetEq. + if (track_neteq_buffer_ && first_payload_received_) + return 0; + } return neteq_.SetExtraDelay(time_ms); } @@ -2665,12 +2671,14 @@ WebRtc_Word32 AudioCodingModuleImpl::PlayoutTimestamp( WebRtc_UWord32* timestamp) { WEBRTC_TRACE(webrtc::kTraceStream, webrtc::kTraceAudioCoding, id_, "PlayoutTimestamp()"); - if (track_neteq_buffer_) { - *timestamp = playout_ts_; - return 0; - } else { - return neteq_.PlayoutTimestamp(*timestamp); + { + CriticalSectionScoped lock(acm_crit_sect_); + if (track_neteq_buffer_) { + *timestamp = playout_ts_; + return 0; + } } + return neteq_.PlayoutTimestamp(*timestamp); } bool AudioCodingModuleImpl::HaveValidEncoder(const char* caller_name) const { diff --git a/webrtc/voice_engine/transmit_mixer.cc b/webrtc/voice_engine/transmit_mixer.cc index 242b965c9f..0e4717c274 100644 --- a/webrtc/voice_engine/transmit_mixer.cc +++ b/webrtc/voice_engine/transmit_mixer.cc @@ -56,7 +56,18 @@ TransmitMixer::OnPeriodicProcess() } #endif - if (_saturationWarning) + bool saturationWarning = false; + { + // Modify |_saturationWarning| under lock to avoid conflict with write op + // in ProcessAudio and also ensure that we don't hold the lock during the + // callback. + CriticalSectionScoped cs(&_critSect); + saturationWarning = _saturationWarning; + if (_saturationWarning) + _saturationWarning = false; + } + + if (saturationWarning) { CriticalSectionScoped cs(&_callbackCritSect); if (_voiceEngineObserverPtr) @@ -66,7 +77,6 @@ TransmitMixer::OnPeriodicProcess() " CallbackOnError(VE_SATURATION_WARNING)"); _voiceEngineObserverPtr->CallbackOnError(-1, VE_SATURATION_WARNING); } - _saturationWarning = false; } } @@ -454,6 +464,7 @@ TransmitMixer::EncodeAndSend() WebRtc_UWord32 TransmitMixer::CaptureLevel() const { + CriticalSectionScoped cs(&_critSect); return _captureLevel; } @@ -1311,6 +1322,8 @@ void TransmitMixer::ProcessAudio(int delay_ms, int clock_drift, LOG(LS_ERROR) << "ProcessStream() error: " << err; } + CriticalSectionScoped cs(&_critSect); + // Store new capture level. Only updated when analog AGC is enabled. _captureLevel = agc->stream_analog_level();