Concurrent access to data with QReadWriteLock

Question

Using Qt, I've got this code in order to protect access to some shared data between threads. I'm pretty sure the idea is correct, but I don't know if RVO and/or RAII could potentially screw the get function. I'm much more used to C and while I understand the idea, I'm not totally familiar with all the "gotchas" for these two concepts.

class DataManager {
    Q_OBJECT

private:
    QVector<DataType> data;
    QReadWriteLock* rwLock;

public:
    DataManager() {
        rwLock = new QReadWriteLock();
    }
    ~DataManager() {
        delete rwLock;
    }
    Q_DISABLE_COPY(DataManager)

    QVector<DataType> getData() {
        QReadWriteLocker lock(rwLock);
        return data;
    }
    QVector<DataType>* beginModifyData() {
        rwLock->lockForWrite();
        return &data;
    }
    void endModifyData() {
        rwLock->unlock();
        emit dataChanged();
    }

signals:
    void dataChanged();
};

In the get function, is it possible that the RAII-type class QReadWriteLocker unlocks the lock before the return copy is made? Thus allowing some thread, that was waiting to write, to overwrite the data being returned.

Also, if somebody writes

QVector<DataType>& myData = dataManager->getData();

Is it possible, due to RVO, that they get a reference to the actual data?

Also, I'd like to receive comments on the code and idea itself. Below I've outlined the reasons why I chose this approach.

1. Simple to use. Since one can only read a copy of the data, they don't have to worry about synchronization and locking.
2. Taking advantage of the implicit sharing of Qt containers, no actual copies will ever be made when using getData(), thus making read-only access very fast.
3. Since, when modifying the data, one gets a pointer to the actual data, again, in most cases, no copies will be made, unless some thread is caching the data (or one of the copies has not yet gone out of scope of whatever got it).
4. Looking at the documentation and code, a non-recursive QReadWriteLock is ultra fast in the non-contended case, not doing any waiting or context-switching (uses atomic operations on an integer).
5. I could potentially do some data validation/fixup in the endModifyData function if needed.

Thanks.

Answer 1

Locking

Let's see what these member functions actually do, and what can go wrong:

getData

The lock acquired in getData basically only ensures that nobody is currently modifying data while inside. As soon as the caller gets the returned value, another thread is free to modify it.

To be a bit clearer, this is how RAII breaks down:

QVector<DataType> getData() {
    QReadWriteLocker lock(rwLock);
    QVector<DataType> temp = data;
    lock.~QReadWriteLocker(); // unlock
    return temp;
}

So the lock doesn't actually cover subsequent usage.

beginModifyData/endModifyData

These two member functions expect to be called in tandem. But what happens if the caller to beginModifyData forgets to call endModifyData (e.g. because an exception was thrown)? Obviously, in that case the lock never gets released.

Also, somebody might store the returned pointer, thus having unsynchronized access to the data after the lock got released.

General stuff

Is there any specific reason why rwLock has to be a pointer? It could simply be of type QReadWriteLock instead.

Is there any specific reason why beginModifyData returns a QVector<DataType>* instead of a QVector<DataType>&?

Your question regarding RVO

No, that is not how return value optimization (RVO) works. For a better understanding, if the compiler applies RVO it might conceptually rewrite the code to something similar to this:

void getData(QVector<DataType>* return_value) {
    QReadWriteLocker lock(rwLock);
    // construct a new QVector<DataType> at the given address (to be passed from the callsite)
    new(return_value) QVector<DataType>(data);
}

// the callsite would be rewritten from
QVector<DataType> myData = dataManager->getData();
// to
QVector<DataType> myData; // uninitialized!
dataManger->getData(&myData);

So this obviously won't work with a reference.

Without RVO, the reference won't work at all, as the lifetime of the returned object ends at the end of its declaration. However, it will work (at least partially) for a const QVector<DataType>& as lifetimes of temporaries get extended for those (though it will still refer to the copy).