Project: webrtc Issues People Development process History Sign in
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 5 users
Status: Available
Owner: ----
Cc:
Components:
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment
CriticalSectionWrapper should have an AssertAcquired() method
Reported by fischman@webrtc.org, Sep 9 2013 Back to list
Came up in CR: http://webrtc-codereview.appspot.com/2032004/diff/92001/webrtc/modules/audio_device/android/opensles_input.cc#newcode377
If CriticalSectionWrapper::AssertAcquired() existed then code could be better self-documenting.
CSW::AA() should assert(false) if the lock is not held and be a no-op in NDEBUG mode.
Sample impl in Chromium: https://code.google.com/p/chromium/codesearch#chromium/src/base/synchronization/lock.h&sq=package:chromium&type=cs&l=32
 
Comment 1 by vrk@webrtc.org, Dec 17 2014
Labels: Area-PeerConnection
Project Member Comment 2 by pthatcher@webrtc.org, Jan 6 2015
Labels: EngTriaged
Owner: pbos@webrtc.org
Hey, pbos, who owns SystemWrappers?
Project Member Comment 3 by pbos@webrtc.org, Jan 7 2015
Cc: pbos@webrtc.org
Owner: pthatcher@webrtc.org
Everyone, enjoy! :)

Note that we do have (and use) thread annotations in webrtc/base/thread_annotations.h to compile-time enforce that locks are taken. These seem to be enough for us in most cases. I think we can close this bug unless you disagree.

In the example fischman@ pointed to HandleOverrun can be annotated as requiring a lock to be taken which would've solved this in compile-time (better than runtime asserts).
Project Member Comment 4 by pthatcher@webrtc.org, Nov 8 2016
Labels: Pri-3
Project Member Comment 5 by anatolid@webrtc.org, Dec 14 2016
Cc: pthatcher@webrtc.org
Owner: ----
Sign in to add a comment