Merge pull request #998 from DonLakeFlyer/Destroyed

Be careful with QObject:::destroyed signal

src/uas/UAS.cc

@@ -3352,8 +3352,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;
     

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);

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<UASInterface*>(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<UASInterface*>(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) {

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<int, QGCUASFileView*> lists;
+    QMap<UASInterface*, QGCUASFileView*> lists;
 
 private:
     Ui::QGCUASFileViewMulti *ui;

src/ui/QGCWaypointListMulti.cc

@@ -25,7 +25,10 @@ QGCWaypointListMulti::QGCWaypointListMulti(QWidget *parent) :
 
 void QGCWaypointListMulti::systemDeleted(QObject* uas)
 {
-    UASInterface* mav = dynamic_cast<UASInterface*>(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<UASInterface*>(uas);
     if (mav)
     {
         int id = mav->getUASID();

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);
 }
 

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<QGCToolWidgetItem *>(QObject::sender());
     toolItemList.removeAll(widget);
     storeWidgetsToSettings();

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();