Boost C++ Libraries: Ticket #5516: Upgrade lock is not acquired when previous upgrade lock releases if another read lock is present

component changed

viboes — Wed, 11 May 2011 06:17:53 GMT

component threads → thread

owner, status changed; cc set

viboes — Wed, 07 Dec 2011 17:59:04 GMT

cc viboes added
owner changed from Anthony Williams to viboes
status new → assigned

It seems to me that this is by definition of the provided feature.

From the documentation: "This is an extension to the multiple-reader / single-write model provided by the SharedLockable concept: a single thread may have upgradable ownership at the same time as others have shared ownership. The thread with upgradable ownership may at any time attempt to upgrade that ownership to exclusive ownership. If no other threads have shared ownership, the upgrade is completed immediately, and the thread now has exclusive ownership, which must be relinquished by a call to unlock(), just as if it had been acquired by a call to lock().

If a thread with upgradable ownership tries to upgrade whilst other threads have shared ownership, the attempt will fail and the thread will block until exclusive ownership can be acquired. " Of course the implementation could give priority to writers, but this should be implemented in another class.

What is then wrong?

Moved to Support request until resolution clarified.

type changed

viboes — Wed, 07 Dec 2011 22:33:07 GMT

type Bugs → Support Requests

fred@… — Wed, 07 Dec 2011 23:34:31 GMT

Quoted: "the thread will block until exclusive ownership can be acquired".

In the example I gave, m3 could have acquired ownership just after m2 released its upgradable lock, because only a read lock was taken at that point, by m1. And yet, it did wait until after m1 released the lock to acquire it.

I understand this sentence of the documentation as meaning the acquisition is eager, but it seems that it is not. Is that correct?

fred@… — Thu, 08 Dec 2011 11:00:58 GMT

The quote in my previous comment is not relevant, actually, sorry about this. My issue is specifically when upgradable ownership may be acquired, because only readers are left, but it is not.

viboes — Fri, 09 Dec 2011 18:04:16 GMT

Coming back to you example

m1 - Trying to take an shared lock m1 - Took a shared lock m2 - Trying to take an upgradable lock m2 - Took an upgradable lock m3 - Trying to take an upgradable lock m2 - Released an upgradable lock m3 - Took an upgradable lock m3 - Releasing locks m1 - Released the lock

Whether these sequence appear before or after in the trace are irrelevant.

m3 - Took an upgradable lock m3 - Releasing locks

m1 - Released the lock

It is up to the scheduler to execute m1 or m3, as both are active threads, no one is blocking for the other. Note that m1 had already a shared lock, so it can continue to be executed independently of whether m3 takes the upgrade lock.

The following sequence could also be possible

m3 - Took an upgradable lock m1 - Released the lock m3 - Releasing locks

fred@… — Fri, 09 Dec 2011 22:50:04 GMT

Viboes, agreed, last steps don't matter. My concern here is on the 7th step:

m1 - Trying to take an shared lock
m1 - Took a shared lock
m2 - Trying to take an upgradable lock
m2 - Took an upgradable lock
m3 - Trying to take an upgradable lock
m2 - Released an upgradable lock
here I would expect m3 to be able to take the upgradable lock, even though m1 still has a read lock
m1 - Released the lock

Because the "release_waiters()" is only called when there are no more readers, we ignore the case when a thread wanting an upgradable lock should be waken up when there are still read locks going on.

This makes the locking non-eager.

type changed

viboes — Thu, 29 Dec 2011 11:37:27 GMT

type Support Requests → Bugs

Replying to fred@…:

Viboes, agreed, last steps don't matter. My concern here is on the 7th step:

m1 - Trying to take an shared lock
m1 - Took a shared lock
m2 - Trying to take an upgradable lock
m2 - Took an upgradable lock
m3 - Trying to take an upgradable lock
m2 - Released an upgradable lock
here I would expect m3 to be able to take the upgradable lock, even though m1 still has a read lock

m1 - Released the lock

Because the "release_waiters()" is only called when there are no more readers, we ignore the case when a thread wanting an upgradable lock should be waken up when there are still read locks going on.

This makes the locking non-eager.

You are right. The fact that in

        void unlock_upgrade()
        {
            boost::mutex::scoped_lock lk(state_change);
            state.upgrade=false;
            bool const last_reader=!--state.shared_count;
            if(last_reader)
            {
                state.exclusive_waiting_blocked=false;
                release_waiters();
            }
        }

state.upgrade has been set at least

shared_cond.notify_all();

must be called.

Please could you try the following

        void unlock_upgrade()
        {
            boost::mutex::scoped_lock lk(state_change);
            state.upgrade=false;
            bool const last_reader=!--state.shared_count;
            if(last_reader)
            {
                state.exclusive_waiting_blocked=false;
                release_waiters();
            } else {
              shared_cond.notify_all();
            }
        }

type changed

viboes — Thu, 29 Dec 2011 14:15:52 GMT

type Bugs → Patches

milestone changed

viboes — Sat, 07 Jan 2012 23:04:32 GMT

milestone To Be Determined → Boost 1.49.0

viboes — Tue, 17 Jan 2012 06:38:28 GMT

Committed in trunk at revision [76543].

status, milestone changed; resolution set

viboes — Mon, 28 May 2012 15:42:48 GMT

status assigned → closed
resolution → fixed
milestone Boost 1.49.0 → Boost 1.50.0

Committed in release branch at [78543]