Delete on inherited QWidget crashes
-
Also, use void QObject::deleteLater() [slot] for better deletion of the QWidget. When a pointer needs to be reset, add the signal/slot connection in your parent class to handle the void QObject::destroyed(QObject * obj = 0) [signal]
and set your QPointer to null if called.
Greetz -
Yes, I just use
@
if (ptr_featureOverview)
{
delete ptr_featureOverview;
ptr_featureOverview = 0;
}
@I think I may have to go the show/hide route.
-
[quote author="raven-worx" date="1381139538"]did i understand you right:
You hold a pointer to the parent widget? And delete the parent widget out of the client widget?[/quote]No, I hold a pointer to the child widget within the parent widget class.
-
can you please post the implementation of your overview widget class?
-
OK, I will go with this before I try show/hide.
[quote author="Jeroentje@home" date="1381139651"]Also, use void QObject::deleteLater() [slot] for better deletion of the QWidget. When a pointer needs to be reset, add the signal/slot connection in your parent class to handle the void QObject::destroyed(QObject * obj = 0) [signal]
and set your QPointer to null if called.
Greetz[/quote] -
Here's the cpp
@
#include "cfeatureoverview.h"CFeatureOverview::CFeatureOverview(QWidget *parent, long lFeatureID) :
QWidget(parent),
lFeatureID(lFeatureID)
{
qDebug() << "constructor IN" << this;this->lFeatureID = lFeatureID; this->setMouseTracking(true); // Set up a connection to get loaded item data (single item). QObject::connect(ptr_oHttp, SIGNAL(symbolDataLoaded(QString)), this, SLOT(symbolDataLoaded(QString))); this->setGeometry(parent->width() - 350, 50, 250, 0); QPalette pal(palette()); QColor bkgrnd; bkgrnd.setRgb(0, 0, 0, 127); pal.setColor(QPalette::Background, bkgrnd); pal.setColor(QPalette::Foreground, Qt::white); this->setAutoFillBackground(true); this->setPalette(pal); this->hide();
}
CFeatureOverview::~CFeatureOverview()
{
qDebug() << "destructor IN" << this;
}void CFeatureOverview::symbolDataLoaded(QString sXml)
{
CDataConnector oDataConnector;
oDataConnector.readXml(sXml.toStdString());//qDebug() << "oDataConnector.v_ptr_oDataSheet.size()=" << oDataConnector.v_ptr_oDataSheet.size(); QFont fontitalic; fontitalic.setFamily("Tahoma"); fontitalic.setItalic(true); fontitalic.setPointSize(6); QGridLayout* layout = new QGridLayout; QFont font; font.setFamily("Tahoma"); font.setPointSize(7); QFont fontbold; fontbold.setFamily("Tahoma"); fontbold.setPointSize(7); fontbold.setBold(true); QLabel* labelIdentification = new QLabel(QString::fromStdString(oDataConnector.getIdentification())); labelIdentification->setFont(fontbold); QColor bkgrnd; bkgrnd.setRgb(255, 255, 255, 127); QPalette palID; palID.setColor(QPalette::Background, bkgrnd); palID.setColor(QPalette::Foreground, Qt::black); labelIdentification->setPalette(palID); labelIdentification->setAutoFillBackground(true); labelIdentification->setMinimumWidth(this->width() - 20); labelIdentification->setAlignment(Qt::AlignCenter); layout->addWidget(labelIdentification, 0, 0, 1, 2, Qt::AlignHCenter); int iRow = 1; for (int iSheet = 0; iSheet < oDataConnector.v_ptr_oDataSheet.size(); iSheet++) { //cout << oDataConnector.v_ptr_oDataSheet[iSheet]->getName(); string sAdjacentTitle = ""; string sAdjacentValue = ""; for (vector<CDataComponent*>::iterator compi = oDataConnector.v_ptr_oDataSheet[iSheet]->vComponents.begin(); compi != oDataConnector.v_ptr_oDataSheet[iSheet]->vComponents.end(); ++compi) { CDataComponent* ptr_oDataComponent = *compi; //qDebug() << QString::fromStdString(ptr_oDataComponent->getName()) << ptr_oDataComponent->getFeatureOverview(); if (ptr_oDataComponent->getFeatureOverview()) { string sValue = ""; if (ptr_oDataComponent->getType() == "dropdown" || ptr_oDataComponent->getType() == "dropdownsearch") { CDataComponentDropDown* ptr_oDropDown = static_cast<CDataComponentDropDown*>(ptr_oDataComponent); sValue = ptr_oDropDown->getText(ptr_oDropDown->getValue()); } else sValue = ptr_oDataComponent->getValue(); if (!oDataConnector.v_ptr_oDataSheet[iSheet]->getAdjacentControls()) // We aren't in an adjacent control sheet, so add anyway. { QLabel* labelTitle = new QLabel(QString::fromStdString(ptr_oDataComponent->getName())); labelTitle->setFont(fontbold); QLabel* labelValue = new QLabel(QString::fromStdString(sValue)); labelValue->setFont(font); layout->addWidget(labelTitle, iRow, 0); layout->addWidget(labelValue, iRow, 1); iRow++; } else // We are in an adjacent control sheet, just remember the values. { sAdjacentTitle = oDataConnector.v_ptr_oDataSheet[iSheet]->getName(); if (sAdjacentValue != "") sAdjacentValue += " "; sAdjacentValue += sValue; } } } if (oDataConnector.v_ptr_oDataSheet[iSheet]->getAdjacentControls() && sAdjacentValue != "") { QLabel* labelTitle = new QLabel(QString::fromStdString(sAdjacentTitle)); labelTitle->setFont(fontbold); QLabel* labelValue = new QLabel(QString::fromStdString(sAdjacentValue)); labelValue->setFont(font); layout->addWidget(labelTitle, iRow, 0); layout->addWidget(labelValue, iRow, 1); iRow++; } } iRow++; QLabel* labelSnapshot = new QLabel("Snapshot Timestamp: " + QString::fromStdString(oDataConnector.getSnapshotDateTime())); labelSnapshot->setFont(fontitalic); layout->addWidget(labelSnapshot, iRow, 0, 1, 2, Qt::AlignRight); this->setGeometry(this->pos().x(), this->pos().y(), this->width(), (iRow + 1) * 20); this->setLayout(layout); this->show(); //qDebug() << QString::fromStdString(oDataConnector.ptr_oDataSheet->vComponents);
}
@ -
Heres the .h
@
#ifndef CFEATUREOVERVIEW_H
#define CFEATUREOVERVIEW_H// C++ includes.
#include <cdataconnector.h>
#include <chttp.h>
//#include <cmapviewport.h>// Qt includes.
#include <QWidget>
#include <QDebug>
#include <QLabel>
#include <QGridLayout>extern CHttp* ptr_oHttp;
namespace Ui {
class CFeatureOverview;
}class CFeatureOverview : public QWidget
{
Q_OBJECTprivate:
long lFeatureID;public:
explicit CFeatureOverview(QWidget *parent = 0, long lFeatureID = 0);
~CFeatureOverview();
long getFeatureID() { return (lFeatureID); }public slots:
void symbolDataLoaded(QString sXml);protected:
};
#endif // CFEATUREOVERVIEW_H
@ -
Here is where its initialised:
@void StreetTrack::showFeatureOverview(long lFeatureID)
{
qDebug() << lFeatureID << ptr_featureOverview;
if (lFeatureID == 0) // Just delete previous overview if one exists.
{if (ptr_featureOverview) { //qDebug() << ptr_featureOverview->width(); qDebug() << "A"; delete ptr_featureOverview; ptr_featureOverview = 0; qDebug() << "Ashould be zero!"; qDebug() << "Ashould be zero" << ptr_featureOverview; //qDebug() << "Deleted that for you."; } return; } if (ptr_featureOverview) { if (ptr_featureOverview->getFeatureID() == lFeatureID) return; // Already loaded. } qDebug() << "B"; if (ptr_featureOverview) delete ptr_featureOverview; ptr_featureOverview = 0; qDebug() << "Bshould be zero" << ptr_featureOverview; // Check there is no feature view open. if (ptr_featureView) return; ptr_featureOverview = new CFeatureOverview(this, lFeatureID); ptr_oHttp->downloadSymbolData(lFeatureID);
}
@ -
you forgot the braces of your if statement:
@
if (ptr_featureOverview)
{
delete ptr_featureOverview;
ptr_featureOverview = 0;
}
@
in your showFeatureOverview() method. So it's deleted but not set to null. Thus it keeps pointing to an invalid memory address which will cause the crash the next time the method is executed. QPointer would take care of this.But since it seems that you execute this method quite often it would be better to use a single instance and use show/hide (as you already stated), instead of creating and destroying a instance every time. This is way cheaper than the current way.
-
OK, good catch! Didn't solve it though.
Will go the show/hide route.
[quote author="raven-worx" date="1381140770"]you forgot the braces of your if statement:
@
if (ptr_featureOverview)
{
delete ptr_featureOverview;
ptr_featureOverview = 0;
}
@
in your showFeatureOverview() method. So it's deleted but not set to null. Thus it keeps pointing to an invalid memory address which will cause the crash the next time the method is executed. QPointer would take care of this.But since it seems that you execute this method quite often it would be better to use a single instance and use show/hide (as you already stated), instead of creating and destroying a instance every time. This is way cheaper than the current way.[/quote]