qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alan Conway <acon...@redhat.com>
Subject Re: [c++] Experience with Alexandrescu's use of "volatile" to enforce locking.
Date Mon, 06 Oct 2008 20:36:21 GMT
On Mon, 2008-10-06 at 16:07 -0400, Andrew Stitcher wrote:
> On Mon, 2008-10-06 at 14:23 -0400, Alan Conway wrote:
> > 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.
> 
> I understand this. To me the assert approach is easy to follow as it is
> self documenting (you are explicitly asserting that a specific lock is
> held), even if it isn't at compile time. The conventions approach -
> yours and the volatile approach - require some knowledge external to the
> code to understand.
> 
> I thought the "volatile" approach had some currency and use, and so
> would be understood - you've demonstrated that isn't true. Your approach
> unfortunately isn't self documenting and makes the code look odd to my
> eyes

Agreed that is a problem. In the past I've also used naming conventions
like fooUnlocked().  I still don't have any solution that I really like.

>  - I'm willing to go along with it, I just happen to think that
> using assert() is better in this case - especially as we have no low
> cost way to do the assertion in any case.

Confused me with that last sentence - the first part says we should  use
assert(), but the second part says we can't?


Mime
View raw message