QGraphicsSceneFindItemBspTreeVisitor::visit crashes due to an obsolete PaintEvent after QGraphicsScene::removeItem

[SGaist]

Hi,

Is your application multithreaded ? If so you could be altering _allItems in removeElement while you are in your foreach loop.

[mbruel]

@SGaist
Hi,
no it's a single thread app.
I've managed to find out, what could be the cause, in my LinkItem::boundingRect, the item->centerPoint is call sceneBoundingRect()->center()
It seems that doing a call to the scene will cause some paint events that will crash later...
I don't understand why...
Normally the items are removed from the scene and from my set _allItems, so I shouldn't call it anymore...
I'm really struggling to understand the issue...
I'm thinking to do a simple basic application to try to reproduce the crash and post it here to show you this weird behaviour...
PS: I'm using QT 5.5.1

[josephdp]

From what I view it, it's like a linked list with each item having links to the next item or if a doubly linked list, a link to the previous and next items.

For me, I think you can have it simpler. When an element is deleted, you should also delete the links. The element should have link properties and not as separate. So that when you delete the element, it will also delete the link (or links) with it.

Thus, in conclusion, an element has links and a polygon contains elements that has links.
To remove an element: first remove links, then remove the element then recalculate.

I hope this makes sense. My suggestion is focusing on the design hierarchy.

[mbruel]

@josephdp
Hello Joseph,
thanks for your reply but in term of design this doesn't work.
My Link has a life in itself. I don't want to delete it when I remove one Element because it is still linking other Elements so still makes sense.
Even when there are no more Elements, I still want it in my scene cause when the user drag back an Elements in the scene they will connect to the Link.
Basically, my link is like a bus on which Equipments can be connected.
I'll try to finish tomorrow a basic example application of my use case and I'll share it so you can see the design and let me know what I'm doing wrong and also if you would do an easier/cleaner way.

[mbruel]

Alright, I've done a little example that illustrate what my software is doing.
It's not so long but as there are several files and so you could just compile it and run it if you'd like, I've posted it on github.
You can find it here: https://github.com/mbruel/testSceneMgr/
The crash happens (not always but often) when I try to delete my LinkElement and so the LinkNMItem associated.
There are no issues when deleting the regular Elements...
Can you explain me what is the difference between both (Elements and Links) and why I'm getting this crash....
Also feel free to let me know if there is a better way to achieve what I'm trying to do.
Thanks in advance.

I've put a small record of the execution of the app on my laptop, you can find it here: https://sendvid.com/5fugajs8
The crash happens with a little delay... but nearly all the time when I delete the Link after having deleted all the Elements.

basically I'm adding/removing Elements to the scene like this:

void MainWindow::addElement(const QPointF &pos, const QSize &size)
{
    Element *elem = new Element(pos, size);
    _scene->addItem(elem->createGraphicsItem());
    _elements.insert(elem);
}

void MainWindow::removeElement(Element *elem)
{

    if (_elements.remove(elem) || _links.remove(elem))
    {
        _scene->removeItem(elem->item());
        _scene->update();
        delete elem;
    }
}

Making my Element a QObject and using deleteLater to postpone the deletion didn't change anything except having the crash few seconds later.

Here is my Element and LinkElement

class Element
{
public:
    Element(const QPointF & pos = QPointF(0,0), const QSize & size = QSize(50,50)):
    _scenePos(pos), _size(size){}

    virtual ~Element();

    virtual QGraphicsItem *createGraphicsItem();
    void deleteItem(){delete _item;_item = Q_NULLPTR;}

    void setPos(const QPointF& pos){_scenePos = pos;}
    QGraphicsItem *item() {return _item;}

    void addLink(LinkElement *link) {_links.insert(link);}
    void removeLink(LinkElement *link){_links.remove(link);}

    void showMenu();

    QPointF pos()  const {return _scenePos;}
    QSize   size() const {return _size;}
protected:
    QPointF _scenePos;
    QSize   _size;
    QColor  _color;

    QGraphicsItem      *_item;
    QSet<LinkElement *> _links;
};

class LinkElement : public Element
{
public:
    LinkElement(const QSet<Element*> &elems, const QPointF & pos = QPointF(0,0)):
        Element(pos), _elements(elems){}
    ~LinkElement();

    QGraphicsItem *createGraphicsItem();

    void removeElement(Element* elem);

private:
    QSet<Element*> _elements;
};

Element::~Element()
{
    for (LinkElement * link : _links)
    {
        link->removeElement(this);
    }
    delete _item;
}

QGraphicsItem *Element::createGraphicsItem()
{
    _item = new ComplexItem(this);
    _item->setFlag(QGraphicsItem::ItemIsMovable, true);
    _item->setFlag(QGraphicsItem::ItemIsSelectable, true);
    return _item;
}

void Element::showMenu()
{
    QMenu menu;
    QAction *qAction = menu.addAction("Delete");
    qAction->setEnabled(true);
    DeleteElementAction *delAction = new DeleteElementAction(this);
    QObject::connect(qAction, SIGNAL(triggered()), delAction, SLOT(remove()));

    menu.exec(QCursor::pos());
}

LinkElement::~LinkElement()
{
    for (Element *elem : _elements)
        elem->removeLink(this);
}

QGraphicsItem *LinkElement::createGraphicsItem()
{
    QSet<QGraphicsItem *> items;
    for (Element *elem : _elements)
    {
        elem->addLink(this);
        items.insert(elem->item());
    }
    _item = new LinkNMItem(this, _scenePos, items);
    _item->setFlag(QGraphicsItem::ItemIsMovable, true);
    _item->setFlag(QGraphicsItem::ItemIsSelectable, true);
    return _item;
}

void LinkElement::removeElement(Element *elem)
{
    static_cast<LinkNMItem*>(_item)->removeItem(elem->item());
    _elements.remove(elem);
}

The DeleteElementAction:

class DeleteElementAction : public QObject
{
    Q_OBJECT

public:
    DeleteElementAction(Element *elem):_element(elem){}

public slots:
    void remove() {MainWindow::getInstance()->removeElement(_element);deleteLater();}

private:
    Element *_element;
};

and my graphics Items:

class GraphicItem
{
public:
    GraphicItem(Element *elem):_element(elem){}

protected:
    Element *_element;
};

class ComplexItem : public QGraphicsItem, public GraphicItem
{
public:
    ComplexItem(Element *elem);

    // QGraphicsItem interface
public:
    QRectF boundingRect() const;
    void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget);

    QPointF centerPoint() const;

    // QGraphicsItem interface
protected:
    virtual void mousePressEvent(QGraphicsSceneMouseEvent *event);
    virtual void mouseMoveEvent(QGraphicsSceneMouseEvent *event);
    virtual void mouseReleaseEvent(QGraphicsSceneMouseEvent *event);
    virtual void contextMenuEvent(QGraphicsSceneContextMenuEvent *event);
};

class LinkItem : public QGraphicsItem, public GraphicItem
{
public:
    LinkItem(Element *elem, const QPointF &movePoint);


    // QGraphicsItem interface
protected:
    virtual void mousePressEvent(QGraphicsSceneMouseEvent *event);
    virtual void mouseMoveEvent(QGraphicsSceneMouseEvent *event);
    virtual void mouseReleaseEvent(QGraphicsSceneMouseEvent *event);
    virtual void contextMenuEvent(QGraphicsSceneContextMenuEvent *event);


protected:
    QPointF _movablePoint;
    QRectF  _movableRect;
    bool    _isMoving;

    static const ushort SHAPE_MOVABLE_RECT = 10;
};

class LinkNMItem : public LinkItem
{
public:
    LinkNMItem(Element *link, const QPointF &movePoint, const QSet<QGraphicsItem*> &items):
        LinkItem(link, movePoint), _items(items){}

    void removeItem(QGraphicsItem *item);

    // QGraphicsItem interface
public:
    QRectF boundingRect() const;
    void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget);

private:
    QSet<QGraphicsItem*> _items;
};


QRectF ComplexItem::boundingRect() const
{
    return QRectF(QPointF(0,0), _element->size());
}

void ComplexItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
{
    painter->setPen(QPen(Qt::green, 2));
    painter->drawRect(boundingRect());
}

void ComplexItem::contextMenuEvent(QGraphicsSceneContextMenuEvent *event)
{
    if (_element)
    {
        _element->showMenu();
    }
    event->accept();
}


void LinkItem::mousePressEvent(QGraphicsSceneMouseEvent *event)
{
    QGraphicsItem::mousePressEvent(event);
    QPointF pos = event->scenePos();
    if (_movableRect.contains(pos))
        _isMoving = true;
}

void LinkItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
{
    if (_isMoving)
    {
        _movablePoint = event->scenePos();
        update();
    }
}

void LinkItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
{
    if (_isMoving)
    {
        QGraphicsItem::mouseReleaseEvent(event);
        _movablePoint = event->scenePos();
        _isMoving = false;
        _element->setPos(_movablePoint);
        prepareGeometryChange();
    }
}

void LinkItem::contextMenuEvent(QGraphicsSceneContextMenuEvent *event)
{
    if (_element)
    {
        _element->showMenu();
    }
    event->accept();
}

QRectF LinkNMItem::boundingRect() const
{
    QPolygonF polygon;
    polygon << _movablePoint;
    for (QGraphicsItem * item : _items)
        polygon << item->sceneBoundingRect().center();

    return polygon.boundingRect().adjusted(-10,-10,10,10);
}

void LinkNMItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
{
    _movableRect = QRectF(_movablePoint, QSize(0,0));
    _movableRect.adjust(-5,-5,5,5);
    painter->setPen(QPen(Qt::red, 2));
    painter->drawEllipse(_movableRect);

    painter->setPen(QPen(Qt::black,2));
    for (QGraphicsItem * item : _items)
        painter->drawLine(_movablePoint, item->sceneBoundingRect().center());

//    painter->setPen(QPen(Qt::blue,2));
//    painter->drawRect(boundingRect());
}

I let you check the rest of the code on github if needed
PS: when I comment out in LinkNMItem::boundingRect()

    for (QGraphicsItem * item : _items)
        polygon << item->sceneBoundingRect().center();

The crash is still happening but not always and takes more time

[mbruel]

I installed QT 5.7 on another computer and got a full stack in Debug for the crash. Here it is:

Thread 1 (Thread 6812.0x1a94):
#0  0x0d12d6e8 in QGraphicsSceneFindItemBspTreeVisitor::visit (this=0x178e0548, items=0x17931358) at graphicsview\qgraphicsscene_bsp.cpp:77
        item = 0x178e1608
        i = 0
#1  0x0d0978bd in QGraphicsSceneBspTree::climbTree (this=0x178e0460, visitor=0x178e0548, rect=..., index=31) at graphicsview\qgraphicsscene_bsp.cpp:245
        node = @0x17931ac0: {{offset = -43.375, leafIndex = 0}, type = QGraphicsSceneBspTree::Node::Leaf}
        childIndex = 63
#2  0x0d0979c6 in QGraphicsSceneBspTree::climbTree (this=0x178e0460, visitor=0x178e0548, rect=..., index=15) at graphicsview\qgraphicsscene_bsp.cpp:259
        node = @0x179319c0: {{offset = 11.75, leafIndex = 0}, type = QGraphicsSceneBspTree::Node::Horizontal}
        childIndex = 31
#3  0x0d097907 in QGraphicsSceneBspTree::climbTree (this=0x178e0460, visitor=0x178e0548, rect=..., index=7) at graphicsview\qgraphicsscene_bsp.cpp:250
        node = @0x17931940: {{offset = -15.75, leafIndex = 0}, type = QGraphicsSceneBspTree::Node::Vertical}
        childIndex = 15
#4  0x0d0979c6 in QGraphicsSceneBspTree::climbTree (this=0x178e0460, visitor=0x178e0548, rect=..., index=3) at graphicsview\qgraphicsscene_bsp.cpp:259
        node = @0x17931900: {{offset = 33.5, leafIndex = 0}, type = QGraphicsSceneBspTree::Node::Horizontal}
        childIndex = 7
#5  0x0d097907 in QGraphicsSceneBspTree::climbTree (this=0x178e0460, visitor=0x178e0548, rect=..., index=1) at graphicsview\qgraphicsscene_bsp.cpp:250
        node = @0x179318e0: {{offset = 39.5, leafIndex = 0}, type = QGraphicsSceneBspTree::Node::Vertical}
        childIndex = 3
#6  0x0d0979c6 in QGraphicsSceneBspTree::climbTree (this=0x178e0460, visitor=0x178e0548, rect=..., index=0) at graphicsview\qgraphicsscene_bsp.cpp:259
        node = @0x179318d0: {{offset = 39.5, leafIndex = 0}, type = QGraphicsSceneBspTree::Node::Horizontal}
        childIndex = 1
#7  0x0d096dcc in QGraphicsSceneBspTree::items (this=0x178e0460, rect=..., onlyTopLevelItems=true) at graphicsview\qgraphicsscene_bsp.cpp:152
        tmp = {<QListSpecialMethods<QGraphicsItem*>> = {<No data fields>}, {p = {static shared_null = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, alloc = 0, begin = 0, end = 0, array = {0x0}}, d = 0x6bcccbac <QListData::shared_null>}, d = 0x6bcccbac <QListData::shared_null>}}
#8  0x0d099023 in QGraphicsSceneBspTreeIndexPrivate::estimateItems (this=0x178e0418, rect=..., order=Qt::AscendingOrder, onlyTopLevelItems=true) at graphicsview\qgraphicsscenebsptreeindex.cpp:389
        q = 0x178e03f8
        rectItems = {<QListSpecialMethods<QGraphicsItem*>> = {<No data fields>}, {p = {static shared_null = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, alloc = 0, begin = 0, end = 0, array = {0x0}}, d = 0x6bcccbac <QListData::shared_null>}, d = 0x6bcccbac <QListData::shared_null>}}
#9  0x0d099804 in QGraphicsSceneBspTreeIndex::estimateTopLevelItems (this=0x178e03f8, rect=..., order=Qt::AscendingOrder) at graphicsview\qgraphicsscenebsptreeindex.cpp:543
        d = 0x178e0418
#10 0x0d08e0fa in QGraphicsScenePrivate::drawItems (this=0x178e01e8, painter=0x28b794, viewTransform=0x28b740, exposedRegion=0x178b3d6c, widget=0x178c7480) at graphicsview\qgraphicsscene.cpp:4712
        exposedSceneRect = {xp = -156, yp = -39, w = 392, h = 233}
        tli = {<QListSpecialMethods<QGraphicsItem*>> = {<No data fields>}, {p = {static shared_null = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, alloc = 0, begin = 0, end = 0, array = {0x0}}, d = 0xffff}, d = 0xffff}}
#11 0x0d0aaf41 in QGraphicsView::paintEvent (this=0x178b3990, event=0x28c368) at graphicsview\qgraphicsview.cpp:3550
        oldRectAdjust = 2
        d = 0x178b39c0
        exposedSceneRect = {xp = -155, yp = -38, w = 390, h = 231}
        painter = {static staticMetaObject = {d = {superdata = 0x0, stringdata = 0x11204c0 <qt_meta_stringdata_QPainter>, data = 0x1120600 <qt_meta_data_QPainter>, static_metacall = 0x0, relatedMetaObjects = 0x0, extradata = 0x0}}, d_ptr = {d = 0x17931818}}
        viewTransformed = true
        viewTransform = {affine = {_m11 = 1, _m12 = 0, _m21 = 0, _m22 = 1, _dx = 155, _dy = 38}, m_13 = 0, m_23 = 0, m_33 = 1, m_type = 1, m_dirty = 0, d = 0x0}
#12 0x0cdc73fd in QWidget::event (this=0x178b3990, event=0x28c368) at kernel\qwidget.cpp:8930
        d = 0x178b39c0
#13 0x0cedb354 in QFrame::event (this=0x178b3990, e=0x28c368) at widgets\qframe.cpp:550
        result = 13
#14 0x0cf60add in QAbstractScrollArea::viewportEvent (this=0x178b3990, e=0x28c368) at widgets\qabstractscrollarea.cpp:1213
No locals.
#15 0x0d0a8a02 in QGraphicsView::viewportEvent (this=0x178b3990, event=0x28c368) at graphicsview\qgraphicsview.cpp:2973
        d = 0x178b39c0
#16 0x0d12977c in QAbstractScrollAreaPrivate::viewportEvent (this=0x178b39c0, event=0x28c368) at widgets/qabstractscrollarea_p.h:111
No locals.
#17 0x0d12859d in QAbstractScrollAreaFilter::eventFilter (this=0x178c5cd0, o=0x178c7480, e=0x28c368) at widgets/qabstractscrollarea_p.h:127
No locals.
#18 0x6badc936 in QCoreApplicationPrivate::sendThroughObjectEventFilters (receiver=0x178c7480, event=0x28c368) at kernel\qcoreapplication.cpp:1099
        obj = 0x178c5cd0
        i = 0
        __PRETTY_FUNCTION__ = "static bool QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*)"
#19 0x0cd8fb50 in QApplicationPrivate::notify_helper (this=0x3b8038, receiver=0x178c7480, e=0x28c368) at kernel\qapplication.cpp:3795
        consumed = false
#20 0x0cd8f9ad in QApplication::notify (this=0x28fe44, receiver=0x178c7480, e=0x28c368) at kernel\qapplication.cpp:3762
        d = 0x3b8038
        __PRETTY_FUNCTION__ = "virtual bool QApplication::notify(QObject*, QEvent*)"
        res = false
#21 0x6badc60f in QCoreApplication::notifyInternal2 (receiver=0x178c7480, event=0x28c368) at kernel\qcoreapplication.cpp:988
        selfRequired = true
        result = false
        cbdata = {0x178c7480, 0x28c368, 0x28c303}
        d = 0x178c7568
        threadData = 0x3b8140
        scopeLevelCounter = {threadData = 0x3b8140}
#22 0x0d10b3d5 in QCoreApplication::sendSpontaneousEvent (receiver=0x178c7480, event=0x28c368) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:234
No locals.
#23 0x0cdbee81 in QWidgetPrivate::sendPaintEvent (this=0x178c7568, toBePainted=...) at kernel\qwidget.cpp:5696
        q = 0x178c7480
        e = {<QEvent> = {_vptr.QEvent = 0x1150364 <vtable for QPaintEvent+8>, static staticMetaObject = {d = {superdata = 0x0, stringdata = 0x6be63100 <qt_meta_stringdata_QEvent>, data = 0x6be645e0 <qt_meta_data_QEvent>, static_metacall = 0x0, relatedMetaObjects = 0x0, extradata = 0x0}}, d = 0x0, t = 12, posted = 0, spont = 1, m_accept = 1, reserved = 5}, m_rect = {x1 = 0, y1 = 0, x2 = 389, y2 = 230}, m_region = {d = 0x17931578, static shared_empty = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, qt_rgn = 0x129b980 <qrp>}}, m_erased = false}
#24 0x0cdbea16 in QWidgetPrivate::drawWidget (this=0x178c7568, pdev=0x178dccc4, rgn=..., offset=..., flags=36, sharedPainter=0x0, backingStore=0x178daf48) at kernel\qwidget.cpp:5636
        flushed = false
        paintEngine = 0x178ca898
        skipPaintEvent = false
        asRoot = false
        onScreen = false
        q = 0x178c7480
        alsoOnScreen = false
        recursive = true
        alsoInvisible = false
        toBePainted = {d = 0x17931578, static shared_empty = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, qt_rgn = 0x129b980 <qrp>}}
        __PRETTY_FUNCTION__ = "void QWidgetPrivate::drawWidget(QPaintDevice*, const QRegion&, const QPoint&, int, QPainter*, QWidgetBackingStore*)"
#25 0x0cd978e3 in QWidgetBackingStore::doSync (this=0x178daf48) at kernel\qwidgetbackingstore.cpp:1356
        w = 0x178c7480
        flags = 36
        toBePainted = {d = 0x17931578, static shared_empty = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, qt_rgn = 0x129b980 <qrp>}}
        wd = 0x178c7568
        offset = {xp = 10, yp = 24}
        i = 0
        updatesDisabled = false
        repaintAllWidgets = false
        inTopLevelResize = false
        tlwRect = {x1 = 308, y1 = 277, x2 = 715, y2 = 585}
        surfaceGeometry = {x1 = 308, y1 = 277, x2 = 715, y2 = 585}
        toClean = {d = 0x178e0800, static shared_empty = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, qt_rgn = 0x129b980 <qrp>}}
        opaqueNonOverlappedWidgets = {a = 32, s = 1, ptr = 0x28c640, {array = "\200t?\027\000\000;\000\000\000\000\000Ú\230\023u\200aÌÐðÇ(\000À9?\027ðÇ(\000\000\000\000\000Ä\200ùk\230Æ(\000\004ÿÆkPÆ(\000µ3ïv\004Þ(\000Õ?\025uè?÷¥þÿÿÿÚ\230\023u\022Ù\022\r@¼\215\027@\000\000\000¸Æ(\000ù¿\230k\000\000\000\000À;?\027¸Æ(\000@¼\215\027\000\000\000\000øÆ(\000ÈÆ(\000\003\032\022\r", q_for_alignment_1 = 16607024021009536, q_for_alignment_2 = 1.5019249325707075e-307}}
        tlwExtra = 0x178e0148
        beginPaintInfo = {wasFlushed = 0, nothingToPaint = 0, backingStoreRecreated = 0}
        dirtyCopy = {d = 0x10f32bc <QRegion::shared_empty>, static shared_empty = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, qt_rgn = 0x129b980 <qrp>}}
#26 0x0cd967d1 in QWidgetBackingStore::sync (this=0x178daf48) at kernel\qwidgetbackingstore.cpp:1151
        tlwExtra = 0x178e0148
#27 0x0cdb5c58 in QWidgetPrivate::syncBackingStore (this=0x178b2d30) at kernel\qwidget.cpp:1955
        bs = 0x178daf48
#28 0x0cdc7a39 in QWidget::event (this=0x178b2cf0, event=0x178e1270) at kernel\qwidget.cpp:9083
        d = 0x178b2d30
#29 0x0cef4fba in QMainWindow::event (this=0x178b2cf0, event=0x178e1270) at widgets\qmainwindow.cpp:1543
        d = 0x178b2d30
#30 0x0cd8fb70 in QApplicationPrivate::notify_helper (this=0x3b8038, receiver=0x178b2cf0, e=0x178e1270) at kernel\qapplication.cpp:3799
        consumed = 23
#31 0x0cd8f9ad in QApplication::notify (this=0x28fe44, receiver=0x178b2cf0, e=0x178e1270) at kernel\qapplication.cpp:3762
        d = 0x3b8038
        __PRETTY_FUNCTION__ = "virtual bool QApplication::notify(QObject*, QEvent*)"
        res = false
#32 0x6badc60f in QCoreApplication::notifyInternal2 (receiver=0x178b2cf0, event=0x178e1270) at kernel\qcoreapplication.cpp:988
        selfRequired = true
        result = false
        cbdata = {0x178b2cf0, 0x178e1270, 0x28d083}
        d = 0x178b2d30
        threadData = 0x3b8140
        scopeLevelCounter = {threadData = 0x3b8140}
#33 0x6bbe3a1f in QCoreApplication::sendEvent (receiver=0x178b2cf0, event=0x178e1270) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:231
No locals.
#34 0x6badd85b in QCoreApplicationPrivate::sendPostedEvents (receiver=0x178b2cf0, event_type=77, data=0x3b8140) at kernel\qcoreapplication.cpp:1649
        e = 0x178e1270
        pe = @0x17930924: {receiver = 0x178b2cf0, event = 0x0, priority = -1}
        r = 0x178b2cf0
        unlocker = {m = @0x28d0fc}
        event_deleter = {d = 0x178e1270}
        __PRETTY_FUNCTION__ = "static void QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*)"
        locker = {val = 3899744}
        startOffset = 6
        i = @0x28d0f8: 6
        cleanup = {receiver = 0x178b2cf0, event_type = 77, data = 0x3b8140, exceptionCaught = true}
#35 0x6badd31a in QCoreApplication::sendPostedEvents (receiver=0x178b2cf0, event_type=77) at kernel\qcoreapplication.cpp:1503
        data = 0x3b8140
#36 0x0d11e9bc in QGraphicsViewPrivate::dispatchPendingUpdateRequests (this=0x178b39c0) at c:/Users/qt/work/qt/qtbase/src/widgets/graphicsview/qgraphicsview_p.h:202
No locals.
#37 0x0d080a92 in QGraphicsScenePrivate::_q_emitUpdated (this=0x178e01e8) at graphicsview\qgraphicsscene.cpp:383
        i = 0
        q = 0x178e01c8
        oldUpdatedRects = {<QListSpecialMethods<QRectF>> = {<No data fields>}, {p = {static shared_null = {ref = {atomic = {_q_value = {<std::__atomic_base<int>> = {static _S_alignment = 4, _M_i = -1}, <No data fields>}}}, alloc = 0, begin = 0, end = 0, array = {0x0}}, d = 0xd081158 <QGraphicsScenePrivate::_q_processDirtyItems()+112>}, d = 0xd081158 <QGraphicsScenePrivate::_q_processDirtyItems()+112>}}
#38 0x0d0961eb in QGraphicsScene::qt_static_metacall (_o=0x178e01c8, _c=QMetaObject::InvokeMetaMethod, _id=13, _a=0x17939468) at .moc/debug/moc_qgraphicsscene.cpp:180
        _t = 0x178e01c8
#39 0x6bafebe6 in QMetaCallEvent::placeMetaCall (this=0x178db998, object=0x178e01c8) at kernel\qobject.cpp:502
No locals.
#40 0x6baff983 in QObject::event (this=0x178e01c8, e=0x178db998) at kernel\qobject.cpp:1263
        mce = 0x178db998
        sw = {receiver = 0x178e01c8, previousSender = 0x0, currentSender = {sender = 0x0, signal = -1, ref = 1}, switched = true}
#41 0x0d089ef9 in QGraphicsScene::event (this=0x178e01c8, event=0x178db998) at graphicsview\qgraphicsscene.cpp:3522
        d = 0x178e01e8
#42 0x0cd8fb70 in QApplicationPrivate::notify_helper (this=0x3b8038, receiver=0x178e01c8, e=0x178db998) at kernel\qapplication.cpp:3799
        consumed = true
#43 0x0cd8d143 in QApplication::notify (this=0x28fe44, receiver=0x178e01c8, e=0x178db998) at kernel\qapplication.cpp:3159
        d = 0x3b8038
        __PRETTY_FUNCTION__ = "virtual bool QApplication::notify(QObject*, QEvent*)"
        res = false
#44 0x6badc60f in QCoreApplication::notifyInternal2 (receiver=0x178e01c8, event=0x178db998) at kernel\qcoreapplication.cpp:988
        selfRequired = true
        result = false
        cbdata = {0x178e01c8, 0x178db998, 0x28db83}
        d = 0x178e01e8
        threadData = 0x3b8140
        scopeLevelCounter = {threadData = 0x3b8140}
#45 0x6bbe3a1f in QCoreApplication::sendEvent (receiver=0x178e01c8, event=0x178db998) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:231
No locals.
#46 0x6badd85b in QCoreApplicationPrivate::sendPostedEvents (receiver=0x0, event_type=0, data=0x3b8140) at kernel\qcoreapplication.cpp:1649
        e = 0x178db998
        pe = @0x1793090c: {receiver = 0x178e01c8, event = 0x0, priority = 0}
        r = 0x178e01c8
        unlocker = {m = @0x28dbfc}
        event_deleter = {d = 0x178db998}
        __PRETTY_FUNCTION__ = "static void QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*)"
        locker = {val = 3899744}
        startOffset = 0
        i = @0x3b8158: 4
        cleanup = {receiver = 0x0, event_type = 0, data = 0x3b8140, exceptionCaught = true}
#47 0x6bb2bb56 in QEventDispatcherWin32::sendPostedEvents (this=0x3b90b0) at kernel\qeventdispatcher_win.cpp:1294
        d = 0x178b1240
#48 0x62a94fd9 in QWindowsGuiEventDispatcher::sendPostedEvents (this=0x3b90b0) at eventdispatchers\qwindowsguieventdispatcher.cpp:81
No locals.
#49 0x6bb28e12 in qt_internal_proc(HWND__*, unsigned int, unsigned int, long)@16 (hwnd=0xe04d0, message=1025, wp=0, lp=0) at kernel\qeventdispatcher_win.cpp:443
        localSerialNumber = 1470
        msg = {hwnd = 0xe04d0, message = 1025, wParam = 0, lParam = 0, time = 2678088, pt = {x = 1806864639, y = 0}}
        dispatcher = 0x3b90b0
        result = 0
        q = 0x3b90b0
        d = 0x178b1240
#50 0x752e62fa in gapfnScSendMessage () from C:\Windows\syswow64\user32.dll
No symbol table info available.
#51 0x000e04d0 in ?? ()
No symbol table info available.
#52 0x00000401 in ?? ()
No symbol table info available.
#53 0x00000000 in ?? ()
No symbol table info available.

[mbruel]

following this link: http://comments.gmane.org/gmane.comp.lib.qt.user/4719
I've noticed that at least on my example, not removing the QGraphicsItem from the scene before deleting it solve the issue.
I'm then checking the number of elements of my scene and it seems to be updated...
I don't really understand how the scene knows that an Item it is supposed to own has been deleted.
what is then the goal of QGraphicScene::removeItem... ?

Edit:

QGraphicsItem::~QGraphicsItem()

Destroys the QGraphicsItem and all its children. If this item is currently associated with a scene, the item will be removed from the scene before it is deleted.

Note: It is more efficient to remove the item from the QGraphicsScene before destroying the item.

So I get how it is removed from the scene... But still I don't get why it is crashing if I manually remove it from the scene before deletion.

Could it be a QT bug, cf https://bugreports.qt.io/browse/QTBUG-18021
Or am I doing something wrong?

Edit2: it is still crashing the same way on my main project... even not using the removeItem :'(

[SGaist]

Is it me or do you have many sets potentially containing pointers to many elements objects which might be deleted at some point ?

[mbruel]

@SGaist
Yes I do have sets of pointers on other objects but it is clear in term of ownership who will delete what.
My Elements have a sets of all the links they are in, and my Links one of all the Elements they are linking.
In their destructors, they remove themselves from the lists where they are referenced.

Element::~Element()
{
    for (LinkElement * link : _links)
    {
        link->removeElement(this);
    }
    delete _item;
}

LinkElement::~LinkElement()
{
    for (Element *elem : _elements)
        elem->removeLink(this);
}

I don't think the problem comes from there... I'm thinking more and more that this could be a bug in QT GraphicScene stack where it is trying to use an object that has been removed and deleted...
I don't know maybe I'm doing something wrong...
any ideas?

[SGaist]

Can you create a minimal compilable example that shows that behavior ? It would make things easier to try to fix be it your code or Qt itself.

[mbruel]

well it's already quite a simple compilable program. You can find a zip of the project here https://github.com/mbruel/testSceneMgr/archive/master.zip
I don't know how I could reproduce it otherwise....
it's just a few classes with no more than 50 lines of code each.
With this project, the crash is reproducible every time on both Windows and Linux.
You just need to launch it, move the link, delete one Element, then delete the link and normally it crashed.

[mbruel]

Here is a video on how to reproduce it the crash in 5 seconds:
https://sendvid.com/4p5hbt10

[josephdp]

Hi @mbruel

Looking into your code, you have a circular dependency for link and element classes which is a bad design.

class Element
{
...
QSet<LinkElement *> _links;
}

class LinkElement : public Element
{
...
QSet<Element*> _elements;
}

You need to have a class that manages both link and element (dependency inversion principle).
Same also with Element and GraphicItem classes.

This way you have a clear view of who's managing who and who's knowledgeable with who.

[mbruel]

@josephdp
Hi Joseph,
In my project I do use intermediate GraphicMangers class between the Element and the Items.
But I still have some reference like on this exemple and it is made by design.
Here is the UML diagram of the example I made. http://postimg.org/image/3zso2igrd/
It is kind of clear:

• Element owns a QGraphicsItem that is its graphical representation. (Element is in charge of the deletion of the QGraphicsItem)
• LinkElement inherit from Element but is more complex and hold a set _elements of pointers that it is linking (no ownership). It has methods to add and remove elements from this set. Its inherited _item is more complex than for a regular Element, instead of using a ComplexItem it will be a LinkNMItem that inherit from LinkItem.
• LinkItem inherit from QGraphicsItem and redefine the mouse events and also the paint. It has a set of pointer on the QGraphicsItems of the Elements that the LinkElement is linking. Also no ownership. There are methods to add/remove those items.

Anyway the crash doesn't come from those "circular" dependencies. It comes from the QGraphicsScene/QGraphicsView.
Doing some tests I can see the QGraphicsScene::removeItem works in the term that QGraphicScene::items().count() decreases BUT for some reason somewhere in QT stack, the item is still in use and the scene/view is not refreshed properly. I need to move another simplier item (one that has a boundingRect with fix size and/or that is not redefining the mouseMoveEvent) in order to have my item really removed from the scene.
I don't understand why... and it is a problem cause I'd like to destroy the item.

If I comment out the deletion of _item in the destructor of Element:

Element::~Element()
{
    for (LinkElement * link : _links)
    {
        link->removeElement(this);
    }
//    delete _item;
}

I then have a memory leak and we can see that the QGraphicsScene::removeItem fails to update the GraphicsView if I delete my link in second.
Cf this video: https://sendvid.com/nagutw1l
The link is still present on the scene until I move other Elements and then disappear at one point.
QT stack holds a pointer on it.
My crash is due to the fact I'm destroying the linkItem although QT still may use it, it even redraws it...

I need a way to make sure it is properly removed and won't be used anymore.

PS: I've changed a little bit the mouse event handlers to properly use or discard the events and it doesn't change the behaviour.

void LinkItem::mouseMoveEvent(QGraphicsSceneMouseEvent *event)
{
    if (_isMoving)
    {
        _movablePoint = event->scenePos();
        update();
        event->accept();
    }
    else
        event->ignore();
}

void LinkItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
{
    QGraphicsItem::mouseReleaseEvent(event);
    if (_isMoving)
    {
        _movablePoint = event->scenePos();
        _isMoving = false;
        _element->setPos(_movablePoint);
        prepareGeometryChange();
    }
}

PS2: I've found a way to avoid this behaviour, it is by setting to true the flag to ignore transformations on the item:

 _item->setFlag(QGraphicsItem::ItemIgnoresTransformations, true);

Unfortunately on my main project I use zoom features that uses transformation so I can't use this "trick"

[mbruel]

Wow... Finally I think I've found a way to avoid the crash \o/
I've read in some places like here http://stackoverflow.com/questions/38458830/crash-after-qgraphicssceneremoveitem-with-custom-item-class or here: http://www.qtcentre.org/archive/index.php/t-33730.html
that it could be an issue that prepareGeometryChange wasn't called.
But I'm calling it all the time I'm changing the size of my item.
I guess QT gets confused cause this happens often: on mouse events...
Anyway reading about BSP tree and continuing to search I found this link: http://tech-artists.org/forum/showthread.php?3229-Qt-Properly-removing-qGraphicItems
where it suggests to not use BSP indexing!

_scene->setItemIndexMethod(QGraphicsScene::ItemIndexMethod::NoIndex);

coooool!
I'll try it tomorrow to see if that solve the issue on my main project but I'm quite confident it will...

[mbruel]

Yep, problem solved!
I don't need BSP indexing on my scene. I think this should be a bit more explained in the the head of the documentation of the QGraphicsScene.
http://doc.qt.io/qt-4.8/qgraphicsscene.html#ItemIndexMethod-enum

- QGraphicsScene::BspTreeIndex: A Binary Space Partitioning tree is applied. All QGraphicsScene's item location algorithms are of an order close to logarithmic complexity, by making use of binary search. Adding, moving and removing items is logarithmic. This approach is best for static scenes (i.e., scenes where most items do not move).

- QGraphicsScene::NoIndex: No index is applied. Item location is of linear complexity, as all items on the scene are searched. Adding, moving and removing items, however, is done in constant time. This approach is ideal for dynamic scenes, where many items are added, moved or removed continuously.

But I still find weird that QT keeps a pointer in the BSP indexing Tree on an QGraphicsItem removed from the scene... It's dangerous as the user should be free to delete that QGraphicsItem.
For me, the BSP tree should do all its last treatment before the end of QGraphicsScene::removeItem...

[SGaist]

Thanks for sharing your findings !

Looks like you may have unearthed something though, maybe report it ?

[mbruel]

yes, I'm going to share this thread on the QT defect I mentioned before. It was open end of 2012, investigated last year but still not fixed.
it may help to provide them an easy way to reproduce the crash.

[aziesemer]

I'm having this exact same problem (also described in https://bugreports.qt.io/browse/QTBUG-18021).

I always call prepareGeometryChange before any geometry change, so I have no idea what the problem is.

Some suggested to call prepareGeometryChange before removing the item from the scene but it didn't fix the issue.

Removing BSP indexing solved but is not a good option for me.

[cSielaff]

Replying here, even though this thread is quite old ... I also ran into the same issue using PySide2. Disabling BSP indexing does indeed work for me, but is a sub-optimal solution because the scene that I'm working with can get arbitrarily large. I also tried to call update prepareGeometryChange before removing the item, and while that did seem to work for a while, the error re-appeared just a few weeks later.

What worked for me (so far) is manually removing all child items before removing the item itself...
To that end, I am overwriting the QGraphicsScene::removeItem method in Python:

class GraphicsScene(QtWidgets.QGraphicsScene):
    def removeItem(self, item: QtWidgets.QGraphicsItem) -> None:
        for child_item in item.childItems():
            super().removeItem(child_item)
        super().removeItem(item)

Note that this will not quite work the same in C++ because QGraphicsScene::removeItem is not a virtual method, so you will probably have to add your own method removeItemSafely or whatever.

Disclaimer: Other methods have worked for me as well ... until they didn't. I have not seen a crash in QGraphicsSceneFindItemBspTreeVisitor::visit since introducing this workaround, but that does not mean that this is actually the solution. Use at your own risk.