On Wed, 2019-08-21 at 12:22 +0300, Nadav Har'El wrote:
> I am guessing (need to verify...) that our rwlock implementation is
> not recursive - a thread already holding the write lock needs to wait
> (forever) for the read lock. If this is true, this is an rwlock bug.

So I was puzzled a bit by the rwlock code. It handles recursive write
lock acquisitions by incrementing a counter:

    // recursive write lock
    if (_wowner == sched::thread::current()) {
        _wrecurse++;
    }

On the unlock side though, it does decrement the counter but then goes
on to wake a write_waiter - which seems wrong. Probably I am missing
something - but why is the second part not inside the else clause where
_wowner is set to nullptr?

void rwlock::wunlock()
{
    WITH_LOCK(_mtx) {
        assert(_wowner == sched::thread::current());

        if (_wrecurse > 0) {
            _wrecurse--;
        } else {
            _wowner = nullptr;
        }

        if (!_write_waiters.empty()) {
            _write_waiters.wake_one(_mtx);
        } else {
            _read_waiters.wake_all(_mtx);
        }
    }
}

I think you're right that if you hold the write lock and then try and
readlock it will fail:

bool rwlock::read_lockable()
{
    return ((!_wowner) && (_write_waiters.empty()));
}

Rick

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/d75b970c54f9fe25cda79f2cc0aa0e7eaee3ec2f.camel%40rossfell.co.uk.

Reply via email to