QML object access through model crashes

JoeCFD

@kshegunov Ok. Qt has its own garbage collection mechanism. Therefore, any derived classes from Qt classes may not need any shared or unique pointers unless they are needed to be deleted immediately for example inside one func call(sometimes I do it). I guess in most cases, raw pointers are good enough while Qt will take care of memory release.
In his case, the pointer is used(returned). I guess a raw pointer is good enough.

GrecKo

@JoeCFD the advantage of using an owning smart pointer in a container is that when removing an element from a container, the pointed-to object will be automatically deleted.

@mzimmers the question is now why are you using this getOutcome() function at all in your model?

How do you expose the uuid in the first place? Do you get it from your OutcomeList? Can't you directly add a role returning an Outcome*?

JoeCFD

@GrecKo That is ok. This case is same as I do in a func call. But, I do not think his usage of shared or unique pointer in this func is correct.

raw pointers will work for mzimmers as well. He got confused with shared or unique pointers.

mzimmers

@GrecKo I'll do my best to explain this briefly yet clearly.

My app has several models. One model is the SpaceModel. The SpaceModel contains a list of Space objects. Each Space contains a list of Outcome UUIDs, which reference items in the OutcomeModel.

I did this because both Spaces and Outcomes are distinct, independent entities in my system (neither "owns" the other), and this was the best I could think of.

So, the getOutcome() is to give the Space objects visibility into the Outcome properties.

I have no doubt there's a better way to do this, but this was the best I could come up with.

GrecKo

@mzimmers alright, that seems sensible enough.

@JoeCFD said in QML object access through model crashes:

@GrecKo That is ok. This case is same as I do in a func call. But, I do not think his usage of shared or unique pointer in this func is correct.
raw pointers will work for mzimmers as well. He got confused with shared or unique pointers.

I didn't understand what you mean.
If the OutcomeModel class actually owns the Outcome instances, using a container with owning smart pointers looks correct to me.

mzimmers

Hi all -

Sorry, but I have to re-open this; problem still exists after all.

Summary (as brief as I can make it):

I have a model for a class I've defined:

class Outcome : public QObject
{
    Q_OBJECT
    QML_ELEMENT

typedef std::shared_ptr<Outcome> OutcomePtr;
typedef QList<OutcomePtr> OutcomeList;

class OutcomeModel : public QAbstractListModel
{
    Q_OBJECT
    QML_ELEMENT

    Outcome *getOutcome(const QUuid &uuid)
    {
        const auto i { getIndex(uuid) };
        if (i == NgaUI::NOT_IN_LIST) {
            return nullptr;
        } else {
            return m_list.at(i).get();
        }
    }

These outcome objects have a N:N relation with another class Space. I maintain the relations in a list in my Space class:

class Space : public QObject {
    Q_OBJECT
    QML_ELEMENT
    QList<QUuid> m_outcomeList;
    Q_PROPERTY(QList<QUuid> outcomeList READ outcomeList WRITE setOutcomeList NOTIFY outcomeListChanged FINAL)
    // all the stuff for above Q_PROPERTY is defined and seems to work.

The size of m_outcomeList can be from 0 to arbitrarily large. (I also have a Space model.)

In QML, I have a Space detail card with the following:

Pane {
    id: spaceDetail
    property Space space
    property var outcomeList: space.outcomeList
    ListView {
        model: spaceDetail.outcomeList
        delegate: outcomeComponent
        Component {
            id: outcomeComponent
            RowLayout {
                property Outcome outcome: outcomeModel.getOutcome(modelData)
                Label {
                    text: outcome !== null ? outcome.name : "no name available"
                }
                // and so on.

When I navigate to a space detail for a space that has a non-0 outcome list, everything works fine. When I then navigate to another space detail, two bad things happen:

the Outcome destructor is invoked for a valid outcome (should not be invoked)
the program crashes after a few seconds. The stack pointer points to stuff within my destructor (which does nothing other than a qDebug().

I have tried this with smart pointers and raw pointers, with the same result.

I know this is a lot to digest, but...can anyone see ANYTHING wrong with what I'm doing?

GrecKo

@mzimmers it appears you have an ownership problem.

Who owns the std::shared_ptr<Outcome> in OutcomeModel and who are they shared with? Do they have a QObject parent?

mzimmers

@GrecKo from my updateModel() function:

std::shared_ptr<Outcome> pOutcome = nullptr;

// determine whether the outcome in this message already exists in list.
const auto listIndex { getIndex(uuid) };
if (listIndex == NgaUI::NOT_IN_LIST) { // will append to list below.
    pOutcome = std::make_shared<Outcome>(this);
} else {
    pOutcome = m_list.at(listIndex);
}
...
if (listIndex == NgaUI::NOT_IN_LIST) {
    beginInsertRows(QModelIndex(), m_list.size(),  m_list.size());
    m_list.append(pOutcome);
    endInsertRows();

EDIT:

Regarding who they're shared with, I do create an instance in QML, using my getOutcome() function, but this works with a raw pointer, so I don't think it's sharing the smart pointer, right?

GrecKo

@mzimmers Does the Outcome constructor set a QObject parent? By calling the base constructor QObject(parent) or with an explicit call to QObject::setParent.

mzimmers

@GrecKo it does now.

And that seems to have fixed the problem.

Before your post:

Outcome::Outcome(QObject *parent)
{}

now:

Outcome::Outcome(QObject *parent) : QObject(parent)
{}

I'm no longer hitting the destructor, and no crashes.

@GrecKo I have no idea how you thought of that, but thank you very much.

EDIT: I'm still doing something not quite right -- now, when I close my application (using the "X" in the window title bar), I then get the destroyed messages, and my program reports as crashed after a few seconds. This I can live with, though I'd like to find the cause.