Archived

This forum has been archived. Please start a new discussion on GitHub.

bug in RWRecMutex

I think I have found a deadlock in RWRecMutex.

Thread A calls readLock
Thread B calls writeLock
Thread A calls readLock

A co-worker has written a little test application that shows this. He will post it in a moment.

Comments

  • Code that shows deadlock in action.
    #include <IceUtil/RWRecMutex.h>
    #include <IceUtil/Thread.h>
    
    using namespace std;
    
    class Base : public IceUtil::Thread {
    protected:
        IceUtil::RWRecMutex &mut;
    public:
        Base( IceUtil::RWRecMutex &m ) : mut(m) {}
    
    };
    
    class Read : public Base {
    public:
        Read( IceUtil::RWRecMutex &m ) : Base(m) { }
    
        void run() {
            IceUtil::ThreadControl self;
            cout << "Trying to get ReadLock." << endl;
            IceUtil::RWRecMutex::RLock rlock(mut);
            cout << "ReadLock aquired." << endl;
    
            self.sleep(IceUtil::Time::seconds(1));
    
            cout << "Trying to get ReadLock again." << endl;
            IceUtil::RWRecMutex::RLock rlock2(mut);  //Deadlock here.
            cout << "ReadLock aquired again." << endl;
        }
    };
    
    class Write : public Base {
    public:
        Write( IceUtil::RWRecMutex &m ) : Base(m) { }
    
        void run() {
            IceUtil::ThreadControl self;
            cout << "Trying to get WriteLock." << endl;
            IceUtil::RWRecMutex::WLock wlock(mut);
            cout << "WriteLock." << endl;
        }
    };
    
    int main() {
        cout << "Starting." << endl;
        IceUtil::RWRecMutex mut;
        IceUtil::ThreadPtr r = new Read(mut);
        IceUtil::ThreadPtr w = new Write(mut);
    
        IceUtil::ThreadControl rtc, wtc;
    
        rtc = r->start();
        wtc = w->start();
    
        rtc.join();
        wtc.join();
    
        cout << "Finished." << endl;
        return 0;
    }
    
    Under Linux using gcc.
  • Thanks for the report!

    I can reproduce the problem here. The cause is that, while there is a waiting writer, readers are unconditionally blocked when they try to acquire a read lock:
    void
    IceUtil::RWRecMutex::readLock() const
    {
        Mutex::Lock lock(_mutex);
    
        //
        // Wait while a writer holds the lock or while writers or an upgrader
        // are waiting to get the lock.
        //
        while(_count < 0 || _waitingWriters != 0)
        {
            _readers.wait(lock);
        }
        ++_count;
    }
    

    The problem in your example is that the caller of readLock() already holds a lock on the mutex, tries to acquire the lock a second time, but then blocks because there is a waiting writer, which prevents it from releasing the initial lock.

    It seems it would be more appropriate to allow those readers that have a lock already to continue to acquire more read locks, and to only block those readers in readLock() that don't have a lock already.

    We'll discuss this internally.

    Cheers,

    Michi.
  • a little more explanation

    Looking at the source src/IceUtil/RWRecMutex.cpp

    The Read thread calls successfully through readLock(), leaving _count at 1.
    The Write thread calls writeLock and since _count isn't 0 it increments _waitingWriters and then waits on _writers.wait()
    The Read thread recursively calls readLock() and since _waitingWriters != 0 it waits on _readers.wait(). Deadlock!