Deleting QGraphicsItem with QGraphicsEffect leads to segfault

tfm123

The following program paints a red background. Pressing the left mouse button paints a white rectangle onto it. The rectangle has a child rectangle and a QGraphicsDropShadowEffect. (QGraphicsOpacityEffect and QGraphicsColorizeEffect also lead to the problem, but less frequently.)

Pressing the right mouse button removes the white rectangle.

Sometimes when removing the rectangle it causes a segmentation fault.

This does not happen if the QGraphicsDropShadowEffect is not applied. It also does not happen when there either is no child item in MyRect or the background is ommited.

(When searching for this issue, I found several hints that a segfault like this could be related to changing the boundingRect() of an item without calling prepareGeometryChange().)

I am really at a loss here. This is part of a bigger project and I boiled it down to the following example:

Main.cc:

#include<QApplication>
#include<QGraphicsView>
#include<QGraphicsScene>
#include<QGraphicsSceneMouseEvent>
#include<QGraphicsRectItem>
#include<QGraphicsDropShadowEffect>
#include<QScreen>

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});
		new QGraphicsRectItem{this};
	}
};

QGraphicsView* view=nullptr;
QGraphicsRectItem* background=nullptr;
MyRect* global_rect=nullptr;

void
createRect(QGraphicsScene* scene)
{
	if (global_rect==nullptr)
	{
		global_rect=new MyRect{};
		auto shadow=new QGraphicsDropShadowEffect{};
		shadow->setBlurRadius(15.0f);
		global_rect->setGraphicsEffect(shadow);
		scene->addItem(global_rect);
		global_rect->setPos(global_rect->mapToParent(global_rect->mapFromScene(view->mapToScene(view->mapFromGlobal(QCursor::pos())))));
	}
}

void
destroyRect(QGraphicsScene* scene)
{
	if (global_rect!=nullptr)
	{
		scene->removeItem(global_rect);
		delete global_rect;
		global_rect=nullptr;
	}
}

class MyScene: public QGraphicsScene
{
	public:
	void mouseReleaseEvent(QGraphicsSceneMouseEvent* me) override
	{
		if (me->button()==Qt::LeftButton)
		{
			createRect(this);
		}
		else if (me->button()==Qt::RightButton)
		{
			destroyRect(this);
		}
		else
		{
			QGraphicsScene::mouseReleaseEvent(me);
		}
	}
};

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

	view=new QGraphicsView{};
	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();

	background=new QGraphicsRectItem{0.0f, 0.0f, 500.0f, 500.0f};
	background->setPen(QPen{Qt::white});
	background->setBrush(QBrush{Qt::red, Qt::SolidPattern});
	scene.addItem(background);

	return qapp.exec();
}

Compiler call:

g++ --std=c++14 -fPIC -Wall -Woverloaded-virtual -Werror -pedantic -g -O0 -fPIC -I/usr/include/x86_64-linux-gnu/qt5/QtGui -I/usr/include/x86_64-linux-gnu/qt5/QtWidgets -I/usr/include/x86_64-linux-gnu/qt5/QtCore -I/usr/include/x86_64-linux-gnu/qt5 -o test Main.cc -lQt5Gui -lQt5Core -lQt5Widgets

Backtrace:

#0  0x00007ffff71a1b84 in QGraphicsItem::parentItem() const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#1  0x00007ffff71a1bab in QGraphicsItem::topLevelItem() const () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#2  0x00007ffff71eab6e in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#3  0x00007ffff71e9ba6 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#4  0x00007ffff71e9bfa in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#5  0x00007ffff71e9bfa in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#6  0x00007ffff71e9ba6 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#7  0x00007ffff71e9c83 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#8  0x00007ffff71ecd3c in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#9  0x00007ffff71ed007 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#10 0x00007ffff71dfc0d in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#11 0x00007ffff7201aaa in QGraphicsView::paintEvent(QPaintEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x00007ffff6f0f218 in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#13 0x00007ffff6ff799e in QFrame::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x00007ffff72005bb in QGraphicsView::viewportEvent(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#15 0x00007ffff76506f1 in QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#16 0x00007ffff6ec7b05 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#17 0x00007ffff6ecf2e1 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#18 0x00007ffff7650990 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#19 0x00007ffff6f07f7a in QWidgetPrivate::sendPaintEvent(QRegion const&) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#20 0x00007ffff6f085e6 in QWidgetPrivate::drawWidget(QPaintDevice*, QRegion const&, QPoint const&, int, QPainter*, QWidgetBackingStore*) ()
 from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#21 0x00007ffff6ed7ebe in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#22 0x00007ffff6ed80e7 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#23 0x00007ffff6ef6f2f in QWidgetPrivate::syncBackingStore() () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#24 0x00007ffff6f0f2e8 in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#25 0x00007ffff6ff799e in QFrame::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#26 0x00007ffff7080d83 in QAbstractScrollArea::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#27 0x00007ffff6ec7b2c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#28 0x00007ffff6ecf2e1 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#29 0x00007ffff7650990 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#30 0x00007ffff765311d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#31 0x00007ffff71d1c32 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#32 0x00007ffff71d74a9 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#33 0x00007ffff767d449 in QObject::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#34 0x00007ffff71e407b in QGraphicsScene::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#35 0x00007ffff6ec7b2c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#36 0x00007ffff6ecf2e1 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#37 0x00007ffff7650990 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#38 0x00007ffff765311d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#39 0x00007ffff76a4bf3 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#40 0x00007ffff446e7f7 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#41 0x00007ffff446ea60 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#42 0x00007ffff446eb0c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#43 0x00007ffff76a4fff in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#44 0x00007ffff764e97a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#45 0x00007ffff76570ec in QCoreApplication::exec() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#46 0x0000555555557b10 in main (argc=1, argv=0x7fffffffe5b8) at Main.cc:92

kshegunov

Hello,
A curious conundrum. The problem is you have events for the graphics effect that are pending in the event queue, when you delete the graphics item. When the events are ultimately processed you get a crash as Qt's referencing a dangling pointer, at least that's my theory. To fix it first I'd try calling:

global_rect->setGraphicsEffect(Q_NULLPTR);

just before deleting the actual graphics item.

Kind regards.

tfm123

Thanks for your feedback.

Unfortunately I already had tried that and it does not change anything.

I'd generally like to know whether or not I am using QGraphicsEffect right.

kshegunov

Okay, can you at least confirm or deny that this is the actual problem - comment out the delete and see if a crash occurs?

tfm123

It does not happen when you do one of the following:

do not add a child to MyRect
do not add a QGraphicsEffect
comment out the delete as you suggested

When commenting out the delete I now noticed something else: The white rectangle of MyRect sometimes remains on the screen although I pressed right mouse button. This is curious because it still is removed from the scene.

My guess is, these are the cases that lead to segfault when the delete is in place. The scene somehow thinks MyRect is still present. If it is actually still in memory it's okay and just an artefact on the screen. If it is actually deleted it results in segfault.

kshegunov

@tfm123 said in Deleting QGraphicsItem with QGraphicsEffect leads to segfault:

When commenting out the delete I now noticed something else: The white rectangle of MyRect sometimes remains on the screen although I pressed right mouse button. This is curious because it still is removed from the scene.

Emit the changed() signal after removing the item to notify the view of the change. I think this should fix it. It may even fix the original problem, although it's a very long shot.

tfm123

I'm not quite sure how to do that. I have not been using signals and slots; I'm only using QGraphics. Is there a method on the view or the scene I can call to do this?

Edit: I have tried QGraphicsView::update(), QGraphicsView::invalidateScene(), QGraphicsScene::update() and QGraphicsScene::invalidate(). Nothing changes anything.

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:

You use global functions to do internal bookkeeping
You use global variables for no good reason
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.