Re: [boost] RFC: Multithreading design constraints

From: Greg Colvin (gcolvin_at_[hidden])
Date: 2000-06-07 02:32:12

Since this has already been discussed so much I'll be snipping liberally.

From: John Maddock <John_Maddock_at_[hidden]>
> Hi all,
> 
> 
> I've been following the comments on multi-threading recently with interest,
> as it's something that's important to me, I've put together a set of design
> principles (below) that could serve as a "straw man list" starting point
> 
> 
> WHAT IS "THREAD SAFE"?
> ~~~~~~~~~~~~~~~~~~~
> 
> I'm going to shamelessly ape Dave Abraham's exception guarantees here (I
> hope you'll forgive me Dave!) - the idea is to present a coherent set of
> thread safety descriptions so that we can all talk a common language :-)

I agree with other comments that these are not yet crisp enough,
and that "instances of an object" is infelicitous, but they are
a good start.  I'd suggest going back to the basics of atomicity
and invariants.

> ...
> 
> WHAT IS A LOCK?
> ~~~~~~~~~~~~
> 
> At it's simplest, a lock is an object, that allows a section of code to be
> protected so that only one thread at a time (or a resticted set of threads)
> can execute that section of code.  Most existing practice (including ACE)
> uses a scoped guard object to protect a section of code, a typical example
> would look something like this:
> 
> lock_type l;
> int count;
> 
> void foo()
> {
>    lock_type::guard g(l);
>    ++count;
> }

Yes, this is the common idiom.  I like it fine.

> However there are a number of possible defects with this idiom:
> 
> Issue 1:
> The variable g is never used.

Not a problem.

> ...
> Issue 2:
> Compiler instruction reordering:

Not a problem.

> ...
> Issue 3:
> Hardware out of order execution:

Not a problem.

> ...
> Issue 4:
> Dangling braces:
> The main problem with the scoped guard object is that the extent of the
> lock may not coincide with the extent of the scope, one solution is to use
> "dangling braces":
>
> void foo()
> {
>    {
>    lock_type::guard g(l);
>    if(g)
>    {
>       ++count;
>    }
>    else throw timeout_exception();  // could carry on with local processing
> here
>    }
>    /* do something unlocked here: */
> }
> 
> Here the extra braces control scope, but are a maintenance nightmare, a
> programmer new to the code may well wonder why they are present and remove
> them. At best this will impair performance, at worst we may get a deadlock.

This is not so cut and dried, but I have rarely needed the extra
braces in my code, and a comment has served as a warning to future
programmers when I have.  Allowing unskilled programmers to maintain
multi-threaded code is a recipe for disaster anyway.

>  (Thanks to the ACE docs for bringing this one to light).
> Solution: add an "acquire" method to the guard object that lets us control
> the locks duration (an analogous example would be "reset" on the smart
> pointers):
> 
> void foo()
> {
>    lock_type::guard g(l);
>    if(g)
>    {
>       ++count;
>    }
>    else throw timeout_exception();  // could carry on with local processing here
>    g.acquire(false);
>    /* do something unlocked here: */
>    if(g.acquire(true))
>    {
>       /* do something locked here */
>    }
>    else throw timeout_exception();  // could carry on with local processing here
> }

So I would write the above as just

   void foo()
   {
      {  // must lock increment of count
         lock_type::guard g(l);
         ++count;
      }  // lock is now released
    }
    /* do something unlocked here: */
   }

and fire the idiot who deletes the braces and the comments.  Well,
a severe scolding anyway, but they had better have a good excuse.

> Issue 5:
> Unnamed variable bug:
> One problem with the scoped variable method is that it is too easy to
> forget to name the guard variable:
>
> void foo()
> {
>    lock_type::guard(l);
>    ++count;
> }
> 
> Which of course doesn't work, however the requirement to use a subsequent 
> "if" statement more or less knocks this one on the head.

It does, but I still don't like the "if" statement, or the clever abuse
of "for".

> Issue 6:
> ...
> WHAT KIND OF LOCKS?
> ~~~~~~~~~~~~~~~~
> ...
> ATOMIC OPERATIONS
> ~~~~~~~~~~~~~~~~
> 
> Although not available on all platforms the performance gains available
> from native atomic operations can not be ignored, the question is which
> operations?  The following class may provide a starting point for an
> interface:

OK, but increment and decrement can sometimes be much faster than
general addition, and returning a bool can sometimes be much faster
than returning an int.

Also, is all this machinery that much more useful than a single
atomic_integer type with a few operations?

> //
> // primary template, not thread safe:
> template <class T>
> struct atom_traits
> {
>         static const bool is_specialized = false;
>         // read:
>         static const bool has_read = false;
>         T read(volatile T* atom)
>         { return *atom; }
> 
>         // write:
>         static const bool has_write = false;
>         void write(volatile T* atom, const T& value)
>         { *atom = value; }
> 
>         // add:
>         static const bool has_add = false;
>         T add(volatile T* atom, const T& value)
>         { return *atom + value; }
> 
>         // test and set:
>         static const bool has_tas = false;
>         bool tas(volatile T* atom)
>         { bool result = (0 == *atom); *atom = 1; return result; }
> 
>         // test and reset:
>         static const bool has_tar = false;
>         bool tar(volatile T* atom)
>         { bool result = (0 != *atom); *atom = 0; return result; }
> 
>         // compare and exchange:
>         static const bool has_cxc = false;
>         bool cxc(volatile T* atom, T* with, const T& value)
>         { 
>                 if(value == *atom)
>                 { 
>                         std::swap(*atom, with) 
>                         return true;
>                 }
>                 return false;
>         }
> };
> 
> ****
> 
> I hope this can provide some kind of starting point.  I've deliberately
> stayed away from issues like reader/writer locks, events, thread creation
> and management etc, likewise I've stayed away from implementation details -
> most of these are already reasonably well known (in ACE for example) - even
> so I may have taken this too far too soon, whatever I'm sure you guys will
> let me know what you think (and what I've missed!).

Thanks for making this start.  I agree with your choice of issues to
avoid -- I've heard Bjarne say of concurrency something like "I know
twenty-five ways to do it wrong".