QTableWidget Crash in Destructor

[mclark]

I’m seeing a crash in my QTableWidget destructor when QTableModel::clearContents() is called. It is trying to delete memory (0xfeeefeee) that has already been deleted.

I have a dialog containing a QTableWidget that I partially fill with about 190 rows (6 columns, all text). I then start a notification service which will notify me when a particular device is discovered on the network. As part of handling a notification I insert another row into the table and fill it. My test case has about 14 notifications.

If, during the notifications, the table is sorted, a crash may happen when closing the dialog. I place setSortingEnabled() calls around the notification handler but to no avail. If no sorting occurs during notifications, all is fine.

What appears to be happening is that the table is sorting while QTableWidgetItems are being added during the notification handling. This leads to duplication of QTableWidget pointers, which causes the ‘deletion of table items already deleted’ problem.

Has anyone experienced this before? Am I being a dolt and forgetting to do something obvious?

MS Visual Studio 2005
Qt 4.3.4 commercial

void Net2Detect::Online( const notifystruct* pNotify )
{
    // The mutex will always be unlocked when the QMutexLocker object is
    // destroyed (when the function returns since locker is an auto variable)
    QMutexLocker locker( &m_NotifyMutex );
 
    if ( !m_bNotifierStarted )
        return;
 
    QString sType( "" );
    if ( !pNotify->type.empty() )
        sType.sprintf( "%s", pNotify->type.c_str() );
 
    if ( (sType != sDevice1) && (sType != sDevice2) && (sType != sDevice3))
        return;
 
    m_pNodeTable->setSortingEnabled( false );
    Net2Notify( pNotify );
    m_pNodeTable->setSortingEnabled( true );
}
 
void Net2Detect::Net2Notify( const notifystruct* pNotify )
{
    CIPAddr addr;
    addr.SetV4Address( pNotify->addr );
    char str[CIPAddr::ADDRSTRINGBYTES];
    CIPAddr::AddrIntoString( addr, str, false, false );
    QString sIP( reinterpret_cast<const char*>(str) );
 
    int nRow = m_pNodeTable->rowCount();
 
    m_pNodeTable->insertRow( nRow );  // Add a row to the table
 
    // Fill in Mode
    Net2TableCell* pCell = new Net2TableCell( "Net2" );
    if ( pCell == NULL )
        return;
 
    m_pNodeTable->setItem( nRow, COL0_Mode, pCell );
 
    // Fill in Name
    QString sName( "" );
    if ( !pNotify->name.empty() )
        sName.sprintf( "%s", pNotify->name.c_str() );
 
    if ( (pCell = new Net2TableCell( sName )) == NULL )
        return;
 
    m_pNodeTable->setItem( nRow, COL1_Name, pCell );
 
    .
    .
    .
 
    // Set the table cells as read-only
    Qt::ItemFlags flags = m_pNodeTable->item( nRow, COL0_Mode )->flags();
    flags ^= Qt::ItemIsEditable;
    m_pNodeTable->item( nRow, COL0_Mode )->setFlags( flags );
    m_pNodeTable->item( nRow, COL1_Name )->setFlags( flags );
    m_pNodeTable->item( nRow, COL2_IP )->setFlags( flags );
    m_pNodeTable->item( nRow, COL3_Type )->setFlags( flags );
    m_pNodeTable->item( nRow, COL4_Ver )->setFlags( flags );
    m_pNodeTable->item( nRow, COL5_CID )->setFlags( flags );
}

void Net2Detect::Online( const notifystruct* pNotify )
{
    // The mutex will always be unlocked when the QMutexLocker object is
    // destroyed (when the function returns since locker is an auto variable)
    QMutexLocker locker( &m_NotifyMutex );

    if ( !m_bNotifierStarted )
        return;

    QString sType( "" );
    if ( !pNotify->type.empty() )
        sType.sprintf( "%s", pNotify->type.c_str() );

    if ( (sType != sDevice1) && (sType != sDevice2) && (sType != sDevice3))
        return;

    m_pNodeTable->setSortingEnabled( false );
    Net2Notify( pNotify );
    m_pNodeTable->setSortingEnabled( true );
}

void Net2Detect::Net2Notify( const notifystruct* pNotify )
{
    CIPAddr addr;
    addr.SetV4Address( pNotify->addr );
    char str[CIPAddr::ADDRSTRINGBYTES];
    CIPAddr::AddrIntoString( addr, str, false, false );
    QString sIP( reinterpret_cast<const char*>(str) );

    int nRow = m_pNodeTable->rowCount();

    m_pNodeTable->insertRow( nRow );  // Add a row to the table

    // Fill in Mode
    Net2TableCell* pCell = new Net2TableCell( "Net2" );
    if ( pCell == NULL )
        return;
    
    m_pNodeTable->setItem( nRow, COL0_Mode, pCell );

    // Fill in Name
    QString sName( "" );
    if ( !pNotify->name.empty() )
        sName.sprintf( "%s", pNotify->name.c_str() );

    if ( (pCell = new Net2TableCell( sName )) == NULL )
        return;

    m_pNodeTable->setItem( nRow, COL1_Name, pCell );

    .
    .
    .

    // Set the table cells as read-only
    Qt::ItemFlags flags = m_pNodeTable->item( nRow, COL0_Mode )->flags();
    flags ^= Qt::ItemIsEditable;
    m_pNodeTable->item( nRow, COL0_Mode )->setFlags( flags );
    m_pNodeTable->item( nRow, COL1_Name )->setFlags( flags );
    m_pNodeTable->item( nRow, COL2_IP )->setFlags( flags );
    m_pNodeTable->item( nRow, COL3_Type )->setFlags( flags );
    m_pNodeTable->item( nRow, COL4_Ver )->setFlags( flags );
    m_pNodeTable->item( nRow, COL5_CID )->setFlags( flags );
}

To copy to clipboard, switch view to plain text mode 

[wysota]

Can we see the backtrace/callstack from the debugger when the crash occurs?

[mclark]

My QDialog class is Net2Detect and is called from MainWin::versionModeSwitch()

MS Visual Studio Call Stack:

QtGuid4.dll!QTableModel::clearContents() Line 753 + 0x2a bytes
QtGuid4.dll!QTableModel::clear() Line 746
QtGuid4.dll!QTableModel::~QTableModel() Line 40
QtGuid4.dll!QTableModel::'scaler deleting destructor'() + 0xf bytes
QtCored4.dll!QObjectPrivate::deleteChildren() Line 1910 + 0x24 bytes
QtGuid4.dll!QWidget::~QWidget() Line 1184
QtGuid4.dll!QFrame::~QFrame() Line 227 + 0x8 bytes
QtGuid4.dll!QAbstractScrollArea::~QAbstractScrollArea() Line 452 + 0xf bytes
QtGuid4.dll!QAbstractItemView::~QAbstractItemView() Line 466 + 0x8 bytes
QtGuid4.dll!QTableView::~QTableView() Line 481 + 0x8 bytes
QtGuid4.dll!QTableWidget::~QTableWidget() Line 1849 + 0x8 bytes
GCE.exe!Net2Detect::~Net2Detect() + 0x48 bytes
GCE.exe!MainWin::versionModeSwitch() Line2386 0xf bytes

QtGuid4.dll!QTableModel::clearContents() Line 753 + 0x2a bytes
QtGuid4.dll!QTableModel::clear() Line 746
QtGuid4.dll!QTableModel::~QTableModel() Line 40
QtGuid4.dll!QTableModel::'scaler deleting destructor'() + 0xf bytes
QtCored4.dll!QObjectPrivate::deleteChildren() Line 1910 + 0x24 bytes
QtGuid4.dll!QWidget::~QWidget() Line 1184
QtGuid4.dll!QFrame::~QFrame() Line 227 + 0x8 bytes
QtGuid4.dll!QAbstractScrollArea::~QAbstractScrollArea() Line 452 + 0xf bytes
QtGuid4.dll!QAbstractItemView::~QAbstractItemView() Line 466 + 0x8 bytes
QtGuid4.dll!QTableView::~QTableView() Line 481 + 0x8 bytes
QtGuid4.dll!QTableWidget::~QTableWidget() Line 1849 + 0x8 bytes
GCE.exe!Net2Detect::~Net2Detect() + 0x48 bytes
GCE.exe!MainWin::versionModeSwitch() Line2386 0xf bytes

To copy to clipboard, switch view to plain text mode 

void QTableModel::clearContents()
{
    for (int i = 0; i < tableItems.count(); ++i) {
        if (tableItems.at(i)) {
            tableItems.at(i)->view = 0;
            delete tableItems.at(i);  // <-- crashes here
            tableItems[i] = 0;
        }
    }
    reset();
}

void QTableModel::clearContents()
{
    for (int i = 0; i < tableItems.count(); ++i) {
        if (tableItems.at(i)) {
            tableItems.at(i)->view = 0;
            delete tableItems.at(i);  // <-- crashes here
            tableItems[i] = 0;
        }
    }
    reset();
}

To copy to clipboard, switch view to plain text mode 

[wysota]

Are you using some special kinds of items or deleting items yourself? What does versionModeSwitch() do?

[mclark]

I'm not using any special items or deleting any Qt objects myself. My table cell class only centers text, otherwise it's a generic QTableWidgetItem. Also, whether I include my '<' operator for sorting or not, I still crash in the same place.

class Net2TableCell : public QTableWidgetItem
{
public:
    Net2TableCell( const QString& text, int type = NET2_TABLECELL_TYPE ) : QTableWidgetItem( text, type )
    {
        setTextAlignment( Qt::AlignCenter );
    }
 
    // For sorting non-alpha table columns (IP Address & case insensitive name).
    bool operator<( const QTableWidgetItem& item ) const;
};
 
bool Net2TableCell::operator<( const QTableWidgetItem& item ) const
{
   bool bReturn = false;
 
   switch ( this->column() )
   {
      case COL1_Name:
         if ( !this->text().isNull() && !item.text().isNull() &&
               this->text().compare( item.text(), Qt::CaseInsensitive ) < 0 )
            bReturn = true;
         break;
      case COL2_IP:
      {
         if ( CIPAddr::StringToAddr( this->text().toAscii() ).GetV4Address() <
              CIPAddr::StringToAddr( item.text().toAscii() ).GetV4Address() )
            bReturn = true;
 
         break;
      }
      default:
         bReturn = QTableWidgetItem::operator <( item );
   }
 
   return bReturn;
}

class Net2TableCell : public QTableWidgetItem
{
public:
    Net2TableCell( const QString& text, int type = NET2_TABLECELL_TYPE ) : QTableWidgetItem( text, type )
    {
        setTextAlignment( Qt::AlignCenter );
    }

    // For sorting non-alpha table columns (IP Address & case insensitive name).
    bool operator<( const QTableWidgetItem& item ) const;
};

bool Net2TableCell::operator<( const QTableWidgetItem& item ) const
{
   bool bReturn = false;

   switch ( this->column() )
   {
      case COL1_Name:
         if ( !this->text().isNull() && !item.text().isNull() &&
               this->text().compare( item.text(), Qt::CaseInsensitive ) < 0 )
            bReturn = true;
         break;
      case COL2_IP:
      {
         if ( CIPAddr::StringToAddr( this->text().toAscii() ).GetV4Address() <
              CIPAddr::StringToAddr( item.text().toAscii() ).GetV4Address() )
            bReturn = true;

         break;
      }
      default:
         bReturn = QTableWidgetItem::operator <( item );
   }

   return bReturn;
}

To copy to clipboard, switch view to plain text mode 

void MainWin::versionModeSwitch( void )
{
   IPv4* pNetIPList = new IPv4[ m_pController->m_nSelectedIfaces ];
   if ( pNetIPList == NULL )
      return;
 
   for ( int i = 0; i < m_pController->m_nSelectedIfaces; i++ )
      pNetIPList[i] = CIPAddr::StringToAddr( m_pController->m_sCurrNetIPLst.at( i ).toAscii() ).GetV4Address();
 
   // Create the lists in advance (from the Online table)
   QStringList sNameLst, sTypeLst, sIPLst, sVerLst, sCIDLst;
 
   // Disable sorting while the table is populated, otherwise sorting
   // may interfere with the insertion order.
   m_pAllOlnTable->setSortingEnabled( false );
 
   for ( int i = 0; i < m_pAllOlnTable->rowCount(); i++ )
   {
      if ( !m_pAllOlnTable->item( i, COL_Status )->text().compare( OLN_ONLINE ) )
      {
         sNameLst.append( m_pAllOlnTable->item( i, COL_Name )->text() );
         sTypeLst.append( m_pAllOlnTable->item( i, COL_DevType )->text() );
         sIPLst.append( m_pAllOlnTable->item( i, COL_IPAddr )->text() );
         sVerLst.append( m_pAllOlnTable->GetSWVer( i ) );
         sCIDLst.append( m_pAllOlnTable->item( i, COL_CID )->text() );
      }
   }
 
   m_pAllOlnTable->setSortingEnabled( true );
 
   // Display all ACN and Net2 devices
   Net2Detect verModeSwitchDlg( this );
   verModeSwitchDlg.Initialize( sNameLst, sTypeLst, sIPLst, sVerLst, sCIDLst,
                                pNetIPList, m_pController->m_nSelectedIfaces );
   verModeSwitchDlg.exec();
 
   if ( pNetIPList != NULL )
      delete [] pNetIPList;
}

void MainWin::versionModeSwitch( void )
{
   IPv4* pNetIPList = new IPv4[ m_pController->m_nSelectedIfaces ];
   if ( pNetIPList == NULL )
      return;

   for ( int i = 0; i < m_pController->m_nSelectedIfaces; i++ )
      pNetIPList[i] = CIPAddr::StringToAddr( m_pController->m_sCurrNetIPLst.at( i ).toAscii() ).GetV4Address();

   // Create the lists in advance (from the Online table)
   QStringList sNameLst, sTypeLst, sIPLst, sVerLst, sCIDLst;

   // Disable sorting while the table is populated, otherwise sorting
   // may interfere with the insertion order.
   m_pAllOlnTable->setSortingEnabled( false );

   for ( int i = 0; i < m_pAllOlnTable->rowCount(); i++ )
   {
      if ( !m_pAllOlnTable->item( i, COL_Status )->text().compare( OLN_ONLINE ) )
      {
         sNameLst.append( m_pAllOlnTable->item( i, COL_Name )->text() );
         sTypeLst.append( m_pAllOlnTable->item( i, COL_DevType )->text() );
         sIPLst.append( m_pAllOlnTable->item( i, COL_IPAddr )->text() );
         sVerLst.append( m_pAllOlnTable->GetSWVer( i ) );
         sCIDLst.append( m_pAllOlnTable->item( i, COL_CID )->text() );
      }
   }

   m_pAllOlnTable->setSortingEnabled( true );

   // Display all ACN and Net2 devices
   Net2Detect verModeSwitchDlg( this );
   verModeSwitchDlg.Initialize( sNameLst, sTypeLst, sIPLst, sVerLst, sCIDLst,
                                pNetIPList, m_pController->m_nSelectedIfaces );
   verModeSwitchDlg.exec();

   if ( pNetIPList != NULL )
      delete [] pNetIPList;
}

To copy to clipboard, switch view to plain text mode 

[wysota]

What is the base type of pNetIPList? What happens if you comment out lines 37 and 38 in the above code?

[mclark]

What is the base type of pNetIPList?

typedef unsigned long IPv4;

What happens if you comment out lines 37 and 38 in the above code?

Commenting out these lines has no effect on the crash in QTableModel::clearContents().

[luf]

void QTableModel::clearContents()
{
    for (int i = 0; i < tableItems.count(); ++i) {
        if (tableItems.at(i)) {
            tableItems.at(i)->view = 0;
            delete tableItems.at(i);  // <-- crashes here
            tableItems[i] = 0;
        }
    }
    reset();
}

void QTableModel::clearContents()
{
    for (int i = 0; i < tableItems.count(); ++i) {
        if (tableItems.at(i)) {
            tableItems.at(i)->view = 0;
            delete tableItems.at(i);  // <-- crashes here
            tableItems[i] = 0;
        }
    }
    reset();
}

To copy to clipboard, switch view to plain text mode 

As far as I can see you are deleting a an item in a table while working on it. Which then would reindex the item (if i'm not mistaken...), making the count all different. If your count gives you 3, and you delete 1 item, i will still end up at 3, where it crashes, because 3 is now really at index 2...?

Make a QList over the items you want to delete, then go through the list and delete the items.

void QTableModel::clearContents()
{
    QList <int> deletelist;
// changed to i++, as you probably want to look at item on pos 0 as well?
    for (int i = 0; i < tableItems.count(); i++) { 
        if (tableItems.at(i)) {
            tableItems.at(i)->view = 0;
            deletelist.append(i); 
            tableItems[i] = 0;
        }
    }
    if (deletelist.isEmpty() == false)
    {
        for (int i = 0; i< deletelist.count(); i++)
        {
             delete tableItems.at(deletelist[i]);
        }
    }
    reset();
}

void QTableModel::clearContents()
{
    QList <int> deletelist;
// changed to i++, as you probably want to look at item on pos 0 as well?
    for (int i = 0; i < tableItems.count(); i++) { 
        if (tableItems.at(i)) {
            tableItems.at(i)->view = 0;
            deletelist.append(i); 
            tableItems[i] = 0;
        }
    }
    if (deletelist.isEmpty() == false)
    {
        for (int i = 0; i< deletelist.count(); i++)
        {
             delete tableItems.at(deletelist[i]);
        }
    }
    reset();
}

To copy to clipboard, switch view to plain text mode 

[mclark]

As far as I can see you are deleting a an item in a table while working on it...

The code for clearContents() is Qt code, nothing I have control over. My debugger kindly shows me where the problem is, and this is always the culprit.

The table in the dialog is stable (any sorting completed) and the cancel button pressed. This leads to the dialog, widget, view ... destructors being called, eventually crashing in clearContents().

[luf]

Hi

Sorry, my bad! I thought you had made your own subclass...

Leif

[wysota]

Are you creating any of the widgets or items on the stack?

[mclark]

Are you creating any of the widgets or items on the stack?

QTableWidget:

m_pNodeTable = new QTableWidget( 0, 6, this );

m_pNodeTable = new QTableWidget( 0, 6, this );

To copy to clipboard, switch view to plain text mode 

QTableWidgetItems:

QString sName( "" );
if ( !pNotify->name.empty() )
	sName.sprintf( "%s", pNotify->name.c_str() );
Net2TableCell* pCell = new Net2TableCell( sName );

QString sName( "" );
if ( !pNotify->name.empty() )
	sName.sprintf( "%s", pNotify->name.c_str() );
Net2TableCell* pCell = new Net2TableCell( sName );

To copy to clipboard, switch view to plain text mode 

All buttons are part of the Qt Designer *.ui file.

It's a fairly simple implementation that I've used elsewhere in the application. The difference is that here I can get notifications (that cause a row insertion) at anytime.

[jpn]

Do you have spans in the table?

[mclark]

Do you have spans in the table?

No spans; 6 cells containing only text.

[mclark]

What prevents Qt from sorting during inserts (please read the full message before answering)?

Does the following sound plausible?

1. A user presses a QTableWidget column header to initiate a sort of 215 rows of 6 cells each (1290 items).
2. I receive a notification to add a row and disable sorting BEFORE Qt has finished the previous sort.

If Qt is not done sorting when I disable sorting and start adding row items, could the item pointers become confused and account for the problems outlined above?

[wysota]

Can (2) happen from within the sort() method of the proxy or the base model? Remember that sorting is synchronous, so unless you provided your own implementation of sort(), the answer will be "no". If you are using multiple threads, then you shouldn't be accessing the same QObject from more than one thread.

[mclark]

This one appears unsolvable so I'll just have to change my design. I'll collect all notifications in the main app. When the dialog is constructed, the table will only contain a snapshot of devices known at the time of dialog instantiation.

The table in the dialog will not be updated dynamically. Not a big deal but the original problem still irritates...

Thanks to all who tried to help!

[wysota]

But do you use more than one thread in your application?

[mclark]

But do you use more than one thread in your application?

Yes, multiple threads are running in the app. In the offending dialog, a notification thread was running. The notification handler was locked using a QMutexLocker, where the mutex was declared as a class member. This should protect the table insertion code, no?

QMutexLocker locker( &m_NotifyMutex );

QMutexLocker locker( &m_NotifyMutex );

To copy to clipboard, switch view to plain text mode 

[wysota]

You can't access QObjects from more than one thread and no mutex will help here. You have to make sure that you follow that rule.