Deleting QGraphicsItem with QGraphicsEffect leads to segfault

[kshegunov]

class MyScene: public QGraphicsScene
{
    Q_OBJECT //< You need also to add this

public:
    void mouseReleaseEvent(QGraphicsSceneMouseEvent* me) override
    {
        if (me->button()==Qt::LeftButton)  {
            createRect(this);
            emit changed(); //< Emit changed()
        }
        else if (me->button()==Qt::RightButton)  {
            destroyRect(this);
            emit changed();
        }
        // ...
    }
};

PS.
Sorry, the example won't work like this, one needs to pass a number of regions to the signal, but I'm not exactly sure how to obtain them in this case.

Perhaps something like this:

class MyScene: public QGraphicsScene
{
    Q_OBJECT

public:
    void mouseReleaseEvent(QGraphicsSceneMouseEvent* me) override
    {
        // ...
        else if (me->button()==Qt::RightButton)  {
            if (global_rect)  {
                QRectF rect = global_rect->rect();
                scene->removeItem(global_rect);

                emit changed(QList<QRectF>() << rect);

                delete global_rect; //< Still might crash, it's a long shot
                global_rect=nullptr;
            }
        }
        // ...
    }
};

[tfm123]

Unfortunately it changes nothing. Without delete there are still the artifects, With delete there still is segfault.

Thanks for your help though.

But I'd like to come back to a more basic question: Do you think that my code is correct this way? Or do I use QT in a wrong way?

[kshegunov]

I have no way to answer that, as I neither have any idea what you're trying to accomplish, nor can I tell if this code is a stripped down example that should reproduce the problem, or is an actual snippet from your project ... or something in between.

[tfm123]

It is a minimal example written to reproduce the error. It is fully compilable, I even provided the compiler call.

I was hoping someone with experience in QT would be able to tell me whether I am using QT the right way.

Or, to put it in other words, is this kind of error expected behaviour for the example program?

[kshegunov]

Well I had to run it through the debugger to figure out what's going on. The problem is you add child items in the constructor of the parent item. At that point the scene the item holds a reference to isn't initialized (apparently), so you get a weird event-race of some kind. Anyway, moving new QGraphicsRectItem{this}; away from the constructor of MyRect and into createRect() should fix the issue, at least it did for me.

EDIT: Well it didn't as it appears. Okay back to the debugger.

As for your code (the one posted), there's a few things I don't like:

1. You use global functions to do internal bookkeeping
2. You use global variables for no good reason
3. You use a heap allocation (i.e. in main()) when that's not really warranted, declaring the view and scene on the stack works just as well.

I don't know if that was the feedback you were looking for, but I hope it helps.

EDIT 2:

Well, apparently it some kind of double-deletion problem. Moving all things as members with auto-storage fixes it. Here's the code I tested with:
main.cpp

#include<QApplication>
#include<QGraphicsView>
#include<QScreen>

#include "myscene.h"

int main(int argc, char ** argv)
{
    QApplication qapp(argc, argv);

    QGraphicsView view;
    MyScene scene;

    QRect rect = QGuiApplication::primaryScreen()->geometry();
    scene.setSceneRect(0.0f, 0.0f, rect.width(), rect.height());
    view.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
    view.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
    view.setFrameShape(QFrame::NoFrame);
    view.setBackgroundBrush(QBrush(Qt::black, Qt::SolidPattern));
    view.setScene(&scene);
    view.showFullScreen();

    return qapp.exec();
}

myscene.h

#ifndef MYSCENE_H
#define MYSCENE_H

#include <QGraphicsScene>
#include <QGraphicsSceneMouseEvent>
#include <QGraphicsRectItem>
#include <QGraphicsDropShadowEffect>

class MyRect : public QGraphicsRectItem
{
public:
    MyRect(QGraphicsItem * parent = nullptr)
        : QGraphicsRectItem(QRectF{0.0f, 0.0f, 100.0f, 100.0f}, parent)
    {
        setPen(QPen(Qt::white));
        setBrush(QBrush(Qt::white, Qt::SolidPattern));
    }
};

class MyScene: public QGraphicsScene
{
    Q_OBJECT

public:
    MyScene(QObject * parent = Q_NULLPTR)
        : QGraphicsScene(parent), background(0.0f, 0.0f, 500.0f, 500.0f), global_rect(Q_NULLPTR)
    {
        background.setPen(QPen(Qt::white));
        background.setBrush(QBrush(Qt::red, Qt::SolidPattern));
        addItem(&background);

        QGraphicsDropShadowEffect * shadow=new QGraphicsDropShadowEffect();
        shadow->setBlurRadius(15.0f);

        global_rect.setGraphicsEffect(shadow);


        addItem(new QGraphicsRectItem(&global_rect));

    }

    void mouseReleaseEvent(QGraphicsSceneMouseEvent* me) override
    {
        if (me->button()==Qt::LeftButton)
        {
            addItem(&global_rect);
            global_rect.setPos(me->scenePos());
        }
        else if (me->button()==Qt::RightButton)
        {
            removeItem(&global_rect);
        }

        QGraphicsScene::mouseReleaseEvent(me);
    }

    QGraphicsRectItem background;
    MyRect global_rect;
};

#endif // MYSCENE_H

Kind regards.

[Chris Kawa]

I've seen this problem recently in my colleague's code, and a (curious enough) workaround we found was calling prepareGeometryChange() before the item was removed from the scene. Unfortunately it's protected so you need to expose it e.g.

class MyRect: public QGraphicsRectItem {
public:
    void ouch() { prepareGeometryChange(); }
    ...
};

//and then
    if (global_rect!=nullptr)
    {
        global_rect->ouch(); //call it
        scene->removeItem(global_rect);
        delete global_rect;
        global_rect=nullptr;
    }

Unfortunately I never got to investigate what's going on in depth but it looks like a Qt bug.
A working theory based on the callstack we got was that the crash occurs when the area of the effect crosses the bounds of the internal BSP structure, but the item itself does not. The idea was that the item is removed and that part of the tree it occupied is invalidated but the part that only displayed the effect is not and somehow still somewhere holds a pointer to now deleted object. Calling prepareGeometryChange() would invalidate the whole area (including the area of the effect) and thus prevented the crash.
As I said - it's just a shot in the dark and can be totally off track, so take it with a grain of salt, but it's definitely not an expected behavior and I'd consider reporting it.

[tfm123]

@Chris-Kawa Thank you a lot for your assessment. Also your suggestion fixes the problem.

I have put the prepareGeometryChange() into MyRect's destructor. I now just delete the item and do not explicitly remove it from the scene. ~QGraphicsItem() does remove itself from the scene and it is called after ~MyRect(). So this is basicly equivalent to what you wrote, but saves the exposure of prepareGeometryChange().

The only thing you cannot do now, is to remove the item from the scene without deleting it. I'm going to file a bug report when i have some time.

Thanks again!

[Oleksandr Malyushytskyy]

Check if item bounding rectangle increases if QGraphicsDropShadowEffect is on.
If not this is a problem.
This would be also a reason why calling prepareGeometryChange() helped.
I had similar problem for my custom items if bounding rectangle was not set properly.

[Oleksandr Malyushytskyy]

@tfm123 As a workaround you could call setGraphicsEffect(0). before removing item., but this does not guarantee to fix all problems, the same as approach you used.
Problem may still occur in other usage cases which were not found by you yet. Drawing of the item should be always inside the bounding box it reports.

[jsulm]

@tfm123 Why do you create a QGraphicsRectItem instance in the body of the constructor?

MyRect(QGraphicsItem* parent = nullptr):
		QGraphicsRectItem{QRectF{0.0f, 0.0f, 100.0f, 100.0f}, parent}
{
    setPen(QPen{Qt::white});
    setBrush(QBrush{Qt::white, Qt::SolidPattern});
    new QGraphicsRectItem{this}; // <-- why this?
}

[tfm123]

@Oleksandr-Malyushytskyy The boundingRect() of the rectangle does not change when the effect is applied to it. But the effect itself has a boundingRect() which changes when the effect is applied. I don't know if this is how it is supposed to be.

[tfm123]

@Oleksandr-Malyushytskyy This is one of the first things I tried and unfortunately it does not help.

[tfm123]

@jsulm Like I already wrote, the issue only triggers when MyRect has a child. This is an example. The only reason for things to be like they are is that they are required to be this way to trigger the issue.

[aziesemer]

@tfm123

I think this bug is described here:
https://bugreports.qt.io/browse/QTBUG-18021

My workaround was to setVisible(false) before removing the item.

[JoeCFD]

@aziesemer Got random crashes of QLabel with PixMap + QGraphicsEffect on Ubuntu 18.04 + Qt 5.15.2.
Backtrace:
#0 0x00007ffff1510fb7 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff1512921 in __GI_abort () at abort.c:79
#2 0x00007ffff155b967 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff1688b0d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3 0x00007ffff15629da in malloc_printerr (str=str@entry=0x7ffff1686c3a "corrupted double-linked list") at malloc.c:5342
#4 0x00007ffff1562b94 in malloc_consolidate (av=av@entry=0x7ffff18bdc40 <main_arena>) at malloc.c:4486
#5 0x00007ffff156a12b in _int_free (have_lock=0, p=<optimized out>, av=0x7ffff18bdc40 <main_arena>) at malloc.c:4392
#6 0x00007ffff156a12b in __GI___libc_free (mem=0x5555600bf810) at malloc.c:3134
#7 0x00007ffff3f512ba in QImageData::~QImageData() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Gui.so.5
#8 0x00007ffff3f517a7 in QImage::~QImage() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Gui.so.5
#9 0x00007ffff3f8b4dc in QRasterPlatformPixmap::~QRasterPlatformPixmap() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Gui.so.5
#10 0x00007ffff3f8b4f9 in QRasterPlatformPixmap::~QRasterPlatformPixmap() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Gui.so.5
#11 0x00007ffff3f81091 in QPixmap::~QPixmap() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Gui.so.5
#12 0x00007ffff3f858b1 in QPixmapCache::remove(QPixmapCache::Key const&) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Gui.so.5
#13 0x00007ffff4b66165 in QWidgetPrivate::invalidateGraphicsEffectsRecursively() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#14 0x00007ffff4b661b2 in QWidgetPrivate::setDirtyOpaqueRegion() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#15 0x00007ffff4b79180 in QWidget::~QWidget() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#16 0x00007ffff4c6baa9 in QLabel::~QLabel() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5

Anyone had the same issue?

[JoeCFD]

@JoeCFD
after I changed the code in the destructor to

    if ( nullptr != m_label ) {
        m_label->setGraphicsEffect( nullptr );
        m_label->setVisible( false );
        m_label->clear();
        delete m_label;
        m_label = nullptr;
    }

backtrace results are different now:

(gdb) backtrace
#0 0x00007ffff418e602 in QPainterPath::~QPainterPath() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Gui.so.5
#1 0x00007ffff4bf61f6 in QRenderRule::~QRenderRule() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#2 0x00007ffff279e5c9 in QHashData::free_helper(void ()(QHashData::Node)) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Core.so.5
#3 0x00007ffff279e5c9 in QHashData::free_helper(void ()(QHashData::Node)) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Core.so.5
#4 0x00007ffff4bf50a3 in QHash<QObject const*, QHash<int, QHash<unsigned long long, QRenderRule> > >::remove(QObject const* const&) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#5 0x00007ffff4bd8846 in QStyleSheetStyleCaches::objectDestroyed(QObject*) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#6 0x00007ffff4bd88dc in QStyleSheetStyleCaches::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#7 0x00007ffff2982ddf in void doActivate<false>(QObject*, int, void**) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Core.so.5
#8 0x00007ffff297c50f in QObject::destroyed(QObject*) () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Core.so.5
#9 0x00007ffff4b7946b in QWidget::~QWidget() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5
#10 0x00007ffff4c6baa9 in QLabel::~QLabel() () at /opt/thirdParties/Qt/5.15.2/gcc_64/lib/libQt5Widgets.so.5

[JoeCFD]

@JoeCFD delete m_label; causes the crash. But the crash does not happen every time when the parent of m_label is destroyed.

[SGaist]

@JoeCFD did you try the workaround mentioned by @Chris-Kawa ?

[JoeCFD]

@SGaist Thanks for your reply. The random crash may be caused by corrupt memory. Will use valgrind to check the code out.