Memory management for list of QObject* results in "Cannot read property X of null" errors-CodePudding

I need to create dynamic list of QObject* (representin a custom model) and expose them to QML. The problem is that QML tries to re-use previously deleted QObject* which ends up with errors at runtime:

qrc:/MyWidget.qml:6: TypeError: Cannot read property 'value' of null

Here is my model:

#include <QObject>

class SubModel : public QObject
{
    Q_OBJECT
public:
    SubModel(int value) : m_value(value) {}

    Q_PROPERTY(int value READ value WRITE setValue NOTIFY valueChanged)

    int value() { return m_value; }

    void setValue(int value)
    {
        m_value = value;
        emit valueChanged();
    }

signals:
    void valueChanged();

private:
    int m_value;
};

Here is the QAbstractListModel containing the SubModel list (note the createSubModels() method):

#include <QAbstractListModel>
#include <QObject>
#include <QVariant>
#include <memory>
#include <vector>

class ModelList : public QAbstractListModel
{
    Q_OBJECT

public:
    enum ModelRole
    {
        SubModelRole = Qt::UserRole
    };
    Q_ENUM(ModelRole)

    void setSubModels(std::vector<std::unique_ptr<SubModel>> subModels)
    {
        beginResetModel();
        m_subModels = std::move(subModels);
        endResetModel();
    }

    int rowCount(const QModelIndex& parent = QModelIndex()) const override
    {
        return m_subModels.size();
    }

    QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override
    {
        if (!index.isValid()) {
            return {};
        }

        switch (role) {
        case ModelRole::SubModelRole:
            return QVariant::fromValue<SubModel*>(m_subModels[index.row()].get());
        }

        return {};
    }

    bool setData(const QModelIndex& index, const QVariant& value, int role) override
    {
        Q_UNUSED(index);
        Q_UNUSED(value);
        Q_UNUSED(role);
        return false;
    }

    QHash<int, QByteArray> roleNames() const override
    {
        QHash<int, QByteArray> roles;
        roles[ModelRole::SubModelRole] = "submodel";
        return roles;
    }

    Q_INVOKABLE void createSubModels()
    {
        std::vector<std::unique_ptr<SubModel>> subModels;
        for (int i = 0; i < rand() % 5   1; i  ) {
            subModels.push_back(std::make_unique<SubModel>(rand() % 100));
        }
        setSubModels(std::move(subModels));
    }

private:
    std::vector<std::unique_ptr<SubModel>> m_subModels;
};

Here is my QML widget using the SubModel instance (note the typed property):

import QtQuick 2.12
import MyLib.SubModel 1.0

Text {
    property SubModel subModel;
    text: subModel.value
}

Here is the main.qml file:

import QtQuick 2.5
import QtQuick.Window 2.2
import QtQuick.Controls 2.15

Window { 
    visible: true
    
    Column {
        Button {
            text: "Create list"
            onClicked: modelList.createSubModels();
        }

        Column {
            Repeater {
                model: modelList

                //// This does generate errors
                MyWidget {
                    subModel: model.modelData
                }
          
                //// This does generate errors
                // Text {
                //    property var data: modelData
                //    text: data.value
                // }
      
                //// This does NOT generate errors!
                // Text {
                //    text: modelData.value;
                // }
            }
        }
    }
}

And finally, here is the main.cpp file:

#include <QGuiApplication>
#include <QQmlApplicationEngine>
#include <QQmlContext>
#include <iostream>

#include "models.h"

int main(int argc, char* argv[])
{
    qmlRegisterUncreatableType<SubModel>(
        "MyLib.SubModel", 1, 0, "SubModel", "This type can't be created in QML");

    auto modelList = std::make_unique<ModelList>();

    QGuiApplication app(argc, argv);

    QQmlApplicationEngine engine;

    QQmlContext* rootContext = engine.rootContext();
    rootContext->setContextProperty("modelList", modelList.get());

    engine.load(QUrl(QStringLiteral("qrc:/main.qml")));

    return app.exec();
}

The firs time the "Create list" button is clicked it works fine. But the second time, the initial SubModule pointers are deleted and QML tries to access value attribute of nullptr, resulting in warning. Then m_subModels is replaced and the GUI is effectively updated. I don't understand why QML tries to access model while I emitted the beginResetModel() signal.

I know that I could change ModelList and make data() returns the actual int value instead of a SubModel pointer but this is not acceptable because I actually have several submodels in real code (separation of concerns).

Solutions I thought of:

Checking for if submodel !== null in QML but this doesn't look good
Replacing std::unique_ptr with new but this creates a memory leak
Using raw pointer with a Qt parent but memory will never be freed until exit
Using QSharedPointer with deleteLater() in destructor but this does not work all the time
Using QSharedPointer with setObjectOwnership(JavaScriptOwnership) in destructor but this seems hacky (not tested)
Using a QList<QObject*> instead of QAbstractListModel but this generated the same errors
Using a QQmlListProperty instead of QAbstractListModel but this generated the same errors
Re-using existing SubModel* instead of re-creating the list but this seems very convoluted

I have spent several days on this problem and I can't find a suitable solution. Would you have an idea please to keep my "SubModel" as it is and avoid QML warnings when the list is re-created?

I uploaded all project files here to easily reproduce the problem (using cmake for building).

Edit: The problem seems related to my usage of property because if I do not store the modelData as a property then there is no error.

CodePudding user response：

Your code seems to be working fine. That's how Repeater works.

When your model is reset, Repeater starts to remove its QQuickItems one by one in reverse order.

// qtdeclarative/src/quick/items/qquickrepeater.cpp
if (d->model) {
    // We remove in reverse order deliberately; so that signals are emitted
    // with sensible indices.
    for (int i = d->deletables.count() - 1; i >= 0; --i) {
        if (QQuickItem *item = d->deletables.at(i)) {
            if (complete)
                emit itemRemoved(i, item);
            d->model->release(item);
        }
    }
    for (QQuickItem *item : qAsConst(d->deletables)) {
        if (item)
            item->setParentItem(nullptr);
    }
}

Depending on Repeater's strategy items may not be removed completely, but pooled for the future use.

At that very moment Item itself still exists, but you model data is not.

 Text {
     property var data: modelData // modelData is undefined at the moment
     text: data.value // "data" is undefined. You see the warning message 
 }

So, best you can do to fix warning messages is to check if modelData is defined.

 Text {
     property var data: modelData
     text: data ? data.value : ""
 }

CodePudding user response：

For reasons explained by @samdavydov the Items in the Repeater stay during a model reset, but by unreferencing m_subModels the SubModels are deleted (because of the unique_ptr) and the views are invalidated because of the destroyed signal as you mention.

By swapping the two vectors during the reset, the old SubModels will stay a bit in memory, until the function exits, at which point the reset has already been completed and the new SubModels are being used:

void setSubModels(std::vector<std::unique_ptr<SubModel>> subModels)
{
   beginResetModel();
   m_subModels.swap(subModels);
   endResetModel();
} //RAII deletes old SubModel at this point

With credit to yourself for finding the swap after my mention of keeping them in memory ;-)