Subj : Re: recursive mutexes
To   : comp.programming.threads
From : Uenal Mutlu
Date : Sun May 15 2005 06:48 pm

"Peter Dimov" wrote
> Uenal Mutlu wrote:
> > "Peter Dimov" wrote
> >> Uenal Mutlu wrote:
> >>> "David Schwartz" wrote
> >>>>     Yes, it can and must know that it already has the lock. A
> >>>> function that operates on X should be defined as being called with
> >>>> X locked, or with X unlocked. It could even take a boolean that
> >>>> indicates whether the caller has locked the object or not, though
> >>>> this is rarely the right thing to do.
> >>>
> >>> You are unnecessarily complicating things and make the program
> >>> slower by doing these testings.
> >>
> >> No, he's in fact making the program faster.
> >
> > Are you sure? Let's see:
> >
> > void X::f()
> > {
> >   Lock();
> >   for (int i = 0; i < 1000; i++)
> >     f();
>
> Stack overflow or deadlock here, you probably wanted to call g().

:-) Of course g() was meant.

> >   Unlock();
> > }
> >
> > void X::g()
> > {
> >  if (!IsLocked())
> >    return;
> >  //...do..something...
> > }
> >
> > Are you saying the above version of g() is faster than this one:  ?
> > void X::g()
> > {
> >  //...do..something...
> > }
>
> Of course not. Why would you want to test anything if you could just use the
> above? We're talking about recursive mutexes, remember?

No I haven't. See below

> void X::f()
> {
>     Lock();
>
>     for (int i = 0; i < 1000; i++)
>     {
>         g();
>     }
>
>     Unlock();
> }
>
> void X::g()
> {
>     Lock();
>
>     //...do..something...
>
>     Unlock();
> }

The above solution works only if your Lock() understands recursive locking.
Otherwise a deadlock will happen.

> compared to
>
> void X::f()
> {
>     Lock();
>
>     for (int i = 0; i < 1000; i++)
>     {
>         g( false );
>     }
>
>     Unlock();
> }
>
> void X::g( bool lock )
> {
>     if( lock ) Lock();
>
>     //...do..something...
>
>     if( lock ) Unlock();
> }

I don't like the above because one usually uses a locker
class like this one to automate the unlocking:

Locker
{
  Locker(mutex& Am) : m(Am)
   {
    m.Lock();
   }
  ~Locker()
   {
     m.Unlock();
   }
  private:
    mutex& m;
};

void X::f()
{
   Locker(m);
   for (int i = 0; i < 1000; i++)
     g();
}

void X::g()
{
  //...do..something...
}


> Let's overlook the fact that the correct version is of course:
>
> void X::f()
> {
>     Lock();
>
>
>     for (int i = 0; i < 1000; i++)
>     {
>         g_unlocked();
>     }
>
>     Unlock();
> }
>
>
> void X::g_unlocked()
> {
>     //...do..something...
> }
>
> void X::g()
> {
>     Lock();
>     g_unlocked();
>     Unlock();
> }

You don't need 2 versions of g() if you use recursive locking.
The overhead of recursive locking is neglectable because
it's just incrementing a counter in Lock() and decrementing it in Unlock().

.