New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 11 users
Status: Available
Owner: ----
Cc:
Components:
NextAction: ----
OS: ----
Pri: 2
Type: Bug


Sign in to add a comment
Fix lock-order inversions reported by thread-sanitizer
Project Member Reported by pbos@webrtc.org, Mar 5 2014 Back to list
Thread-sanitizer has an experimental deadlock detector, use it and fix reported lock inversions to avoid deadlocks: https://code.google.com/p/thread-sanitizer/wiki/DeadlockDetector
 
Project Member Comment 1 by pbos@webrtc.org, Mar 5 2014
Blockedon: webrtc:3001
Project Member Comment 2 by pbos@webrtc.org, Mar 5 2014
Blockedon: webrtc:3002
Project Member Comment 3 by pbos@webrtc.org, Mar 5 2014
Blockedon: webrtc:3003
Project Member Comment 4 by pbos@webrtc.org, Mar 25 2014
Cc: glider@chromium.org dvyukov@chromium.org
Project Member Comment 5 by pbos@webrtc.org, Mar 25 2014
Cc: kcc@chromium.org
FYI I'll start pinging the bugs without replies after the M35 cut on Friday.
Project Member Comment 6 by pbos@webrtc.org, May 19 2014
Blockedon: webrtc:3230
Blocking: chromium:372754
Project Member Comment 8 by glider@chromium.org, May 19 2014
I'm enabling detect_deadlocks in TSan via the default options (base/debug/sanitizer_options.cc, https://codereview.chromium.org/272763005)
This shouldn't directly affect WebRTC, but please LMK if it does.
Project Member Comment 9 by pbos@webrtc.org, Jun 22 2014
Blocking: webrtc:3509
Project Member Comment 10 by pbos@webrtc.org, Jul 2 2014
Cc: pbos@webrtc.org
 Issue 3509  has been merged into this issue.
Project Member Comment 11 by mflodman@webrtc.org, Sep 30 2014
Labels: Area-Video
Project Member Comment 12 by bugdroid1@chromium.org, Oct 7 2014
The following revision refers to this bug:
  http://code.google.com/p/webrtc/source/detail?r=7386

------------------------------------------------------------------
r7386 | pbos@webrtc.org | 2014-10-07T14:27:27.932795Z

Changed paths:
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/talk/media/webrtc/webrtcvideoengine2.cc&spec=svn7386&r_previous=7385&r=7386&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/build/tsan_suppressions_webrtc.cc&spec=svn7386&r_previous=7385&r=7386&format=side

Remove potential deadlock in WebRtcVideoEngine2.

Fixes lock-order inversions between capturer's SignalVideoFrame and
WebRtcVideoSendStream. Additionally also removes all deadlock
suppressions for WebRtcVideoEngine2.

R=stefan@webrtc.org
TBR=kjellander@webrtc.org
BUG= 1788 ,2999

Review URL: https://webrtc-codereview.appspot.com/26729004
-----------------------------------------------------------------
Project Member Comment 13 by pbos@webrtc.org, Dec 1 2014
Cc: sprang@webrtc.org mflodman@webrtc.org
 Issue 2959  has been merged into this issue.
Project Member Comment 14 by mflodman@webrtc.org, Dec 10 2014
Labels: EngTriaged Mstone-42
Peter,
What is the status of this bug?

Setting M42, same as for 3001 and 3002 .
Project Member Comment 15 by mflodman@webrtc.org, Dec 10 2014
And ignore my question about status...
Project Member Comment 16 by pbos@webrtc.org, Feb 19 2015
Labels: -Mstone-42 Mstone-43
Can hope for 43, it's not impossible, but don't hold your breath. :)
Project Member Comment 17 by vikasmarwaha@webrtc.org, Mar 31 2015
Did this make for M43?
Project Member Comment 18 by mflodman@webrtc.org, Apr 1 2015
I have one more I'm trying hard to fix before the cut.

Peter,
Your status?
Project Member Comment 19 by pbos@webrtc.org, Apr 1 2015
Labels: -Mstone-43 Mstone-45
We've quite a few supressions still, removing them should be easier with the new API, I don't think we'll make M43. Let's try for M45 for at least the webrtc:: ones.

Current state of suppressions (I don't know whether all trigger or if some are resolved already):

"deadlock:rtc::MessageQueueManager::Clear\n"
"deadlock:rtc::MessageQueueManager::ClearInternal\n"
"deadlock:webrtc::RTCPReceiver::SetSsrcs\n"
"deadlock:webrtc::test::UdpSocketManagerPosixImpl::RemoveSocket\n"
"deadlock:webrtc::vcm::VideoReceiver::RegisterPacketRequestCallback\n"
"deadlock:webrtc::ViECaptureImpl::ConnectCaptureDevice\n"
"deadlock:webrtc::ViEChannel::StartSend\n"
"deadlock:webrtc::ViECodecImpl::GetSendSideDelay\n"
"deadlock:webrtc::ViEEncoder::OnLocalSsrcChanged\n"
"deadlock:webrtc::ViESender::RegisterSendTransport\n"

Project Member Comment 20 by pbos@webrtc.org, Apr 10 2015
Blockedon: webrtc:4534
Project Member Comment 21 by pbos@webrtc.org, Apr 10 2015
Blockedon: webrtc:4535
Project Member Comment 22 by pbos@webrtc.org, Apr 15 2015
Blockedon: webrtc:4552
Project Member Comment 23 by pbos@webrtc.org, Apr 16 2015
Blocking: webrtc:4542
Project Member Comment 24 by pbos@webrtc.org, Apr 16 2015
Blocking: -webrtc:4542
Project Member Comment 25 by pbos@webrtc.org, Apr 16 2015
Blockedon: webrtc:4542
Project Member Comment 26 by bugdroid1@chromium.org, May 28 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/2251d6e17438e1a085ff4f88ad19de513214bec1

commit 2251d6e17438e1a085ff4f88ad19de513214bec1
Author: Peter Boström <pbos@webrtc.org>
Date: Thu May 28 12:10:39 2015

Remove ViESender.

Registers transport on construction removing the need for ViESender as a
hop and removing a potential deadlock by removing RegisterSendTransport.

BUG= 1695 , 2999
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/57449004

Cr-Commit-Position: refs/heads/master@{#9309}

[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/tools/valgrind-webrtc/memcheck/suppressions.txt
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video/call.cc
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video/video_receive_stream.cc
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video/video_send_stream.cc
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video_engine/BUILD.gn
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video_engine/video_engine_core.gypi
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video_engine/vie_channel.cc
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video_engine/vie_channel.h
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video_engine/vie_channel_group.cc
[modify] http://crrev.com/2251d6e17438e1a085ff4f88ad19de513214bec1/webrtc/video_engine/vie_channel_group.h
[delete] http://crrev.com/259bd2034c3d3ee7f2dc4d481e9bf896a3a4d6ef/webrtc/video_engine/vie_sender.cc
[delete] http://crrev.com/259bd2034c3d3ee7f2dc4d481e9bf896a3a4d6ef/webrtc/video_engine/vie_sender.h

Project Member Comment 28 by bugdroid1@chromium.org, Jun 26 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/4b91bd08979fcfb191cdae27ad24936beefce735

commit 4b91bd08979fcfb191cdae27ad24936beefce735
Author: Peter Boström <pbos@webrtc.org>
Date: Fri Jun 26 04:58:16 2015

Move frame input (ViECapturer) to webrtc/video/.

Renames ViECapturer to VideoCaptureInput and initializes several
parameters on construction instead of setters.

Also removes an old deadlock suppression.

BUG= 1695 , 2999
R=asapersson@webrtc.org, mflodman@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/53559004.

Cr-Commit-Position: refs/heads/master@{#9508}

[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/talk/media/webrtc/fakewebrtccall.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/talk/media/webrtc/fakewebrtccall.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/talk/media/webrtc/fakewebrtcvideoengine.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/tools/valgrind-webrtc/memcheck/suppressions.txt
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/test/frame_generator_capturer.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/test/frame_generator_capturer.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/test/vcm_capturer.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/test/vcm_capturer.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/test/video_capturer.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/test/video_capturer.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/BUILD.gn
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/full_stack.cc
[rename] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/video_capture_input.cc
[rename] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/video_capture_input.h
[rename] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/video_capture_input_unittest.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/video_send_stream.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/video_send_stream.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video/webrtc_video.gypi
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video_engine/overuse_frame_detector.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video_engine/overuse_frame_detector.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video_engine/overuse_frame_detector_unittest.cc
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video_engine/video_engine_core_unittests.gyp
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video_engine/vie_defines.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video_engine/vie_encoder.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/video_send_stream.h
[modify] http://crrev.com/4b91bd08979fcfb191cdae27ad24936beefce735/webrtc/webrtc_tests.gypi

Project Member Comment 29 by pbos@webrtc.org, Jul 3 2015
Blockedon: webrtc:4456
Project Member Comment 30 by pbos@webrtc.org, Sep 4 2015
Blockedon: webrtc:4973
Project Member Comment 31 by bugdroid1@chromium.org, Sep 23 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/7cf04452622e8622151d8ab13a43daa437bb8ee1

commit 7cf04452622e8622151d8ab13a43daa437bb8ee1
Author: Peter Boström <pbos@webrtc.org>
Date: Wed Sep 23 15:04:00 2015

Remove ViEChannel::StartSend deadlock suppression.

No longer lock-order inverting since RTP/RTCP modules are instantiated
on construction and no longer guarded by a separate lock.

BUG=webrtc:2999
R=stefan@webrtc.org

Review URL: https://codereview.webrtc.org/1347283004 .

Cr-Commit-Position: refs/heads/master@{#10034}

[modify] http://crrev.com/7cf04452622e8622151d8ab13a43daa437bb8ee1/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc

Project Member Comment 32 by mflodman@webrtc.org, Dec 10 2015
Blocking: -chromium:372754
What's left in this bug Peter?

I'm not allowed to keep chromium:372754 in the blocking field for some reason.
Project Member Comment 33 by kjellander@webrtc.org, Dec 17 2015
Re #32: I think cross-project blocked on/blocking references are not possible until Chromium has migrated to Monorail, unfortunately. 
Project Member Comment 34 by pbos@webrtc.org, Dec 17 2015
Five-six suppressions:

$ git grep "deadlock:"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:62:"deadlock:rtc::MessageQueueManager::Clear\n"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:63:"deadlock:rtc::MessageQueueManager::ClearInternal\n"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:80:"deadlock:webrtc::RTCPReceiver::SetSsrcs\n"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:81:"deadlock:webrtc::test::UdpSocketManagerPosixImpl::RemoveSocket\n"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:82:"deadlock:webrtc::vcm::VideoReceiver::RegisterPacketRequestCallback\n"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:83:"deadlock:webrtc::ViEEncoder::OnLocalSsrcChanged\n"
Project Member Comment 35 by pbos@webrtc.org, Jan 27 2016
Labels: -Mstone-45 Mstone-51
Project Member Comment 36 by bugdroid1@chromium.org, Apr 26 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/d486dc1bd7f1e0eff7118f14b22768067dcd9cef

commit d486dc1bd7f1e0eff7118f14b22768067dcd9cef
Author: Peter Boström <pbos@webrtc.org>
Date: Tue Apr 26 11:23:12 2016

Remove some TSan deadlock suppressions.

Looks like these are no longer actively triggering, so they can be removed.

BUG=webrtc:2999
R=kjellander@webrtc.org

Review URL: https://codereview.webrtc.org/1916923002 .

Cr-Commit-Position: refs/heads/master@{#12502}

[modify] https://crrev.com/d486dc1bd7f1e0eff7118f14b22768067dcd9cef/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc

Project Member Comment 37 by pbos@webrtc.org, Apr 26 2016
Owner: ----
Status: Available
Remaining ones are, for anyone interested in taking over:

pbos@deimos:~/webrtc/src (master)$ git grep deadlock:
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:61:"deadlock:rtc::MessageQueueManager::Clear\n"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:62:"deadlock:rtc::MessageQueueManager::ClearInternal\n"
webrtc/build/sanitizers/tsan_suppressions_webrtc.cc:79:"deadlock:webrtc::test::UdpSocketManagerPosixImpl::RemoveSocket\n"


The MessageQueueManager ones aren't super obvious, and the latter is test-only. Not intending to do anything to that one right now, so marking as available.
Project Member Comment 38 by kjellander@webrtc.org, Oct 5 2016
Labels: M-51
Project Member Comment 39 by anatolid@webrtc.org, Nov 11 2016
Labels: -Mstone-51 -M-51
Removing the (outdated) milestone label since this bug is currently blocked on several open bugs which don't have milestone labels themselves.
Project Member Comment 40 by stefan@webrtc.org, Apr 28 2017
Components: -Video Internals
Video issues have been resolved. Remaining issues are in base/. Moving to Internals.
Project Member Comment 41 by stefan@webrtc.org, Apr 28 2017
Components: -Internals Video
Changed the wrong bug. Moving back.
Sign in to add a comment