Open Bug 833733 Opened 12 years ago Updated 2 years ago

data race on TracePosix::_prevAPITickCount and probably _prevTickCount too

Categories

(Core :: WebRTC, defect, P5)

x86_64
Linux
defect

Tracking

()

People

(Reporter: jseward, Unassigned)

References

Details

(Whiteboard: [data-race] [helgrind])

In media/webrtc/trunk/src/system_wrappers/source/trace_posix.cc WebRtc_Word32 TracePosix::AddTime(char* traceMessage, const TraceLevel level) const { ... 69 prevTickCount = _prevAPITickCount; 70 _prevAPITickCount = ms_time; ... } _prevAPITickCount is subject to inconsistent locking here, and I suspect _prevTickCount in the then-clause just above. The writing thread holds two locks, MediaEngineWebRTC::mMutex (I think, not 100% sure) and webrtc::voe::SharedData::_apiCritPtr. The reading thread holds neither of these locks. trace_posix.h declares both _prevAPITickCount and _prevTickCount as volatile, which suggests the original authors were aware of the potential multithreaded access. However, as is widely documented, the idea that 'volatile' helps in any way with synchronisation is a myth. eg http://en.wikipedia.org/wiki/Volatile_variable#In_C_and_C.2B.2B ("Operations on volatile variables are not atomic, nor do they establish a proper happens-before relationship for threading.")
Helgrind report: ---------------------------------------------------------------- Lock at 0x1B651720 was first observed at 0x4A0A283: pthread_mutex_init (/home/sewardj/VgTRUNK/merge/helgrind/hg_intercepts.c:429) by 0x5058AD8: PR_NewLock (nsprpub/pr/src/pthreads/ptsynch.c:154) by 0x7352D3B: mozilla::MediaEngineWebRTC::MediaEngineWebRTC() (ff-opt/dom/media/../../dist/include/mozilla/Mutex.h:49) by 0x7352E7A: mozilla::MediaManager::GetBackend() (dom/media/MediaManager.cpp:1043) by 0x735346A: mozilla::GetUserMediaDevicesRunnable::Run() (dom/media/MediaManager.cpp:750) by 0x7CB6858: nsThread::ProcessNextEvent(bool, bool*) (xpcom/threads/nsThread.cpp:630) by 0x7C7D360: NS_ProcessNextEvent_P(nsIThread*, bool) (ff-opt/xpcom/build/nsThreadUtils.cpp:238) by 0x7CB7462: nsThread::ThreadFunc(void*) (xpcom/threads/nsThread.cpp:265) by 0x505E176: _pt_root (nsprpub/pr/src/pthreads/ptthread.c:162) by 0x4A0A126: mythread_wrapper (/home/sewardj/VgTRUNK/merge/helgrind/hg_intercepts.c:219) by 0x30E2C07D13: start_thread (in /usr/lib64/libpthread-2.15.so) by 0x30E28F167C: clone (in /usr/lib64/libc-2.15.so) Lock at 0x29DCEE48 was first observed at 0x4A0A283: pthread_mutex_init (/home/sewardj/VgTRUNK/merge/helgrind/hg_intercepts.c:429) by 0x80B6DC2: webrtc::CriticalSectionPosix::CriticalSectionPosix() (media/webrtc/trunk/src/system_wrappers/source/critical_section_posix.cc:26) by 0x80B6D09: webrtc::CriticalSectionWrapper::CreateCriticalSection() (media/webrtc/trunk/src/system_wrappers/source/critical_section.cc:24) by 0x8140271: webrtc::voe::SharedData::SharedData() (media/webrtc/trunk/src/voice_engine/shared_data.cc:28) by 0x8154D7C: GetVoiceEngine (media/webrtc/trunk/src/voice_engine/voice_engine_impl.h:145) by 0x8154F6C: webrtc::VoiceEngine::Create() (media/webrtc/trunk/src/voice_engine/voice_engine_impl.cc:85) by 0x74D91AF: mozilla::MediaEngineWebRTC::EnumerateAudioDevices(nsTArray<nsRefPtr<mozilla::MediaEngineAudioSource> >*) (content/media/webrtc/MediaEngine WebRTC.cpp:162) by 0x73534A5: mozilla::GetUserMediaDevicesRunnable::Run() (dom/media/MediaManager.cpp:754) by 0x7CB6858: nsThread::ProcessNextEvent(bool, bool*) (xpcom/threads/nsThread.cpp:630) by 0x7C7D360: NS_ProcessNextEvent_P(nsIThread*, bool) (ff-opt/xpcom/build/nsThreadUtils.cpp:238) by 0x7CB7462: nsThread::ThreadFunc(void*) (xpcom/threads/nsThread.cpp:265) by 0x505E176: _pt_root (nsprpub/pr/src/pthreads/ptthread.c:162) Possible data race during read of size 4 at 0x298D7A9C by thread #48 Locks held: none at 0x80C0CB0: webrtc::TracePosix::AddTime(char*, webrtc::TraceLevel) const (media/webrtc/trunk/src/system_wrappers/source/trace_posix.cc:69) by 0x80C03A4: webrtc::TraceImpl::AddImpl(webrtc::TraceLevel, webrtc::TraceModule, int, char const*) (media/webrtc/trunk/src/system_wrappers/source/trace_impl.cc:621) by 0x80C0B74: webrtc::Trace::Add(webrtc::TraceLevel, webrtc::TraceModule, int, char const*, ...) (media/webrtc/trunk/src/system_wrappers/source/trace_impl.cc:821) by 0x80BF466: webrtc::ThreadPosix::Run() (media/webrtc/trunk/src/system_wrappers/source/thread_posix.cc:350) by 0x80BF576: StartThread (media/webrtc/trunk/src/system_wrappers/source/thread_posix.cc:72) by 0x4A0A126: mythread_wrapper (/home/sewardj/VgTRUNK/merge/helgrind/hg_intercepts.c:219) by 0x30E2C07D13: start_thread (in /usr/lib64/libpthread-2.15.so) by 0x30E28F167C: clone (in /usr/lib64/libc-2.15.so) This conflicts with a previous write of size 4 by thread #45 Locks held: 2, at addresses 0x1B651720 0x29DCEE48 at 0x80C0CB6: webrtc::TracePosix::AddTime(char*, webrtc::TraceLevel) const (media/webrtc/trunk/src/system_wrappers/source/trace_posix.cc:70) by 0x80C03A4: webrtc::TraceImpl::AddImpl(webrtc::TraceLevel, webrtc::TraceModule, int, char const*) (media/webrtc/trunk/src/system_wrappers/source/trace_impl.cc:621) by 0x80C0B74: webrtc::Trace::Add(webrtc::TraceLevel, webrtc::TraceModule, int, char const*, ...) (media/webrtc/trunk/src/system_wrappers/source/trace_impl.cc:821) by 0x815B55D: webrtc::AudioDeviceModuleImpl::PlatformAudioLayer() const (media/webrtc/trunk/src/modules/audio_device/main/source/audio_device_impl.cc:2050) by 0x815B631: webrtc::AudioDeviceModuleImpl::CreatePlatformSpecificObjects() (media/webrtc/trunk/src/modules/audio_device/main/source/audio_device_impl.cc:201) by 0x815B86A: webrtc::AudioDeviceModuleImpl::Create(int, webrtc::AudioDeviceModule::AudioLayer) (media/webrtc/trunk/src/modules/audio_device/main/source/audio_device_impl.cc:99) by 0x81467BA: webrtc::VoEBaseImpl::Init(webrtc::AudioDeviceModule*) (media/webrtc/trunk/src/voice_engine/voe_base_impl.cc:360) by 0x74D8CF4: mozilla::MediaEngineWebRTC::EnumerateAudioDevices(nsTArray<nsRefPtr<mozilla::MediaEngineAudioSource> >*) (content/media/webrtc/MediaEngineWebRTC.cpp:174) Address 0x298D7A9C is 224076 bytes inside a block of size 224088 alloc'd at 0x4A072E1: operator new(unsigned long) (/home/sewardj/VgTRUNK/merge/coregrind/m_replacemalloc/vg_replace_malloc.c:298) by 0x80BF58C: webrtc::TraceImpl::CreateInstance() (media/webrtc/trunk/src/system_wrappers/source/trace_impl.cc:65) by 0x80BF6C8: webrtc::TraceImpl::StaticInstance(webrtc::CountOperation, webrtc::TraceLevel) (media/webrtc/trunk/src/system_wrappers/source/../../system_wrappers/interface/static_instance.h:70) by 0x80C09F0: webrtc::Trace::CreateTrace() (media/webrtc/trunk/src/system_wrappers/source/trace_impl.cc:741) by 0x80E7FE0: webrtc::ViESharedData::ViESharedData() (media/webrtc/trunk/src/video_engine/vie_shared_data.cc:38) by 0x80D9339: webrtc::ViEBaseImpl::ViEBaseImpl() (media/webrtc/trunk/src/video_engine/vie_base_impl.cc:63) by 0x80E1484: GetVideoEngine (media/webrtc/trunk/src/video_engine/../video_engine/vie_impl.h:104) by 0x80E15A0: webrtc::VideoEngine::Create() (media/webrtc/trunk/src/video_engine/vie_impl.cc:61) by 0x74D8C8C: mozilla::MediaEngineWebRTC::EnumerateVideoDevices(nsTArray<nsRefPtr<mozilla::MediaEngineVideoSource> >*) (content/media/webrtc/MediaEngineWebRTC.cpp:44) by 0x7353478: mozilla::GetUserMediaDevicesRunnable::Run() (dom/media/MediaManager.cpp:750) by 0x7CB6858: nsThread::ProcessNextEvent(bool, bool*) (xpcom/threads/nsThread.cpp:630) by 0x7C7D360: NS_ProcessNextEvent_P(nsIThread*, bool) (ff-opt/xpcom/build/nsThreadUtils.cpp:238) ----------------------------------------------------------------
Probably is a bug in theory (though likely rare to non-existent in practice), but in a non-critical portion of the webrtc codebase (used only in generating debug messages). This is an upstream bug in webrtc.org code, an issue should be filed there, and note the issue number here.
Severity: normal → minor
Depends on: webrtc_updates
Priority: -- → P5
Whiteboard: [WebRTC] [blocking-webrtc-]
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-] [data-race] [helgrind]
backlog: --- → webRTC+
Rank: 58
Whiteboard: [WebRTC] [blocking-webrtc-] [data-race] [helgrind] → [data-race] [helgrind]
Severity: minor → S4
You need to log in before you can comment on or make changes to this bug.