From 491b1b01e172f1963a72802278eae58a11a1fc50 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Tue, 25 Nov 2014 08:50:23 -0800 Subject: [PATCH] Be careful with QObject:::destroyed signal Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already been destroyed. --- src/uas/UAS.cc | 4 ++-- src/ui/MainWindow.cc | 3 +++ src/ui/QGCUASFileViewMulti.cc | 33 ++++++++++++++++---------------- src/ui/QGCUASFileViewMulti.h | 4 ++-- src/ui/QGCWaypointListMulti.cc | 5 ++++- src/ui/WaypointEditableView.cc | 3 +++ src/ui/designer/QGCToolWidget.cc | 5 +++-- src/ui/menuactionhelper.cpp | 5 ++++- 8 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/uas/UAS.cc b/src/uas/UAS.cc index aca75e965..e308e9476 100644 --- a/src/uas/UAS.cc +++ b/src/uas/UAS.cc @@ -3374,8 +3374,8 @@ void UAS::addLink(LinkInterface* link) void UAS::removeLink(QObject* object) { - // Be careful of the fact that by the time this signal makes it through the queue - // the link object has already been destructed. So no dynamic_cast for example. + // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already + // been destroyed. LinkInterface* link = (LinkInterface*)object; diff --git a/src/ui/MainWindow.cc b/src/ui/MainWindow.cc index e5f87c531..817b8ba8e 100644 --- a/src/ui/MainWindow.cc +++ b/src/ui/MainWindow.cc @@ -1410,6 +1410,9 @@ void MainWindow::simulateLink(bool simulate) { void MainWindow::commsWidgetDestroyed(QObject *obj) { + // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already + // been destroyed. + if (commsWidgetList.contains(obj)) { commsWidgetList.removeOne(obj); diff --git a/src/ui/QGCUASFileViewMulti.cc b/src/ui/QGCUASFileViewMulti.cc index fb627ea04..e2751625d 100644 --- a/src/ui/QGCUASFileViewMulti.cc +++ b/src/ui/QGCUASFileViewMulti.cc @@ -10,45 +10,44 @@ QGCUASFileViewMulti::QGCUASFileViewMulti(QWidget *parent) : { ui->setupUi(this); setMinimumSize(600, 80); - connect(UASManager::instance(), SIGNAL(UASCreated(UASInterface*)), this, SLOT(systemCreated(UASInterface*))); - connect(UASManager::instance(), SIGNAL(activeUASSet(int)), this, SLOT(systemSetActive(int))); + connect(UASManager::instance(), &UASManager::UASCreated, this, &QGCUASFileViewMulti::systemCreated); + connect(UASManager::instance(), SIGNAL(activeUASSet(UASInterface*)), this, SLOT(systemSetActive(UASInterface*))); if (UASManager::instance()->getActiveUAS()) { systemCreated(UASManager::instance()->getActiveUAS()); - systemSetActive(UASManager::instance()->getActiveUAS()->getUASID()); + systemSetActive(UASManager::instance()->getActiveUAS()); } } void QGCUASFileViewMulti::systemDeleted(QObject* uas) { - UASInterface* mav = dynamic_cast(uas); - if (mav) + Q_ASSERT(uas); + + // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already + // been destroyed. + + UASInterface* mav = static_cast(uas); + QGCUASFileView* list = lists.value(mav, NULL); + if (list) { - int id = mav->getUASID(); - QGCUASFileView* list = lists.value(id, NULL); - if (list) - { - delete list; - lists.remove(id); - } + delete list; + lists.remove(mav); } } void QGCUASFileViewMulti::systemCreated(UASInterface* uas) { - if (!uas) { - return; - } + Q_ASSERT(uas); QGCUASFileView* list = new QGCUASFileView(ui->stackedWidget, uas->getFileManager()); - lists.insert(uas->getUASID(), list); + lists.insert(uas, list); ui->stackedWidget->addWidget(list); // Ensure widget is deleted when system is deleted connect(uas, SIGNAL(destroyed(QObject*)), this, SLOT(systemDeleted(QObject*))); } -void QGCUASFileViewMulti::systemSetActive(int uas) +void QGCUASFileViewMulti::systemSetActive(UASInterface* uas) { QGCUASFileView* list = lists.value(uas, NULL); if (list) { diff --git a/src/ui/QGCUASFileViewMulti.h b/src/ui/QGCUASFileViewMulti.h index 6b77613ec..f9ed4f7b0 100644 --- a/src/ui/QGCUASFileViewMulti.h +++ b/src/ui/QGCUASFileViewMulti.h @@ -23,11 +23,11 @@ public: public slots: void systemDeleted(QObject* uas); void systemCreated(UASInterface* uas); - void systemSetActive(int uas); + void systemSetActive(UASInterface* uas); protected: void changeEvent(QEvent *e); - QMap lists; + QMap lists; private: Ui::QGCUASFileViewMulti *ui; diff --git a/src/ui/QGCWaypointListMulti.cc b/src/ui/QGCWaypointListMulti.cc index 752e8cdd4..ce34b7188 100644 --- a/src/ui/QGCWaypointListMulti.cc +++ b/src/ui/QGCWaypointListMulti.cc @@ -25,7 +25,10 @@ QGCWaypointListMulti::QGCWaypointListMulti(QWidget *parent) : void QGCWaypointListMulti::systemDeleted(QObject* uas) { - UASInterface* mav = dynamic_cast(uas); + // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already + // been destroyed. + + UASInterface* mav = static_cast(uas); if (mav) { int id = mav->getUASID(); diff --git a/src/ui/WaypointEditableView.cc b/src/ui/WaypointEditableView.cc index 0813598e0..45955cd16 100644 --- a/src/ui/WaypointEditableView.cc +++ b/src/ui/WaypointEditableView.cc @@ -302,6 +302,9 @@ void WaypointEditableView::initializeActionView(int actionID) void WaypointEditableView::deleted(QObject* waypoint) { + // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already + // been destroyed. + Q_UNUSED(waypoint); } diff --git a/src/ui/designer/QGCToolWidget.cc b/src/ui/designer/QGCToolWidget.cc index 57cb635a1..37a3226a9 100644 --- a/src/ui/designer/QGCToolWidget.cc +++ b/src/ui/designer/QGCToolWidget.cc @@ -561,8 +561,9 @@ void QGCToolWidget::addToolWidget(QGCToolWidgetItem* widget) void QGCToolWidget::widgetRemoved() { - //Must static cast and not dynamic cast since the object is in the destructor - //and we only want to use it as a pointer value + // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already + // been destroyed. + QGCToolWidgetItem *widget = static_cast(QObject::sender()); toolItemList.removeAll(widget); storeWidgetsToSettings(); diff --git a/src/ui/menuactionhelper.cpp b/src/ui/menuactionhelper.cpp index 459ebaade..ee75a8944 100644 --- a/src/ui/menuactionhelper.cpp +++ b/src/ui/menuactionhelper.cpp @@ -25,7 +25,10 @@ QAction *MenuActionHelper::createToolAction(const QString &title, const QString void MenuActionHelper::removeDockWidget() { - QObject *dockWidget = QObject::sender(); //Note that we can't cast to QDockWidget because we are in its destructor + // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already + // been destroyed. + + QObject *dockWidget = QObject::sender(); Q_ASSERT(dockWidget); qDebug() << "Dockwidget:" << dockWidget->objectName() << "of type" << dockWidget->metaObject()->className(); -- 2.22.0