QMutableListIterator remove() crashing on Windows

[Valheru]

Compiled with Visual Studio 2008, an implementation iterating thought a QModelIndexList (obtained using QItemSelectionModel::selectedRows() ) using QMutableListIterator causes a crash when calling QMutableListIterator::remove(). This only occurs on Windows, using GCC on Linux (haven't tested MinGW on Windows) causes no crash. The error when it crashes on Windows is a genereal heap corruption error, and the debugger crashes exactly on that line. The list itself is valid, and this occurs right after program startup, so I am 99% certain that the heap isn't becoming corrupted earlier on (No delete operations have been done whatsoever on the list before this, and it deisplays perfectly in the model without crashing, which you would expect it to do if there had been any ehap corruption before this).
Is this a (known) bug in Qt or maybe in the MSVC compiler? Has anyone encountered this before?

[wysota]

Can we see the exact code used to manipulate the container using the iterator?

[Valheru]

Yes, of course:

QList< File* > BaseModel::cleanSelection( QModelIndexList &selection ) const
{
    /* We want to filter the index list here, since we cannot allow a selection to contain both
    root items and children of those root items. Here we traverse the list, filtering out any
    children who's parents are also in the list */
 
    QMutableListIterator< QModelIndex > it( selection );
    QList< File* > files;
 
    while( it.hasNext() ){
        QModelIndex idx = it.next();
        BaseType *base = static_cast< BaseType* >( idx.internalPointer() );
 
        if( base->type() == "File" ){
            File *file = dynamic_cast< File* >( base );
 
            if( file ){
                //If the current files parent is also in the list, then we don't want to process it
                if( selection.indexOf( index( downloadqueue->indexOf( file->parent() ), 0 ) ) != -1 ){
                    files.append( file );
                    it.remove();
                }
            }
        }
    }
 
    return files;
}

QList< File* > BaseModel::cleanSelection( QModelIndexList &selection ) const
{
    /* We want to filter the index list here, since we cannot allow a selection to contain both
    root items and children of those root items. Here we traverse the list, filtering out any
    children who's parents are also in the list */

    QMutableListIterator< QModelIndex > it( selection );
    QList< File* > files;

    while( it.hasNext() ){
        QModelIndex idx = it.next();
        BaseType *base = static_cast< BaseType* >( idx.internalPointer() );

        if( base->type() == "File" ){
            File *file = dynamic_cast< File* >( base );

            if( file ){
                //If the current files parent is also in the list, then we don't want to process it
                if( selection.indexOf( index( downloadqueue->indexOf( file->parent() ), 0 ) ) != -1 ){
                    files.append( file );
                    it.remove();
                }
            }
        }
    }

    return files;
}

To copy to clipboard, switch view to plain text mode 

It crashes on line 21, it.remove().
This code is being called from here:

void KNewzModel::moveToTop()
{
    QModelIndexList selection = view->selectionModel()->selectedRows();
 
    if( selection.size() < 1 )
        return;
 
    QList< File* > files = cleanSelection( selection );
    QMutexLocker lock( &downloadqueue->mutex() );
    QModelIndex rootIdx = index( 0, 0 );
    QModelIndex childIdx = index( 0, 0, rootIdx );
 
    foreach( const QModelIndex &idx, selection ){
        BaseType *base = static_cast< BaseType* >( idx.internalPointer() );
 
        if( base->type() == "NzbFile" ){
            NzbFile *nzbFile = dynamic_cast< NzbFile* >( base );
 
            if( !nzbFile )
                break;
 
            //No need to move the first item to the top, it's there already
            if( nzbFile == downloadqueue->first() )
                continue;
 
            int row = downloadqueue->indexOf( nzbFile );
            Q_ASSERT( row >= 0 && row < downloadqueue->size() );
            QModelIndex idx = index( row, 0 );
            bool expanded = view->isExpanded( idx );
            beginRemoveRows( QModelIndex(), row, row );
            nzbFile = downloadqueue->takeAt( row );
            int rows = nzbFile->size();
            endRemoveRows();
            row = rootIdx.row();
            beginInsertRows( QModelIndex(), row, row );
            downloadqueue->insert( row, nzbFile );
            endInsertRows();
            QModelIndex newIdx = index( row, 0 );
            emit dataChanged( newIdx, index( rows, columnCount(), newIdx ) );
 
            if( files.size() > 0 ){
                File *firstChild = nzbFile->first();
 
                /* Check if any children were selected. We can't just ignore them now,
                    because this action can legally operate on both children and their top-level
                    parent simultaneously */
                QMutableListIterator< File* > it( files );
 
                while( it.hasNext() ){
                    File *file = it.next();
 
                    if( file->parent() == nzbFile ){
                        it.remove();
                        int row = nzbFile->indexOf( file );
 
                        if( row >= 0 && row < nzbFile->size() ){
                            beginRemoveRows( newIdx, row, row );
                            file = nzbFile->takeAt( row );
                            endRemoveRows();
                            row = nzbFile->indexOf( firstChild );
 
                            if( row < 0 )
                                row = 0;
 
                            beginInsertRows( newIdx, row, row );
                            nzbFile->insert( row, file );
                            endInsertRows();
                            emit dataChanged( index( row, 0, newIdx ), index( row, columnCount(), newIdx ) );
                        }
                    }
                }
            }
 
            if( expanded )
                view->setExpanded( newIdx, true );
        }else{
            File *file = dynamic_cast< File* >( base );
            NzbFile *nzbFile = file->parent();
            int row = nzbFile->indexOf( file );
 
            /* We want row > 0 here because we don't need to move the first child
               to the top since it's already there */
            if( row > 0 && row < nzbFile->size() ){
                QModelIndex parentIdx = index( downloadqueue->indexOf( nzbFile ), 0 );
                Q_ASSERT( parentIdx.isValid() );
                beginRemoveRows( parentIdx, row, row );
                file = nzbFile->takeAt( row );
                endRemoveRows();
                beginInsertRows( parentIdx, 0, 0 );
                nzbFile->prepend( file );
                endInsertRows();
                emit dataChanged( index( 0, 0, parentIdx ), index( 0, columnCount(), parentIdx ) );
            }
        }
 
    }
 
    view->selectionModel()->clearSelection();
}

void KNewzModel::moveToTop()
{
    QModelIndexList selection = view->selectionModel()->selectedRows();

    if( selection.size() < 1 )
        return;

    QList< File* > files = cleanSelection( selection );
    QMutexLocker lock( &downloadqueue->mutex() );
    QModelIndex rootIdx = index( 0, 0 );
    QModelIndex childIdx = index( 0, 0, rootIdx );

    foreach( const QModelIndex &idx, selection ){
        BaseType *base = static_cast< BaseType* >( idx.internalPointer() );

        if( base->type() == "NzbFile" ){
            NzbFile *nzbFile = dynamic_cast< NzbFile* >( base );

            if( !nzbFile )
                break;

            //No need to move the first item to the top, it's there already
            if( nzbFile == downloadqueue->first() )
                continue;

            int row = downloadqueue->indexOf( nzbFile );
            Q_ASSERT( row >= 0 && row < downloadqueue->size() );
            QModelIndex idx = index( row, 0 );
            bool expanded = view->isExpanded( idx );
            beginRemoveRows( QModelIndex(), row, row );
            nzbFile = downloadqueue->takeAt( row );
            int rows = nzbFile->size();
            endRemoveRows();
            row = rootIdx.row();
            beginInsertRows( QModelIndex(), row, row );
            downloadqueue->insert( row, nzbFile );
            endInsertRows();
            QModelIndex newIdx = index( row, 0 );
            emit dataChanged( newIdx, index( rows, columnCount(), newIdx ) );

            if( files.size() > 0 ){
                File *firstChild = nzbFile->first();

                /* Check if any children were selected. We can't just ignore them now,
                    because this action can legally operate on both children and their top-level
                    parent simultaneously */
                QMutableListIterator< File* > it( files );

                while( it.hasNext() ){
                    File *file = it.next();

                    if( file->parent() == nzbFile ){
                        it.remove();
                        int row = nzbFile->indexOf( file );

                        if( row >= 0 && row < nzbFile->size() ){
                            beginRemoveRows( newIdx, row, row );
                            file = nzbFile->takeAt( row );
                            endRemoveRows();
                            row = nzbFile->indexOf( firstChild );

                            if( row < 0 )
                                row = 0;

                            beginInsertRows( newIdx, row, row );
                            nzbFile->insert( row, file );
                            endInsertRows();
                            emit dataChanged( index( row, 0, newIdx ), index( row, columnCount(), newIdx ) );
                        }
                    }
                }
            }

            if( expanded )
                view->setExpanded( newIdx, true );
        }else{
            File *file = dynamic_cast< File* >( base );
            NzbFile *nzbFile = file->parent();
            int row = nzbFile->indexOf( file );

            /* We want row > 0 here because we don't need to move the first child
               to the top since it's already there */
            if( row > 0 && row < nzbFile->size() ){
                QModelIndex parentIdx = index( downloadqueue->indexOf( nzbFile ), 0 );
                Q_ASSERT( parentIdx.isValid() );
                beginRemoveRows( parentIdx, row, row );
                file = nzbFile->takeAt( row );
                endRemoveRows();
                beginInsertRows( parentIdx, 0, 0 );
                nzbFile->prepend( file );
                endInsertRows();
                emit dataChanged( index( 0, 0, parentIdx ), index( 0, columnCount(), parentIdx ) );
            }
        }

    }

    view->selectionModel()->clearSelection();
}

To copy to clipboard, switch view to plain text mode