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. Andrew >
