Re: [boost] Re: [Boost.Thread] mutex Win32 implementation changes

From: Peter Dimov (pdimov_at_[hidden])
Date: 2004-07-20 06:23:40

Alexander Terekhov wrote:
> Peter Dimov wrote:
> [...]
>> Yes, I see it now. I see no protection in MS's CRITICAL_SECTION
>> against this, either. It's basically (recursivity omitted)
>>
>> void lock( critical_section * p )
>> {
>>     if( atomic_increment(p->LockCount) != 0 )
>>     {
>>         slow_lock_path( p );
>>     }
>> }
>>
>> void unlock( critical_section * p )
>> {
>>     if( atomic_decrement(p->LockCount) >= 0 )
>>     {
>>         slow_unlock_path( p );
>>     }
>> }
>>
>> and it seems to me that slow_unlock_path happily accesses *p, in
>> particular something called p->LockSemaphore (whether it is really a
>> semaphore is another story).
>
> Well, absent try/timed operations, that scheme is "posix safe", but
> slow once you have a bit of contention.

Here's TryEnterCriticalSection (-recursivity) for reference:

bool try_lock( critical_section * p )
{
    return atomic_compare_exchange( p->LockCount, -1, 0 ) == 0;
}

CRITICAL_SECTIONs have no timed_lock.

Your version can be made posix-safe at the expense of three atomic ops
instead of one in unlock:

void unlock()
{
    atomic_increment( refs_ );

    // as before

    if( atomic_decrement( refs_ ) == 0 )
    {
        CloseHandle( event_ );
    }
}

void destroy()
{
    if( atomic_decrement( refs_ ) == 0 )
    {
        CloseHandle( event_ );
    }
}

That's too slow, I guess? But maybe you'd be able to think of a way to
somehow combine the refs_ manipulation with the lock_ manipulation?

The event pool solution would also work, but the problem there is that the
pool would need its own synchronization. Unless it's an "interlocked slist".