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

Issue metadata

Status: Archived
Owner: ----
Closed: Sep 2017
Cc:
Components:
NextAction: ----
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

Issue description

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: ----
Project Member

Comment 6 by pbos@webrtc.org, Sep 19 2017

Status: Archived (was: Available)

Sign in to add a comment