Closed
Bug 828917
Opened 12 years ago
Closed 12 years ago
data race on IncomingVideoStream::incoming_render_thread_
Categories
(Core :: WebRTC: Audio/Video, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: jseward, Assigned: jesup)
References
Details
(Whiteboard: [webrtc][blocking-webrtc+] [data-race] [helgrind][qa-])
Attachments
(1 file)
(deleted),
patch
|
derf
:
review+
|
Details | Diff | Splinter Review |
media/webrtc/trunk/src/modules/video_render/main/source/incoming_video_stream.cc
IncomingVideoStream::IncomingVideoStreamProcess reads
incoming_render_thread_ without holding a lock at line 287. By
contrast, IncomingVideoStream::Stop() reads and writes it only after
doing 'thread_critsect_.Enter()'.
How this can screw up is with the following interleaving:
T1 (in ::IncomingVideoStreamProcess()) vs T2 (::Stop())
T2:241 thread_critsect_.Enter();
T2:242 if (incoming_render_thread_) {
T1:287 if (incoming_render_thread_ == NULL) { /*NOT TAKEN*/
T1:290 }
T1 now believes we are not in a terminating state
T2:243 ThreadWrapper* thread = incoming_render_thread_;
T2:244 incoming_render_thread_ = NULL;
T2 sets it to 'terminating' state
If T1 now accesses incoming_render_thread_ again, we will segfault.
Line 292 is:
thread_critsect_.Enter();
If this was pushed upwards to cover the test at line 287 I think
we'd be safe.
Reporter | ||
Comment 1•12 years ago
|
||
The relevant complaint from Helgrind. The 4th and 5th stacks are the main
race report. Note how the writer holds 3 locks but the reader holds none.
----------------------------------------------------------------
Lock at 0x1F4367D8 was first observed
at 0x4A0A0C9: pthread_mutex_init (hg_intercepts.c:429)
by 0x8061802: webrtc::CriticalSectionPosix::CriticalSectionPosix() (critical_section_posix.cc:26)
by 0x8061749: webrtc::CriticalSectionWrapper::CreateCriticalSection() (critical_section.cc:24)
by 0x807F847: webrtc::IncomingVideoStream::IncomingVideoStream(int, unsigned int) (incoming_video_stream.cc:39)
by 0x8082109: webrtc::ModuleVideoRenderImpl::AddIncomingRenderStream(unsigned int, unsigned int, float, float, float, float) (video_render_impl.cc:470
)
by 0x80A82BC: webrtc::ViERenderer::Init(unsigned int, float, float, float, float) (vie_renderer.cc:120)
by 0x80A841D: webrtc::ViERenderer::CreateViERenderer(int, int, webrtc::VideoRender&, webrtc::ViERenderManager&, unsigned int, float, float, float, flo
at) (vie_renderer.cc:31)
by 0x80A899C: webrtc::ViERenderManager::AddRenderStream(int, void*, unsigned int, float, float, float, float) (vie_render_manager.cc:142)
by 0x808F6F7: webrtc::ViERenderImpl::AddRenderer(int, webrtc::RawVideoType, webrtc::ExternalRenderer*) (vie_render_impl.cc:384)
by 0x74CD429: mozilla::MediaEngineWebRTCVideoSource::Start(mozilla::SourceMediaStream*, int) (MediaEngineWebRTCVideo.cpp:269)
by 0x7344A86: mozilla::MediaOperationRunnable::Run() (MediaManager.h:224)
by 0x7C74686: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:627)
Lock at 0x1F436848 was first observed
at 0x4A0A0C9: pthread_mutex_init (hg_intercepts.c:429)
by 0x8061802: webrtc::CriticalSectionPosix::CriticalSectionPosix() (critical_section_posix.cc:26)
by 0x8061749: webrtc::CriticalSectionWrapper::CreateCriticalSection() (critical_section.cc:24)
by 0x807F850: webrtc::IncomingVideoStream::IncomingVideoStream(int, unsigned int) (incoming_video_stream.cc:40)
by 0x8082109: webrtc::ModuleVideoRenderImpl::AddIncomingRenderStream(unsigned int, unsigned int, float, float, float, float) (video_render_impl.cc:470)
by 0x80A82BC: webrtc::ViERenderer::Init(unsigned int, float, float, float, float) (vie_renderer.cc:120)
by 0x80A841D: webrtc::ViERenderer::CreateViERenderer(int, int, webrtc::VideoRender&, webrtc::ViERenderManager&, unsigned int, float, float, float, float) (vie_renderer.cc:31)
by 0x80A899C: webrtc::ViERenderManager::AddRenderStream(int, void*, unsigned int, float, float, float, float) (vie_render_manager.cc:142)
by 0x808F6F7: webrtc::ViERenderImpl::AddRenderer(int, webrtc::RawVideoType, webrtc::ExternalRenderer*) (vie_render_impl.cc:384)
by 0x74CD429: mozilla::MediaEngineWebRTCVideoSource::Start(mozilla::SourceMediaStream*, int) (MediaEngineWebRTCVideo.cpp:269)
by 0x7344A86: mozilla::MediaOperationRunnable::Run() (MediaManager.h:224)
by 0x7C74686: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:627)
Lock at 0x263391E8 was first observed
at 0x4A0A0C9: pthread_mutex_init (hg_intercepts.c:429)
by 0x8061802: webrtc::CriticalSectionPosix::CriticalSectionPosix() (critical_section_posix.cc:26)
by 0x8061749: webrtc::CriticalSectionWrapper::CreateCriticalSection() (critical_section.cc:24)
by 0x8082437: webrtc::ModuleVideoRenderImpl::ModuleVideoRenderImpl(int, webrtc::VideoRenderType, void*, bool) (video_render_impl.cc:97)
by 0x8082516: webrtc::VideoRender::CreateVideoRender(int, void*, bool, webrtc::VideoRenderType) (video_render_impl.cc:80)
by 0x80A8A37: webrtc::ViERenderManager::AddRenderStream(int, void*, unsigned int, float, float, float, float) (vie_render_manager.cc:128)
by 0x808F6F7: webrtc::ViERenderImpl::AddRenderer(int, webrtc::RawVideoType, webrtc::ExternalRenderer*) (vie_render_impl.cc:384)
by 0x74CD429: mozilla::MediaEngineWebRTCVideoSource::Start(mozilla::SourceMediaStream*, int) (MediaEngineWebRTCVideo.cpp:269)
by 0x7344A86: mozilla::MediaOperationRunnable::Run() (MediaManager.h:224)
by 0x7C74686: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:627)
by 0x7C3B1D0: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:237)
by 0x7C751F9: nsThread::ThreadFunc(void*) (nsThread.cpp:265)
Possible data race during read of size 8 at 0x261A8BB8 by thread #67
Locks held: none
at 0x80803FC: webrtc::IncomingVideoStream::IncomingVideoStreamProcess() (incoming_video_stream.cc:287)
by 0x8080828: webrtc::IncomingVideoStream::IncomingVideoStreamThreadFun(void*) (incoming_video_stream.cc:282)
by 0x8069EDE: webrtc::ThreadPosix::Run() (thread_posix.cc:361)
by 0x8069FB6: StartThread (thread_posix.cc:72)
by 0x4A09F6C: mythread_wrapper (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 8 by thread #45
Locks held: 3, at addresses 0x1F4367D8 0x1F436848 0x263391E8
at 0x80800E1: webrtc::IncomingVideoStream::Stop() (incoming_video_stream.cc:244)
by 0x8081DED: webrtc::ModuleVideoRenderImpl::StopRender(unsigned int) (video_render_impl.cc:653)
by 0x80A8206: webrtc::ViERenderer::StopRender() (vie_renderer.cc:50)
by 0x808F013: webrtc::ViERenderImpl::StopRender(int) (vie_render_impl.cc:258)
by 0x74CD6B4: mozilla::MediaEngineWebRTCVideoSource::Stop(mozilla::SourceMediaStream*, int) (MediaEngineWebRTCVideo.cpp:311)
by 0x7344B16: mozilla::MediaOperationRunnable::Run() (MediaManager.h:247)
by 0x7C74686: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:627)
by 0x7C3B1D0: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:237)
Address 0x261A8BB8 is 40 bytes inside a block of size 328 alloc'd
at 0x4A072A1: operator new(unsigned long) (vg_replace_malloc.c:298)
by 0x80820F8: webrtc::ModuleVideoRenderImpl::AddIncomingRenderStream(unsigned int, unsigned int, float, float, float, float) (video_render_impl.cc:470)
by 0x80A82BC: webrtc::ViERenderer::Init(unsigned int, float, float, float, float) (vie_renderer.cc:120)
by 0x80A841D: webrtc::ViERenderer::CreateViERenderer(int, int, webrtc::VideoRender&, webrtc::ViERenderManager&, unsigned int, float, float, float, float) (vie_renderer.cc:31)
by 0x80A899C: webrtc::ViERenderManager::AddRenderStream(int, void*, unsigned int, float, float, float, float) (vie_render_manager.cc:142)
by 0x808F6F7: webrtc::ViERenderImpl::AddRenderer(int, webrtc::RawVideoType, webrtc::ExternalRenderer*) (vie_render_impl.cc:384)
by 0x74CD429: mozilla::MediaEngineWebRTCVideoSource::Start(mozilla::SourceMediaStream*, int) (MediaEngineWebRTCVideo.cpp:269)
by 0x7344A86: mozilla::MediaOperationRunnable::Run() (MediaManager.h:224)
by 0x7C74686: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:627)
by 0x7C3B1D0: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:237)
by 0x7C751F9: nsThread::ThreadFunc(void*) (nsThread.cpp:265)
by 0x505E156: _pt_root (ptthread.c:158)
----------------------------------------------------------------
Assignee | ||
Comment 2•12 years ago
|
||
Upstream issue with webrtc.org trunk; we need to file an issue (and verify it's not fixed).
Assignee: nobody → rjesup
Depends on: webrtc_updates
Priority: -- → P2
Whiteboard: [webrtc][blocking-webrtc+]
Reporter | ||
Updated•12 years ago
|
Whiteboard: [webrtc][blocking-webrtc+] → [webrtc][blocking-webrtc+] [data-race] [helgrind]
Assignee | ||
Comment 3•12 years ago
|
||
I need to generate a patch for this
Assignee | ||
Comment 4•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #721670 -
Flags: review?(tterribe)
Comment 5•12 years ago
|
||
Comment on attachment 721670 [details] [diff] [review]
move critsect to cover thread existence test (upstream issue 1465)
Review of attachment 721670 [details] [diff] [review]:
-----------------------------------------------------------------
Don't forget to file that upstream issue.
Attachment #721670 -
Flags: review?(tterribe) → review+
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ff35a246bb0
Already filed (note the comment on the patch - Issue 1465)
Comment 7•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Comment 8•12 years ago
|
||
Backed out for now while we investigate bug 848966. I'll re-land whatever comes up clean.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cb82d9f3f9c
Updated•12 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cab296a4940
Windows PGO M2 is looking green.
Comment 10•12 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Whiteboard: [webrtc][blocking-webrtc+] [data-race] [helgrind] → [webrtc][blocking-webrtc+] [data-race] [helgrind][qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•