On Mon, 2008-10-06 at 13:39 -0400, Andrew Stitcher wrote: > On Mon, 2008-10-06 at 13:29 -0400, Alan Conway wrote: > > On Mon, 2008-10-06 at 11:38 -0400, Andrew Stitcher wrote: > > > On Fri, 2008-10-03 at 17:31 -0400, Alan Conway wrote: > > > > ... > > > > (1) my usual style being: > > > > - all public member functions are thread safe. > > > > - use ScopedLock to acquire/release locks. > > > > - private, unlocked functions that are only intended to be called with > > > > the lock already held take a dummy extra paramater of type ScopedLock& > > > > > > > > The dummy ScopedLock& parameter marks functions that are not locked and > > > > also makes it hard to accidentally call them in an unlocked context. > > > > > > This would explain why I occasionally see odd functions with an unused > > > ScopedLock& parameter, and try and figure out why the person who wrote > > > it did that. Then sigh, and go and remove the useless parameter before > > > checking in my changes. > > > > > > Did I miss a wiki note/or an email? Oh, such is life. > > > > I generally stick a comment in the .h file but very possible I've > > forgotten on occasion. I sent round an email when I first started doing > > this, but that's easy to miss. I'll be more careful to include comments > > in future. > > > > Any other useful conventions out there for marking thread safe/unsafe > > functions? I think the public == thread safe is generally a good rule > > but I've yet to find a satisfactory way of marking private thread-unsafe > > functions as such. > > Generally I just put a comment (in header file or both) saying that a > prerequisite for this function is to be holding a particular lock. > > I think "the correct" way to do this is to use something like > "assert(<lock held>);". I think this is the correct way as I think you > should document (as many as possible) preconditions with asserts. > However we don't have a way of expressing <lock held> efficiently as far > as I know.
The point of the signature change (or Alexandrescus use of volatile) is to catch errors at compile time rather than run time. Asserts and comments don't help with that. The idea of the dummy ScopedLock& parameter is to make it impossible to call the function unless you have a local ScopedLock or were passed a ScopedLock&, so it's more difficult to accidentally call an unlocked function when you don't hold the lock.
