Removing items from QTreeWidget causes SIGBUS

[hdino]

Hi,

I have a simple QMainWindow with a QTreeWidget and two functions to add and to remove items. Basically, the functions look like this:

void MainWindow::addObject(const std::string &name) {
    QTreeWidgetItem *new_item = new QTreeWidgetItem();
    new_item->setText(0, QString::fromStdString(name));
    std::lock_guard<std::mutex> tree_devices_lock(tree_devices_mutex_);
    ui->treeDevices->addTopLevelItem(new_item);
}

void MainWindow::removeObject(const std::string &name) {
    std::lock_guard<std::mutex> tree_devices_lock(tree_devices_mutex_);

    for (int index = 0; index < ui->treeDevices->topLevelItemCount(); ++index) {
        QTreeWidgetItem *current_item = ui->treeDevices->topLevelItem(index);
        if (current_item->text(0).toStdString() == name) {
            delete ui->treeDevices->takeTopLevelItem(index);
            break;
        }
    }
}

Now after adding and removing some items the program crashes with a bus error (SIGBUS). According to valgrind, Qt tries to access the item I deleted in the second function. Do you have any idea why? Is the delete line correct?

Valgrind output:

==1765== Invalid read of size 8
==1765==    at 0x4CBFA76: ??? (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4CB03FE: QTreeView::drawRow(QPainter*, QStyleOptionViewItem const&, QModelIndex const&) const (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4CB5A46: QTreeView::drawTree(QPainter*, QRegion const&) const (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4CBA482: QTreeView::paintEvent(QPaintEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4A3E587: QWidget::event(QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4AE1D1D: QFrame::event(QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4C531BA: QAbstractItemView::viewportEvent(QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4CBB40A: QTreeView::viewportEvent(QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x58F72BA: QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Core.so.5.11.3)
==1765==    by 0x4A004A0: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x4A0794F: QApplication::notify(QObject*, QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5.11.3)
==1765==    by 0x58F75A8: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/x86_64-linux-gnu/libQt5Core.so.5.11.3)
==1765==  Address 0xf38cdb8 is 40 bytes inside a block of size 64 free'd
==1765==    at 0x483708B: operator delete(void*, unsigned long) (vg_replace_malloc.c:585)
==1765==    by 0x1520E0: MainWindow::removeObject(std::string const&) (mainwindow.cpp:59)

GDB backtrace:

Thread 1 "test_viewer" received signal SIGBUS, Bus error.
0x00007ffff7d3887d in ?? () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
(gdb) bt
#0  0x00007ffff7d3887d in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#1  0x00007ffff7d38a89 in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#2  0x00007ffff7d2b2aa in QTreeView::indexRowSizeHint(QModelIndex const&) const () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#3  0x00007ffff7d2c9cc in QTreeViewPrivate::itemHeight(int) const () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#4  0x00007ffff7d2e2d6 in QTreeViewPrivate::updateScrollBars() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#5  0x00007ffff7d3574e in QTreeView::updateGeometries() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#6  0x00007ffff7cbf5ed in QAbstractItemView::doItemsLayout() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#7  0x00007ffff7d33b29 in QTreeView::doItemsLayout() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#8  0x00007ffff7cc231d in QAbstractItemView::timerEvent(QTimerEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#9  0x00007ffff7d340f2 in QTreeView::timerEvent(QTimerEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#10 0x00007ffff6f1e13b in QObject::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#11 0x00007ffff7ab7a1b in QWidget::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x00007ffff7b5ad1e in QFrame::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#13 0x00007ffff7b5d934 in QAbstractScrollArea::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x00007ffff7ccbe99 in QAbstractItemView::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#15 0x00007ffff7a794b1 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#16 0x00007ffff7a80950 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#17 0x00007ffff6ef45a9 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#18 0x00007ffff6f44c78 in QTimerInfoList::activateTimers() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#19 0x00007ffff6f454d4 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#20 0x00007ffff5ab5f2e in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#21 0x00007ffff5ab61c8 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#22 0x00007ffff5ab625c in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#23 0x00007ffff6f45863 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#24 0x00007ffff2fb83e1 in  () at /lib/x86_64-linux-gnu/libQt5XcbQpa.so.5
#25 0x00007ffff6ef327b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x00007ffff6efb262 in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#27 0x0000555555583b9c in main(int, char**) (argc=1, argv=0x7fffffffd1a8) at /home/main.cpp:10

[jsulm]

@hdino said in Removing items from QTreeWidget causes SIGBUS:

delete ui->treeDevices->takeTopLevelItem(index);

Try deleteLater instead:

ui->treeDevices->takeTopLevelItem(index)->deleteLater();

[hdino]

Unfortunately, QTreeWidgetItem doesn't provide deleteLater(), c.f. this post. They suggest to use delete.

[JonB]

@hdino
As you have noted, there are several posts out there stating that you should just be able to delete and it should work. It would seem likely that you are not the only person who would want to delete a QTreeWidgetItem.

Would you care to set up & test a small, standalone for this to see if you can reproduce? Because if you can it would be worthy of a bug submit.

P.S.
If you want a nasty workaround: looking at the trace and seeing it's on a timer for a scroll or redraw or something, you could try setting to do your delete on a timer itself and see if the fault goes away.
P.P.S.
Try replacing delete ui->treeDevices->takeTopLevelItem(index) with plain ui->treeDevices->takeTopLevelItem(index);. Does it still actually mis-perform, it's the removal of the item rather than the deletion which is the issue?

[hdino]

@JonB
Thanks for your suggestions. I tried a plain takeTopLevelItem without delete and the error didn't occur anymore. To further investigate this, I set up a small standalone application, but that works just fine. Thus, I guess there is something else wrong with my application (although valgrind did not show anything suspicious...).

Code of the standalone application:

#include <QApplication>
#include <QMainWindow>
#include <QTreeWidget>

#include <functional>
#include <memory>
#include <string>
#include <thread>

/** #################### EXTERNAL LIBRARY #################### **/

class ExternalLibrary
{
public:
    typedef std::function<void (const std::string &)> Callback;

    ExternalLibrary(Callback new_object_callback, Callback removed_object_callback);
    ~ExternalLibrary();

private:
    void doWork();

    Callback new_object_callback_;
    Callback removed_object_callback_;

    bool run_;
    std::thread thread_;
};

ExternalLibrary::ExternalLibrary(Callback new_object_callback, Callback removed_object_callback) :
    new_object_callback_(new_object_callback),
    removed_object_callback_(removed_object_callback),
    run_(true),
    thread_(&ExternalLibrary::doWork, this)
{}

ExternalLibrary::~ExternalLibrary()
{
    run_ = false;
    thread_.join();
}

void ExternalLibrary::doWork()
{
    while (run_)
    {
        new_object_callback_("Test1");
        std::this_thread::sleep_for(std::chrono::milliseconds(20));
        new_object_callback_("Test2");
        std::this_thread::sleep_for(std::chrono::milliseconds(20));
        new_object_callback_("Test3");
        std::this_thread::sleep_for(std::chrono::milliseconds(200));
        removed_object_callback_("Test2");
        std::this_thread::sleep_for(std::chrono::milliseconds(20));
        removed_object_callback_("Test3");
        std::this_thread::sleep_for(std::chrono::milliseconds(20));
        removed_object_callback_("Test1");
        std::this_thread::sleep_for(std::chrono::milliseconds(200));
    }
}

/** #################### MAIN WINDOW #################### **/

class MainWindow : public QMainWindow
{
public:
    MainWindow();

    void addObject(const std::string &name);
    void removeObject(const std::string &name);

private:
    QTreeWidget *tree_widget_;

    std::unique_ptr<ExternalLibrary> lib_;
};

MainWindow::MainWindow()
{
    setCentralWidget(new QWidget()); // QMainWindow takes ownership
    setFixedSize(400, 300);

    tree_widget_ = new QTreeWidget(centralWidget()); // centralWidget takes ownership

    lib_.reset(new ExternalLibrary([this](const std::string &name){ addObject(name); }, [this](const std::string &name){ removeObject(name); }));
}

void MainWindow::addObject(const std::string &name)
{
    QTreeWidgetItem *new_item = new QTreeWidgetItem();
    new_item->setText(0, QString::fromStdString(name));
    tree_widget_->addTopLevelItem(new_item);
}

void MainWindow::removeObject(const std::string &name)
{
    for (int index = 0; index < tree_widget_->topLevelItemCount(); ++index)
    {
        QTreeWidgetItem *current_item = tree_widget_->topLevelItem(index);
        if (current_item->text(0).toStdString() == name)
        {
            delete tree_widget_->takeTopLevelItem(index);
            break;
        }
    }
}

/** #################### MAIN #################### **/

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);

    MainWindow main_window;
    main_window.show();

    return a.exec();
}

[J.Hilk]

hi @hdino
can you try the following? Maybe the inlining of the delete causes problems.

if (current_item->text(0).toStdString() == name) {
            auto itemToDelete = ui->treeDevices->takeItem(index); 
            if(itemToDelete)
                   delete itemToDelete;
            break;
        }

theoretically calling delete on a nullptr should have no effect, but it doesn't hurt to check either.

I set up a small standalone application, but that works just fine.

Do you use the tree widgetItem in a lambda expression? Or track it with a member pointer?

[JonB]

@hdino said in Removing items from QTreeWidget causes SIGBUS:

To further investigate this, I set up a small standalone application, but that works just fine.

I had a feeling this might be the case, else other people would be experiencing your issue. That's why a standalone is always advisable. In some shape or form, it feels like you still have a dangling reference to the deleted item somewhere, either explicitly or implicitly. Obviously please try @J-Hilk's suggestions, especially lambda or member pointers (delete nullptr really should be fine).