QAbstractListModel populated in a Worker Thread (not the GUI one)?

[mbruel]

Hi,

Can we populate the data of a QAbstractListModel (or even a QSortFilterProxyModel) outside the View thread?

I've found this thread where it says it should work if we use a Mutex and this one where it gives a hack to make it work but I still have rarely some side effects with my view having extra empty lines.

I'm working on ClementineRemote.
Basically my GUI has 2 Views using either a PlayListModel or a RemoteFileModel.

The first case, is even using a PlayListProxyModel and I've opened this thread where I've found a hack to make the View "synchronized".

With the second case where I'm only using a RemoteFileModel in the ListView in QML, I'm still having some synchronization issues.
So the ListView is using a relying on QList<RemoteFile>.
It is populated on the ConnectionWorker Thread here:

void ClementineRemote::rcvListOfRemoteFiles(const pb::remote::ResponseFiles &files)
{
    if (files.error().size())
        qDebug() << "[MsgType::LIST_FILES] ERROR: " << files.error().c_str();
    else
    {

        QMutexLocker lock(&_secureRemoteFiles);
        if (_remoteFiles.size())
        {
            emit preClearRemoteFiles(_remoteFiles.size() - 1);
            _remoteFiles.clear();
            emit postClearRemoteFiles();
        }

//        // HACK to resolve ListView issue (displaying empty Rows after the number of elements)
//        int nbFiles = _remoteFiles.size();
//        Q_UNUSED(nbFiles)

        qint32 nbRemoteFiles = files.files_size();
        if (nbRemoteFiles)
        {
            emit preAddRemoteFiles(nbRemoteFiles -1 );
            _remoteFiles.reserve(nbRemoteFiles);

            for (auto it = files.files().cbegin(), itEnd = files.files().cend();
                 it != itEnd; ++it)
            {
//                qDebug() << (it->isdir()?"dir":"file") << " : " << it->filename().c_str();
                _remoteFiles << RemoteFile(it->filename(), it->isdir());
            }
            emit postAddRemoteFiles();
        }

        qDebug() << "[MsgType::LIST_FILES] Nb Files: " << _remoteFiles.size();


        _remoteFilesPath = files.relativepath().c_str();
        emit updateRemoteFilesPath(_remoteFilesPath);
    }
}

It is stable when I let debug traces. When they are commented, I've some times some artefacts when I repopulate the data (_remoteFiles) with less elements. In the View I'd see extra empty line.

I've added a Mutex used when I repopulate the data (cf the function above) and I'm also using it in the data and rowCount methods of the RemoteFileModel:

int RemoteFileModel::rowCount(const QModelIndex &parent) const
{
    // For list models only the root node (an invalid parent) should return the list's size. For all
    // other (valid) parents, rowCount() should return 0 so that it does not become a tree model.
    if (parent.isValid() || !_remote)
        return 0;

    QMutexLocker lock(_remote->secureRemoteFiles());
    return _remote->numberOfRemoteFiles();
}

QVariant RemoteFileModel::data(const QModelIndex &index, int role) const
{
    if (!index.isValid() || !_remote)
        return QVariant();

    QMutexLocker lock(_remote->secureRemoteFiles());

    // https://stackoverflow.com/questions/9485339/design-pattern-qt-model-view-and-multiple-threads
    if(_remote->numberOfRemoteFiles() <= index.row())
        return QVariant();

    const RemoteFile &file = _remote->remoteFile(index.row());
    switch (role) {
    case RemoteFileRole::filename:
        return file.filename;
    case RemoteFileRole::isDir:
        return file.isDir;
    }

    return QVariant();
}

What should I do to make sure the synchronisation is always correct and so the View (or Model) perfectly matches the data?
Shall I lock the Mutex before beginRemoveRows and release it after endRemoveRows in the View Thread?

Would you advise to just update the Model ONLY in the View Thread? This means sending signals with the data received on the Worker Thread and thus making unnecessary temporary copies right?

Edit: if I don't move my ConnectionWorker in another Thread, all those issues disappear and I don't need any Mutexes... But I would prefer to have the network handling made in a Thread to always have the GUI reactive. I suppose that the Mutexes would lock it a bit but all the data is already received, it's just a matter of updating the data structure.

[Christian Ehrlicher]

The easiest solution is to simply pass the data from the worker to the gui thread and do the insertion to the model there. Everything else will lead to problems sooner or later.

[mbruel]

@Christian-Ehrlicher well what's bothering me is that we then need to make at least one copy of the data received by the worker...
The worker receives data on its own structure, it then need to send a copy of it via a signal, and then the GUI thread will copy (or move) this into its own structure. It doesn't look really efficient.
For now, I've moved back my ConnectionWorker in the main Thread and it's working just fine.
As the Sockets are asynchronous, I suppose I could keep this architecture.

Anyway, I'd like to know if it is possible to publish PROPERLY the data of Models in another Thread and if so what are all the steps required. Where do we need Mutex synchronisation on the Model side?

I've seen there are those for private signals in QAbstractItemModel:

    void rowsAboutToBeInserted(const QModelIndex &parent, int first, int last, QPrivateSignal);
    void rowsInserted(const QModelIndex &parent, int first, int last, QPrivateSignal);

    void rowsAboutToBeRemoved(const QModelIndex &parent, int first, int last, QPrivateSignal);
    void rowsRemoved(const QModelIndex &parent, int first, int last, QPrivateSignal);

Would it be a way to listen on them in the View and lock any redraw during the AboutTo signal and the Complete one?

[Christian Ehrlicher]

I don't see why you need to copy your data a second time. Fetch them in the worker, send a signal to notify the model that there is new data, let the model copy the data into the internal structure. Why do you want to pass the data via signals/slots?

Anyway, I'd like to know if it is possible to publish PROPERLY the data of Models in another Thread and if so what are all the steps required. Where do we need Mutex synchronisation on the Model side?

No

[mbruel]

@Christian-Ehrlicher said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

I don't see why you need to copy your data a second time. Fetch them in the worker, send a signal to notify the model that there is new data, let the model copy the data into the internal structure. Why do you want to pass the data via signals/slots?

Hum true, I just need a Mutex in the Worker to lock its internal structure (in case he would receive a new one). I'm going to try. Thanks :)

@Christian-Ehrlicher said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

No

Alright that's a categoric answer! Is it published somewhere in Qt documentation?

[mbruel]

@Christian-Ehrlicher said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

I don't see why you need to copy your data a second time. Fetch them in the worker, send a signal to notify the model that there is new data, let the model copy the data into the internal structure. Why do you want to pass the data via signals/slots?

Well in fact, I'm receiving the data in the Worker from a Socket and filling a QBuffer (cf here) until the data is complete and ready to be processed.
If I'm signalling the main Thread to process the data, it means I need to copy this QBuffer...
It doesn't look that a QBuffer is movable...
Don't know if it has implicit sharing...
So I'd end up making at least 1 copy if I don't want to lock my Connection Thread to let it receive other request.
Am I missing something?

[Christian Ehrlicher]

@mbruel said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

Alright that's a categoric answer! Is it published somewhere in Qt documentation?

Not directly - when you can fix all the issues with the begin/end stuff it may work so my 'No' is my personal opinion from working with the models and views for a long time (and fixing bugs for them within Qt) - it's just not worth the trouble (and I'm sure @VRonin and others will tell you the same).

[mbruel]

@Christian-Ehrlicher
ok cool,
I've updated my code and remove the QBuffer to use directly QByteArray.
In fact no need to move it, as I'm using protobuf (not my choice) and that they are movable.
So I'm doing this from the Worker:

    case pb::remote::LIST_FILES:
#ifdef __USE_CONNECTION_THREAD__
        _secureRemoteFiles.lockForWrite();
        _remoteFilesData = std::move(msg);
        emit remoteFilesUpdatedByWorker();
#else
        rcvListOfRemoteFiles(msg.response_files());
#endif
        break;

and I can just do the model update back in the GUI thread without any copy! \o/

void ClementineRemote::onRemoteFilesUpdatedByWorker()
{
    rcvListOfRemoteFiles(_remoteFilesData.response_files());
    _remoteFilesData.clear_response_files();
    _secureRemoteFiles.unlock();
}

Indeed not worthing the trouble of updating the Model directly in the Worker Thread.
But if it should work nicely, it would be nice to know how ;)

[Christian Ehrlicher]

Nice to see it working with this easy solution. I already wanted suggest you to create the data with new so you only need to copy the pointers.

Updating the model directly from another thread is really hard - it would require a mutex in every function in your model and you must somehow execute the begin/end functions in the main thread.

[mbruel]

@Christian-Ehrlicher said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

Nice to see it working with this easy solution. I already wanted suggest you to create the data with new so you only need to copy the pointers.

yep that wasn't so hard, not sure why I was thinking I would need to send the structure inside the signal...

@Christian-Ehrlicher said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

it would require a mutex in every function in your model

that would be great to have the list. I thought those two would be enough for a read only Model in the View:

    int rowCount(const QModelIndex &parent = QModelIndex()) const override;
    QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;

@Christian-Ehrlicher said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

somehow execute the begin/end functions in the main thread

Well I don't see how to manage that if it's the Working Thread that updates the data...
Doing some synchronisation in order to achieve it would be just less efficient than letting the main thread updating the data...

Any expert in the area to learn how to achieve it? @VRonin maybe?

[Christian Ehrlicher]

@mbruel said in QAbstractListModel populated in a Worker Thread (not the GUI one)?:

Well I don't see how to manage that if it's the Working Thread that updates the data...
Doing some synchronisation in order to achieve it would be just less efficient than letting the main thread updating the data...

Correct, therefore my recommendation - don't try it :)