[boost] Re: Boost.Threads: Do we need all those mutexes?

From: Anthony Williams (anthony_w.geo_at_[hidden])
Date: 2004-07-02 04:24:17

Michael Glassford <glassfordm_at_[hidden]> writes:

> Anthony Williams wrote:

>> Michael Glassford <glassfordm_at_[hidden]> writes:

>>>Anthony Williams wrote:

>>>>Once you have a write lock (the "higher class" of lock), then you keep your
>>>>write lock until you release it. If you want a read lock for the whole
>>>>time, but only a write lock for a portion of that time, then you write code
>>>>like the first example. Your "reverse case" actually fulfils that
>>>>requirement, since there is a portion of the code that only needs a read
>>>>lock. Therefore it should look like:
>>>>
>>>>       void f(read_write_mutex m)
>>>>       {
>>>>           read_write_mutex::read_lock r(m); // we need at least a read lock
>>>>                                             // for everything
>>>>           {
>>>>               read_write_mutex::write_lock w(r);
>>>>               // do stuff that needs the write lock here
>>>>           }
>>>>           if (...)
>>>>           {
>>>>               // we only need the read lock here
>>>>               //...
>>>>           }
>>>>           {
>>>>               read_write_mutex::write_lock w(r);
>>>>               // do more stuff that needs the write lock here
>>>>           }
>>>>       }
>>>
>>>Again, this might be a better way to write it, but how do you prevent
>>>someone from writing it the first way?
>> See above.
>
> The above code has a problem, however. This:
>
>      void f(read_write_mutex m)
>      {
>          read_write_mutex::read_lock w(m);
>              //Block until we get read-lock
>
>          //do stuff
>
>          read_write_mutex::write_lock(r);
>              //Convert to write-lock; may fail; then what?

Don't allow it to fail --- block.

>          //do more stuff
>    }
>
> Is not the same as this:
>
>      void f(read_write_mutex m)
>      {
>          read_write_mutex::read_write_lock l(m, write_locked);
>             //Block until we get write-lock
>
>          //do stuff
>
>          l.demote();
>              //Convert to read-lock; never fails
>
>          //do more stuff
>    }
>
> The reason is that the first example always succeeds in getting the write
> lock eventually, and lock demotion always succeeds as well, so we always get
> the lock we want. In the second example, we can always get the read-lock,
> but the lock promotion may fail, so we may never get the write lock.

Not an issue if promotion is blocking.

> The reason lock promotion may fail is that if two threads holding read locks
> are both waiting for promotion to write-locks, neither will ever get the
> write lock (which can't be obtained until all other read-locks have been
> released) and they will deadlock. So, unless you know of a better solution,
> at most one thread must ever be allowed to wait for lock promotion and any
> other promotion attempts must fail.

In which case, don't do it like that. I see this as the fundamental problem of
promotion --- if the idea of promotion is to ensure that no one has modified
the data since you read it, then if two threads have read locks and try and
promote them to write locks, either you deadlock or one will fail. If it
fails, what do you do? You have to release your read lock to let the other
thread do the write, and then try again. This makes me think that such a
feature is not particularly useful in the general case. Maybe a try_write_lock
variant is in order?

I might code it so that the write_lock does the release/reacquire itself, but
this is a difficult issue. Allowing deadlocks is not a good thing, but I
really don't like the idea of the try_xxx locks because it raises the issue of
"have I got the lock or not?". I'm also not a fan of control by member
functions --- this is a resource, so we should use RAII.

>>>I think one of the most common use cases for lock demotion would be: obtain
>>>a write lock and make modifications to a resource; demote to a read lock to
>>>release other readers and use the resource, making sure that it is still in
>>>the state it was when we had the write lock (i.e., that no other thread
>>>obtained a write lock and changed it between the time we released our write
>>>lock and the time we obtained our read lock).
>>>
>>>With a read/write lock, this is expressed in a straightforward way:
>>>
>>>     void f(read_write_mutex m)
>>>     {
>>>         read_write_mutex::write_lock l(m);
>>>
>>>         //Modify the resource
>>>
>>>         l.demote(); //Release other readers
>>>
>>>         //Use the resource
>>>     }
>>>
>>>With the separate read-lock and write-lock, how do you express this in a way
>>>that is straightforward, doesn't do unnecessary work when the locks are
>>>released, etc.?
>>       void f(read_write_mutex m)
>>       {
>>           read_write_mutex::read_lock r(m);
>>           {
>>               read_write_mutex::write_lock l(r);
>>               //Modify the resource
>>           } // release other readers
>>           //Use the resource
>>       }
>
> This has the same problem I mention above. Unless I'm missing something, it
> also performs unnecessary work (a promotion followed by a demotion, as
> opposed to only a demotion in my example).

I agree there is the possibility for unnecessary work, but in my view it
clearly marks the region where writing is done. With a read-write lock, I can
write:

void g(read_write_mutex::read_write_lock& l);

void f(read_write_mutex& m)
{
    read_write_mutex::read_write_lock l(m,write_locked);
    g(l);
    // Are we still locked for writing, or just reading?
}

void f2(read_write_mutex& m)
{
    read_write_mutex::read_write_lock l(m,read_locked);
    g(l);
    // Are we still locked for just reading, or for writing as well?
}

Whereas with separate locks it's clear what's happening when I write:

void g(read_write_mutex::read_lock& l);
void g(read_write_mutex::write_lock& l);

void f(read_write_mutex& m)
{
    read_write_mutex::read_lock l(m);
    g(l);
    // We're still locked for just reading, whether or not g did writing
}

void f2(read_write_mutex& m)
{
    read_write_mutex::write_lock l(m);
    g(l);
    // We're still locked for writing
}

Anthony
-- 
Anthony Williams
Senior Software Engineer, Beran Instruments Ltd.