From: Yuval Ronen (ronen_yuval_at_[hidden])
Date: 2007-08-22 15:33:29
Howard Hinnant wrote:
> On Aug 22, 2007, at 1:07 PM, Yuval Ronen wrote:
>
>> Howard Hinnant wrote:
>>> On Aug 22, 2007, at 11:22 AM, Yuval Ronen wrote:
>>>
>>>> Howard Hinnant wrote:
>>>>> On Aug 22, 2007, at 8:05 AM, Yuval Ronen wrote:
>>>>>
>>>>>> Yuval Ronen wrote:
>>>>>>> Howard Hinnant wrote:
>>>>>>>
>>>>>>>> Because of this, it is
>>>>>>>> not possible (in the above use case) for there to be a set_mutex
>>>>>>>> on
>>>>>>>> the condition to change the facade, since both facades are
>>>>>>>> simultaneously in use.
>>>>>>> Yes, I've realized that too late. My set_mutex() function is
>>>>>>> useless
>>>>>>> because it has to be atomic with the wait().
>>>>>> I've just thought that it might be not so useless after all.
>>>>>> 'set_mutex'
>>>>>> is supposed to be called after the mutex was locked, and before
>>>>>> calling
>>>>>> condition::wait. Because the mutex is locked, we are protected
>>>>>> against
>>>>>> simultaneous use. There can be a problem when multiple readers
>>>>>> lock
>>>>>> for
>>>>>> read, and simultaneously call set_mutex, but if we assume they all
>>>>>> set
>>>>>> the same mutex it shouldn't be a problem.
>>>>> How would wake from wait be handled? I.e. what mutex (facade)
>>>>> would
>>>>> it use to lock with? The last one set may not correspond to the
>>>>> proper one on wake.
>>>> It would lock the same mutex it unlocked upon entering wait().
>>> Could you prototype or sketch this out? Sorry, I'm not following.
>> Perhaps some code will help me clarify my ideas (taking your
>> advice :) ):
>>
>> template <class Mutex>
>> class condition
>> {
>> typedef Mutes mutex_type;
>> mutex_type *m_mutex;
>>
>> public:
>> condition() : m_mutex(NULL) { }
>> explicit condition(mutex_type &a_mutex) : m_mutex(&a_mutex) { }
>>
>> void set_mutex(mutex_type &a_mutex) { m_mutex = &a_mutex; }
>>
>> void wait()
>> {
>> assert(m_mutex); // overhead only in debug builds
>> do_wait(*m_mutex);
>> }
>> };
>>
>> Of course that's a very partial implementation, but I hope it's enough
>> to convey my intent.
>
> It is the do_wait function that I didn't know how to implement. But
> maybe something like:
>
> do_wait(mutex_type& m)
> {
> internal_mutex.lock();
> mutex_type* local_m = &m;
> m.unlock();
> sleep on internal_mutex;
> internal_mutex.unlock();
> local_m->lock();
> }
>
> The local_m is used in case someone calls set_mutex with another
> facade while we're sleeping. I think that will work, but of course
> haven't tested it.
Actually you don't need local_m. The function parameter 'm' is itself a
local variable (of type 'mutex_type&'), just like local_m;
About the rest of the function details, I'm sure you know them much
better than I do. What I do know, is that if the current do_wait accept
a lock, there should be no problem converting it to accept a mutex.
> Now looking at the client side:
>
> shared_mutex mut;
> condition<shared_mutex> cv;
>
> void foo()
> {
> scoped_lock<shared_mutex> _(mut.exclusive());
> while (cant_write())
> {
> cv.set_mutex(mut.exclusive())
> cv.wait();
> }
> // now safe to write to protected data
> }
>
> Does that look about right for what you are suggesting?
Yes.
> I guess the facades are member data in the shared_mutex? If not, who
> manages their storage?
Yes, making them data members sounds right, for the reason you specify.
> I at first put the call to set_mutex outside the while-statement, but
> then realized that doesn't work. Someone else could set_mutex while
> you're sleeping, then you get a spurious wakeup, then you sleep with
> the wrong facade on the next iteration. So you have to use the
> set_mutex directly before the call to wait().
I agree.
But one needs to remember that calling set_mutex is necessary only in
the rare cases (which I have yet to see a use case for) where the same
mutex/cv pair needs to be wait()ed by both readers and writers.