Re: Threads: the Mutex class

From: William Kempf (sirwillard_at_[hidden])
Date: 2000-08-09 09:07:40

--- In boost_at_[hidden], Jesse Jones <jejones_at_a...> wrote:
> >Here's a rough starting point for a mutex class.
> 
> I think there are two likely paths Boost can take when providing 
threading
> classes. It can either provide a bare bones set of classes to allow
> portable thread safe classes or it can provide portable classes for 
writing
> multi-threaded code.
> 
> For the bare bones class it's questionable as to whether we need to 
share
> the mutex between processes, have timeouts, or possibly even 
recursive
> timeouts. And even if we decide to create a more comprehensive set 
of
> classes it would arguably be better to have two mutex classes since 
it may
> be possible to implement the primitive one more efficiently on some 
OS's.

I defined things the way I did on purpose, to generate debate about 
some issues.  To me personally, it's very debatable whether or not a 
mutex should be sharable between processes.  On the one hand it's an 
essential quality for a lot of concurrent programming chores.  On the 
other hand, it adds a lot of overhead and may be very difficult 
(impossible?) to implement on some platforms today.  Timeouts are not 
quite the same.  They shouldn't add any overhead to the mutex and 
it's essential in most code to be able to at the very least do a 
try_lock.  I opted to use a timed lock instead in order to generate 
debate about timed vs. try.  I don't think it's an option that we 
have neither.  Recursion is another very debatable topic.  It does 
add overhead, though in all honesty a recursive mutex can be modeled 
over a non-recursive mutex with very little effort and minimal impact 
to speed and size.  Since recursive mutexes will prevent a lot of 
deadlock conditions I'm in favor of them, but let's hear the 
opponents to this speak up.
 
> Windows for example has three primary synchronization mechanisms:
> 
> 1) Critical Sections are recursive, but don't have timeouts, and 
can't be
> accessed from a different process. However, in the absence of 
contention,
> they're very fast to lock.

The critical section does, however, have a try lock (at least on 
NT).  The absence of a timed lock (or a more portable try lock) has 
resulted in a lot of code using the mutex in favor of the lighter 
weight and faster critical section.  For people who are absolutely 
dependent on getting the most performance possible then my interface 
may not be great on the Windows platform... but should a decision as 
important as this be made on such a corner case?

> 2) Mutexes are like criticals sections but may have a timeout and 
can be
> accessed from a different thread (eg by name).

By name is actually the only way ;).
 
> 3) Semaphores are like mutexes but also include a count.

You missed Events.

> The classs below looks like a good start, but in most cases it 
provides
> more functionality and overhead than I need.

Overhead depends on implementation.  As for functionality... you 
haven't said what functionality you think should be missing.  Unless 
it's the timed lock you don't want... and I still think we need 
either timed or try locks.
 
> >class mutex
> >{
> >private:
> >	// Should copying be allowed?
> >	mutex(const mutex&);
> >	operator=(const mutex&);
> 
> Maybe. I can imagine a thread safe wrapper around an STL container 
with the
> STL container and a mutex as member data. It might be nice if the 
wrapper
> could use the compiler provided copy ctor. But the new mutex 
shouldn't copy
> the old mutex's locked state.

Probably not.  Another thought on copy-semantics is that it behaves 
like a smart_ptr copy.  For some cases this seems appropriate, but in 
general I'd expect it to be the wrong choice.  I mention it only to 
get people to thinking about possible semantics for a copy... IF a 
copy should even be allowed.
 
> >public:
> >	// Creates a new mutex.  If a name is specified the mutex
> >	// will be created in such a way that it's sharable between
> >	// processes. Should the sharable functionality be included?
> >	// Today this would require support from the platform, and
> >	// if the language were to adopt it it could be complicated
> >	// to include this on all platforms.  However, the platforms
> >	// I'm familiar with would allow for this and it's almost an
> >	// essential quality for some constructs.
> >	mutex(char* name=0);
> 
> This should be a const pointer of course. Probably explicit as 
well. I also
> would prefer a NULL instead of 0. :-)

The first two points are obvious.  I was more concerned with 
specifying the syntax then I was in tightening up the interface 
specifications here.  As for the NULL... that's a preference thing at 
best ;).

> I'm a bit dubious about the notion of
> the ctor being smart enough to create the correct OS object. To do 
this
> right you also need to know if a timeout will be used with lock 
(and you
> can't wait until lock to create the OS object because of race 
conditions).

You're thinking too much about one platform (Windows) and about 
getting the most performance.  Even on Windows you might get better 
performance by using a spin lock instead of the OS primitive critical 
section.  In that case you'd be able to include portable try and/or 
timed locks.  However, a Mutex is fast enough on Windows for most 
purposes any way.
 
> Unless there's a relatively popular platform with threads, but in-
process
> only I'm inclined to think we should include the sharable 
functionality.

I am to some extent.  It may be better to have a named_mutex with 
similar semantics then to force this issue onto the mutex class.  It 
may even be that this is something we just shouldn't have because of 
portability constraints.
 
> It might be worthwhile to allow for a mutex to start out locked. 
The idea
> behind this is that other threads will be blocked allowing you to 
do some
> additional initialization without worrying about race conditions.

In general there shouldn't be race conditions here.  If we allow 
named mutexes then the potential exists, and we should consider it.  
I left it out initially because we've removed all ability to unlock 
the mutex from the public interface here.  This could be remedied by 
modifying the mutex::lock class, and in fact after thinking about it 
overnight I'm in favor of adding lock and unlock methods to this 
inner class.  We should rename it to sentry to avoid name clashes 
with the function names.  Refined mutex::lock (now mutex::sentry) 
would look like this:

class sentry
{
public:
   explicit sentry(mutex& mx, bool locked=true);
   ~sentry();

   void lock();
   bool lock(int timeout);
   void unlock();
};

> >	// Destoys the mutex.
> >	~mutex();
> >
> >	// Used to lock and unlock the mutex.
> >	class lock
> >	{
> >	public:
> >		lock(mutex& mx);
> >		// Throws an exception if timed out
> 
> What exception?

One we'd have to define, of course.  That shouldn't cloud the 
discussion here, so I left the specifics out.

> >		lock(mutex& mx, int timeout);
> 
> Seconds? Milliseconds?

I originally had a comment stating milliseconds.  Obviously I dropped 
the comment before posting.  Sorry about that.

> >		~lock();
> >	};
> >
> >private:
> >	// Locks the mutex.  If already locked the thread enters a
> >	// suspended state until it can lock the mutex.  Should this
> >	// be recursive?
> >	void do_lock();
> 
> Recursive sounds nice to me. :-)

It does to me as well, but others will not agree.  Let's hear their 
POV.
 
> >	// Attempts to lock the mutex.  If the mutex can not be
> >	// locked within the given time frame the result is false;
> >	// otherwise it's true.
> >	bool do_lock(int timeout);
> 
> This should just throw. That way implementors have at least a shot 
at using
> any error codes returned by the OS.

This is an internal method.  In my original design the c-tor that 
called it did the throwing.  With the revised sentry it may be 
appropriate to throw here instead, but I'm not buying that just yet.  
Exceptions are a pretty heavy handed form of error handling.  The 
code required to use a timed lock would become burdensome if a time 
out threw an exception.  The only (typically) valid reason for this 
call to fail would be because of timeout, so we shouldn't need access 
to any error codes returned by the OS.  If the rare cases are 
important then maybe they should be the only cases that throw, but a 
time out would still result in a simple false return.
 
> >	// Unlocks the mutex.
> >	void do_unlock();
> >
> >	// Should we have an is_locked()?  If so, does it report a
> >	// lock by any thread or only by the current thread?
> 
> Only if someone comes up with a good reason for having it.

Obviously ;).  IMHO an is_locked method rarely, if ever, is useful.  
It can't be used in conjunction with a lock in an atomic operation, 
so it's not as useful as first thought by people new to concurrent 
programming.  Its presence may confuse the user because of this.

William Kempf