Use of QTreeWidget causing heap corruption

eos pengwern

I have a QTreeWidget with two levels of QTreeWidgetItems, which needs to be redrawn from time to time when the underlying structure, which it represents to the user, changes. The mechanism I use to redraw the QTreeWidget is basically as set out below:

In header file:

void redrawTable();
void populateEntry(int index, QTreeWidgetItem *parent);

MyCustomDataType           theData;
QTreeWidget                theTable;
QVector<QTreeWidgetItem *> theItemList;

In .cpp file:

void MyWidget::redrawTable()
{
    theTable -> clear();   // Should delete all QTreeWidgetItems and their
                           // children if I've read the documentation right

    theItemList.clear();   // So, if I've read the documentation right, these
                           // pointers can be safely discarded since the objects
                           // they once pointed to have been deleted

    int nItems = theData.getNumberOfItems();
    for (int i=nItems; i >= 0; i--)
    {
        QTreeWidgetItem *item = new QTreeWidgetItem(theTable -> invisibleRootItem());
        populateEntry(i, item);
    }
}

void MyWidget::populateEntry(int index, QTreeWidgetItem *parent)
{
    theItemList.append(new QTreeWidgetItem(parent));  // These items are kept in the list
                                                      // because we'll want to be able
                                                      // to update their text from time 
                                                      // to time
    theItemList.last() -> setText(0, theData.getCurrentColumnZeroText(index));
    theItemList.last() -> setText(1, theData.getCurrentColumnOneText(index));
    theItemList.last() -> setText(2, theData.getCurrentColumnTwoText(index));
}

As my comments indicate, I've managed to convince myself that this is correct and that the various QTreeWidgetItem * pointers in 'theItemList' are being created and destroyed safely. Certainly, it works just fine: when appropriate the table is redrawn and looks exactly as it should do, with no error messages being reported.

Here, then is the problem. I can perform certain sequences of actions within my GUI that lead to some serious heap corruption issues; really weird stuff (failed std::moves, fields being randomly deallocated deep within objects which are otherwise intact, etc.), and while I always get the errors after the same sequences of actions, the precise nature of the errors and the objects which they affect appear to be totally random. This suggests to me that I'm seeing the outworking of an error that lies elsewhere, far away from the code that happens to be running when the error surfaces. A lot of investigation over many days has enabled me to identify that the one thing in common with all the "failure cases" is that the above code has been executed a little while before the failure occurs. If I comment out the call to 'redrawTable' (i.e.if I still make all the same changes as before to the underlying data structures, but I just refrain from updating the widget) then the heap errors never occur.

However, I've run the above code through Windows Application Verifier, and Intel Inspector, and no errors get flagged up while it is executing.

What am I missing?

VRonin

I can't see any evident error. the only strange thing is your loop for (int i=nItems; i >= 0; i--) this will iterate nItems+1 times, what you probably want is for (int i=nItems-1; i >= 0; i--) but this shouldn't cause a heap corruption anyway

eos pengwern

@VRonin Thank you, but the loop is in fact correct for my use case.

kshegunov

@eos-pengwern said in Use of QTreeWidget causing heap corruption:

However, I've run the above code through Windows Application Verifier, and Intel Inspector, and no errors get flagged up while it is executing.

Enable all stack smashing dummies and heap-boundary checks your compiler supports (usually compiling in debug mode is enough) and try to get a precise location for the crash. I can't see anything plainly wrong, but in release mode you can write over foreign memory without much of a problem. Put asserts where appropriate to facilitate debug checks.

eos pengwern

@kshegunov Thank you. That is what I'm doing, and the problem is that each time an error occurs it is different and apparently random; usually an access violation thrown from deep within a system DLL, usually relating to a memory location which has no relation to any variable that I've declared as far as I can see, and yet I often notice that others of my own data structures are corrupted (even though they reside at addresses far from the access violation). The only pattern I can spot is that they never happen if I comment out 'redrawTable()'.

VRonin

what is fitItemList ?

eos pengwern

@VRonin I'm sorry; that should be 'theItemList' (I've edited the original question now). A typical mistake when one changes the variable names in the one's original code in the hope of making things clearer!

VRonin

This post is deleted!

VRonin

can you replace theData.getCurrentColumnZeroText(index), theData.getCurrentColumnOneText(index), etc. with some constant text and see if you still get the heap corruption?

eos pengwern

@VRonin I've just tried that and I'm afraid it makes no difference.

VRonin

one more try:
turn QVector<QTreeWidgetItem *> theItemList; into QVector<QPointer< QTreeWidgetItem > > theItemList;

then after theTable -> clear(); call

QCoreApplication::processEvents();
Q_ASSERT(std::all_of(theItemList.constBegin(),theItemList.constEnd(),[](const QPointer< QTreeWidgetItem >& val)->bool {return !val;}));

eos pengwern

@VRonin Thank you; I won't be able to pursue this until later this evening, but I'll report back when I've done so.

eos pengwern

@VRonin OK, I'm suitably embarrassed about this... I've found the problem, and it wasn't in the way that I've been reconstructing the QTreeWidget. Tucked in with the redrawing code (and removed from the example code I gave above for the sake of simplicity) was a reference to an innocent-looking QVector in which I stored the values of 'index' in an order which, as it turns out, was wrong. I've corrected that error, which was leading to the QVector being addressed out-of-bounds later on. It was that out-of-bounds address later on which was precipitating the crash; I still don't understand why the debugger couldn't just tell me that, rather than giving baroque and outlandish heap errors, but there we go. Thank you for your help along the way, which made me look at the code a bit more critically and so led me to notice the error.