Merge pull request #1382 from DonLakeFlyer/LinkDelete

Use QSharedPointer for cross-thread Link references

src/FactSystem/FactSystemTestBase.cc

@@ -48,7 +48,7 @@ void FactSystemTestBase::_init(MAV_AUTOPILOT autopilot)
     
     MockLink* link = new MockLink();
     link->setAutopilotType(autopilot);
-    _linkMgr->addLink(link);
+    _linkMgr->_addLink(link);
     _linkMgr->connectLink(link);
     
     // Wait for the uas to work it's way through the various threads

src/VehicleSetup/SetupViewButtons.qml

@@ -35,7 +35,7 @@ Rectangle {
 
             QGCLabel {
                 width: parent.width
-                text: "You must be connected to your board to use all available setup options."
+                text: "Full setup options are only available when connected to vehicle and full parameter list has completed downloading."
                 wrapMode: Text.WordWrap
                 horizontalAlignment: Text.AlignHCenter
             }

src/VehicleSetup/SetupViewTest.cc

@@ -64,7 +64,7 @@ void SetupViewTest::_clickThrough_test(void)
     MockLink* link = new MockLink();
     Q_CHECK_PTR(link);
     link->setAutopilotType(MAV_AUTOPILOT_PX4);
-    LinkManager::instance()->addLink(link);
+    LinkManager::instance()->_addLink(link);
     linkMgr->connectLink(link);
     QTest::qWait(5000); // Give enough time for UI to settle and heartbeats to go through
     

src/comm/LinkInterface.h

@@ -37,6 +37,7 @@ along with PIXHAWK. If not, see <http://www.gnu.org/licenses/>.
 #include <QMutex>
 #include <QMutexLocker>
 #include <QMetaType>
+#include <QSharedPointer>
 
 class LinkManager;
 class LinkConfiguration;
@@ -50,38 +51,10 @@ class LinkInterface : public QThread
 {
     Q_OBJECT
 
-    // Only LinkManager is allowed to _connect, _disconnect or delete a link
+    // Only LinkManager is allowed to create/delete or _connect/_disconnect a link
     friend class LinkManager;
 
 public:
-    LinkInterface() :
-        QThread(0),
-        _ownedByLinkManager(false),
-        _deletedByLinkManager(false),
-        _flaggedForDeletion(false)
-    {
-        // Initialize everything for the data rate calculation buffers.
-        inDataIndex  = 0;
-        outDataIndex = 0;
-
-        // Initialize our data rate buffers.
-        memset(inDataWriteAmounts, 0, sizeof(inDataWriteAmounts));
-        memset(inDataWriteTimes,   0, sizeof(inDataWriteTimes));
-        memset(outDataWriteAmounts,0, sizeof(outDataWriteAmounts));
-        memset(outDataWriteTimes,  0, sizeof(outDataWriteTimes));
-
-        qRegisterMetaType<LinkInterface*>("LinkInterface*");
-    }
-
-    /**
-     * @brief Destructor
-     * LinkManager take ownership of Links once they are added to it. Once added to LinkManager
-     * use LinkManager::deleteLink to remove if necessary.
-     **/
-    virtual ~LinkInterface() {
-        Q_ASSERT(!_ownedByLinkManager || _deletedByLinkManager);
-    }
-
     /**
      * @brief Get link configuration (if used)
      * @return A pointer to the instance of LinkConfiguration if supported. NULL otherwise.
@@ -169,7 +142,25 @@ public slots:
      * @param length The length of the data array
      **/
     virtual void writeBytes(const char *bytes, qint64 length) = 0;
-
+    
+protected:
+    // Links are only created by LinkManager so constructor is not public
+    LinkInterface() :
+        QThread(0)
+    {
+        // Initialize everything for the data rate calculation buffers.
+        inDataIndex  = 0;
+        outDataIndex = 0;
+        
+        // Initialize our data rate buffers.
+        memset(inDataWriteAmounts, 0, sizeof(inDataWriteAmounts));
+        memset(inDataWriteTimes,   0, sizeof(inDataWriteTimes));
+        memset(outDataWriteAmounts,0, sizeof(outDataWriteAmounts));
+        memset(outDataWriteTimes,  0, sizeof(outDataWriteTimes));
+        
+        qRegisterMetaType<LinkInterface*>("LinkInterface*");
+    }
+    
 signals:
 
     /**
@@ -338,10 +329,8 @@ private:
      * @return True if connection could be terminated, false otherwise
      **/
     virtual bool _disconnect(void) = 0;
-
-    bool _ownedByLinkManager;   ///< true: This link has been added to LinkManager, false: Link not added to LinkManager
-    bool _deletedByLinkManager; ///< true: Link being deleted from LinkManager, false: error, Links should only be deleted from LinkManager
-    bool _flaggedForDeletion;   ///< true: Garbage colletion ready
 };
 
+typedef QSharedPointer<LinkInterface> SharedLinkInterface;
+
 #endif // _LINKINTERFACE_H_

src/comm/LinkManager.cc

@@ -52,6 +52,7 @@ LinkManager::LinkManager(QObject* parent)
     , _configUpdateSuspended(false)
     , _configurationsLoaded(false)
     , _connectionsSuspended(false)
+    , _nullSharedLink(NULL)
 {
     connect(&_portListTimer, &QTimer::timeout, this, &LinkManager::_updateConfigurationList);
     _portListTimer.start(1000);
@@ -68,7 +69,7 @@ LinkManager::~LinkManager()
     Q_ASSERT_X(_links.count() == 0, "LinkManager", "LinkManager::_shutdown should have been called previously");
 }
 
-LinkInterface* LinkManager::createLink(LinkConfiguration* config)
+LinkInterface* LinkManager::createConnectedLink(LinkConfiguration* config)
 {
     Q_ASSERT(config);
     LinkInterface* pLink = NULL;
@@ -89,33 +90,31 @@ LinkInterface* LinkManager::createLink(LinkConfiguration* config)
 #endif
     }
     if(pLink) {
-        addLink(pLink);
+        _addLink(pLink);
+        connectLink(pLink);
     }
     return pLink;
 }
 
-LinkInterface* LinkManager::createLink(const QString& name)
+LinkInterface* LinkManager::createConnectedLink(const QString& name)
 {
     Q_ASSERT(name.isEmpty() == false);
     for(int i = 0; i < _linkConfigurations.count(); i++) {
         LinkConfiguration* conf = _linkConfigurations.at(i);
         if(conf && conf->name() == name)
-            return createLink(conf);
+            return createConnectedLink(conf);
     }
     return NULL;
 }
 
-void LinkManager::addLink(LinkInterface* link)
+void LinkManager::_addLink(LinkInterface* link)
 {
     Q_ASSERT(link);
 
-    // Take ownership for delete
-    link->_ownedByLinkManager = true;
-
     _linkListMutex.lock();
 
-    if (!_links.contains(link)) {
-        _links.append(link);
+    if (!containsLink(link)) {
+        _links.append(QSharedPointer<LinkInterface>(link));
         _linkListMutex.unlock();
         emit newLink(link);
     } else {
@@ -145,14 +144,12 @@ bool LinkManager::connectAll()
 
     bool allConnected = true;
 
-    _linkListMutex.lock();
-    foreach (LinkInterface* link, _links) {
-        Q_ASSERT(link);
-        if (!link->_connect()) {
+    foreach (SharedLinkInterface sharedLink, _links) {
+        Q_ASSERT(sharedLink.data());
+        if (!sharedLink.data()->_connect()) {
             allConnected = false;
         }
     }
-    _linkListMutex.unlock();
 
     return allConnected;
 }
@@ -161,15 +158,12 @@ bool LinkManager::disconnectAll()
 {
     bool allDisconnected = true;
 
-    _linkListMutex.lock();
-    foreach (LinkInterface* link, _links)
-    {
-        Q_ASSERT(link);
-        if (!link->_disconnect()) {
+    foreach (SharedLinkInterface sharedLink, _links) {
+        Q_ASSERT(sharedLink.data());
+        if (!sharedLink.data()->_disconnect()) {
             allDisconnected = false;
         }
     }
-    _linkListMutex.unlock();
 
     return allDisconnected;
 }
@@ -197,51 +191,34 @@ bool LinkManager::disconnectLink(LinkInterface* link)
         if(config) {
             config->setLink(NULL);
         }
-        // Link is now done and over with. We can't yet delete it because it
-        // takes a while for the MAVLink protocol to take notice of it. We
-        // flag it for delayed deletion for final clean up.
-        link->_flaggedForDeletion = true;
-        QTimer::singleShot(1000, this, &LinkManager::_delayedDeleteLink);
+        _deleteLink(link);
         return true;
     } else {
         return false;
     }
 }
 
-void LinkManager::_delayedDeleteLink()
-{
-    _linkListMutex.lock();
-    foreach (LinkInterface* link, _links)
-    {
-        Q_ASSERT(link);
-        if (link->_flaggedForDeletion) {
-            qDebug() << "Link deleted: " << link->getName();
-            _linkListMutex.unlock();
-            deleteLink(link);
-            return;
-        }
-    }
-    _linkListMutex.unlock();
-}
-
-void LinkManager::deleteLink(LinkInterface* link)
+void LinkManager::_deleteLink(LinkInterface* link)
 {
     Q_ASSERT(link);
 
     _linkListMutex.lock();
 
-    Q_ASSERT(_links.contains(link));
-    _links.removeOne(link);
-    Q_ASSERT(!_links.contains(link));
+    bool found = false;
+    for (int i=0; i<_links.count(); i++) {
+        if (_links[i].data() == link) {
+            _links.removeAt(i);
+            found = true;
+            break;
+        }
+    }
+    Q_UNUSED(found);
+    Q_ASSERT(found);
 
     _linkListMutex.unlock();
 
     // Emit removal of link
     emit linkDeleted(link);
-
-    Q_ASSERT(link->_ownedByLinkManager);
-    link->_deletedByLinkManager = true;   // Signal that this is a valid delete
-    delete link;
 }
 
 /**
@@ -249,29 +226,13 @@ void LinkManager::deleteLink(LinkInterface* link)
  */
 const QList<LinkInterface*> LinkManager::getLinks()
 {
-    _linkListMutex.lock();
-    QList<LinkInterface*> ret(_links);
-    _linkListMutex.unlock();
-    return ret;
-}
-
-const QList<SerialLink *> LinkManager::getSerialLinks()
-{
-    _linkListMutex.lock();
-    QList<SerialLink*> s;
-
-    foreach (LinkInterface* link, _links)
-    {
-        Q_ASSERT(link);
-
-        SerialLink* serialLink = qobject_cast<SerialLink*>(link);
-
-        if (serialLink)
-            s.append(serialLink);
+    QList<LinkInterface*> list;
+    
+    foreach (SharedLinkInterface sharedLink, _links) {
+        list << sharedLink.data();
     }
-    _linkListMutex.unlock();
-
-    return s;
+    
+    return list;
 }
 
 /// @brief If all new connections should be suspended a message is displayed to the user and true
@@ -296,10 +257,8 @@ void LinkManager::setConnectionsSuspended(QString reason)
 
 void LinkManager::_shutdown(void)
 {
-    QList<LinkInterface*> links = _links;
-    foreach(LinkInterface* link, links) {
-        disconnectLink(link);
-        deleteLink(link);
+    while (_links.count() != 0) {
+        disconnectLink(_links[0].data());
     }
 }
 
@@ -489,3 +448,43 @@ void LinkManager::_updateConfigurationList(void)
     }
 }
 
+bool LinkManager::containsLink(LinkInterface* link)
+{
+    bool found = false;
+    
+    foreach (SharedLinkInterface sharedLink, _links) {
+        if (sharedLink.data() == link) {
+            found = true;
+            break;
+        }
+    }
+    
+    return found;
+}
+
+bool LinkManager::anyConnectedLinks(void)
+{
+    bool found = false;
+    
+    foreach (SharedLinkInterface sharedLink, _links) {
+        if (sharedLink.data()->isConnected()) {
+            found = true;
+            break;
+        }
+    }
+    
+    return found;
+}
+
+SharedLinkInterface& LinkManager::sharedPointerForLink(LinkInterface* link)
+{
+    for (int i=0; i<_links.count(); i++) {
+        if (_links[i].data() == link) {
+            return _links[i];
+        }
+    }
+    
+    // This should never happen
+    Q_ASSERT(false);
+    return _nullSharedLink;
+}

src/comm/LinkManager.h

@@ -103,21 +103,18 @@ public:
     /// Sets the flag to allow new connections to be made
     void setConnectionsAllowed(void) { _connectionsSuspended = false; }
 
-    /// Creates (and adds) a link  based on the given configuration instance. LinkManager takes ownership of this object. To delete
-    /// it, call LinkManager::deleteLink.
-    LinkInterface* createLink(LinkConfiguration* config);
+    /// Creates, connects (and adds) a link  based on the given configuration instance.
+    LinkInterface* createConnectedLink(LinkConfiguration* config);
 
-    /// Creates (and adds) a link  based on the given configuration name. LinkManager takes ownership of this object. To delete
-    /// it, call LinkManager::deleteLink.
-    LinkInterface* createLink(const QString& name);
+    /// Creates, connects (and adds) a link  based on the given configuration name.
+    LinkInterface* createConnectedLink(const QString& name);
 
-    /// Adds the link to the LinkManager. LinkManager takes ownership of this object. To delete
-    /// it, call LinkManager::deleteLink.
-    void addLink(LinkInterface* link);
-
-    /// Deletes the specified link. Will disconnect if connected.
-    // TODO Will also crash if called. MAVLink protocol is not handling the disconnect properly.
-    void deleteLink(LinkInterface* link);
+    /// Returns true if the link manager is holding this link
+    bool containsLink(LinkInterface* link);
+    
+    /// Returns the QSharedPointer for this link. You must use SharedLinkInterface if you are going to
+    /// keep references to a link in a thread other than the main ui thread.
+    SharedLinkInterface& sharedPointerForLink(LinkInterface* link);
 
     /// Re-connects all existing links
     bool connectAll();
@@ -130,6 +127,14 @@ public:
 
     /// Disconnect the specified link
     bool disconnectLink(LinkInterface* link);
+    
+    /// Returns true if there are any connected links
+    bool anyConnectedLinks(void);
+    
+    // The following APIs are public but should not be called in normal use. The are mainly exposed
+    // here for unit test code.
+    void _deleteLink(LinkInterface* link);
+    void _addLink(LinkInterface* link);
 
 signals:
     void newLink(LinkInterface* link);
@@ -141,13 +146,12 @@ signals:
 private slots:
     void _linkConnected(void);
     void _linkDisconnected(void);
-    void _delayedDeleteLink();
 
 private:
     /// All access to LinkManager is through LinkManager::instance
     LinkManager(QObject* parent = NULL);
     ~LinkManager();
-
+    
     virtual void _shutdown(void);
 
     bool _connectionsSuspendedMsg(void);
@@ -155,7 +159,12 @@ private:
     SerialConfiguration* _findSerialConfiguration(const QString& portName);
 
     QList<LinkConfiguration*>   _linkConfigurations;    ///< List of configured links
-    QList<LinkInterface*>       _links;                 ///< List of available links
+    
+    /// List of available links kept as QSharedPointers. We use QSharedPointer since
+    /// there are other objects that maintain copies of these links in other threads.
+    /// The reference counting allows for orderly deletion.
+    QList<SharedLinkInterface>  _links;
+    
     QMutex                      _linkListMutex;         ///< Mutex for thread safe access to _links list
 
     bool    _configUpdateSuspended;                     ///< true: stop updating configuration list
@@ -163,6 +172,8 @@ private:
     bool    _connectionsSuspended;                      ///< true: all new connections should not be allowed
     QString _connectionsSuspendedReason;                ///< User visible reason for suspension
     QTimer  _portListTimer;
+    
+    SharedLinkInterface _nullSharedLink;
 };
 
 #endif

src/comm/MAVLinkProtocol.cc

@@ -175,8 +175,12 @@ void MAVLinkProtocol::_linkStatusChanged(LinkInterface* link, bool connected)
     Q_ASSERT(link);
     
     if (connected) {
-        Q_ASSERT(!_connectedLinks.contains(link));
-        _connectedLinks.append(link);
+        foreach (SharedLinkInterface sharedLink, _connectedLinks) {
+            Q_ASSERT(sharedLink.data() != link);
+        }
+        
+        // Use the same shared pointer as LinkManager
+        _connectedLinks.append(LinkManager::instance()->sharedPointerForLink(link));
         
         if (_connectedLinks.count() == 1) {
             // This is the first link, we need to start logging
@@ -192,8 +196,16 @@ void MAVLinkProtocol::_linkStatusChanged(LinkInterface* link, bool connected)
         link->writeBytes(cmd, strlen(cmd));
         link->writeBytes(init, 4);
     } else {
-        Q_ASSERT(_connectedLinks.contains(link));
-        _connectedLinks.removeOne(link);
+        bool found = false;
+        for (int i=0; i<_connectedLinks.count(); i++) {
+            if (_connectedLinks[i].data() == link) {
+                found = true;
+                _connectedLinks.removeAt(i);
+                break;
+            }
+        }
+        Q_UNUSED(found);
+        Q_ASSERT(found);
         
         if (_connectedLinks.count() == 0) {
             // Last link is gone, close out logging

src/comm/MAVLinkProtocol.h

@@ -285,7 +285,10 @@ private:
     void _startLogging(void);
     void _stopLogging(void);
     
-    QList<LinkInterface*> _connectedLinks;  ///< List of all links connected to protocol
+    /// List of all links connected to protocol. We keep SharedLinkInterface objects
+    /// which are QSharedPointer's in order to maintain reference counts across threads.
+    /// This way Link deletion works correctly.
+    QList<SharedLinkInterface> _connectedLinks;
     
     bool _logSuspendError;      ///< true: Logging suspended due to error
     bool _logSuspendReplay;     ///< true: Logging suspended due to replay

src/comm/MAVLinkSimulationLink.cc

@@ -106,7 +106,7 @@ MAVLinkSimulationLink::MAVLinkSimulationLink(QString readFile, QString writeFile
     srand(QTime::currentTime().msec());
     maxTimeNoise = 0;
     this->id = getNextLinkId();
-    LinkManager::instance()->addLink(this);
+    LinkManager::instance()->_addLink(this);
 }
 
 MAVLinkSimulationLink::~MAVLinkSimulationLink()

src/comm/SerialLink.h

@@ -102,12 +102,11 @@ private:
 class SerialLink : public LinkInterface
 {
     Q_OBJECT
+    
     friend class SerialConfiguration;
+    friend class LinkManager;
+    
 public:
-
-    SerialLink(SerialConfiguration* config);
-    ~SerialLink();
-
     // LinkInterface
 
     LinkConfiguration* getLinkConfiguration();
@@ -154,9 +153,13 @@ private slots:
     void _rerouteDisconnected(void);
 
 private:
+    // Links are only created/destroyed by LinkManager so constructor/destructor is not public
+    SerialLink(SerialConfiguration* config);
+    ~SerialLink();
+    
     // From LinkInterface
-    bool _connect(void);
-    bool _disconnect(void);
+    virtual bool _connect(void);
+    virtual bool _disconnect(void);
 
     // Internal methods
     void _emitLinkError(const QString& errorMsg);

src/comm/TCPLink.h

@@ -115,12 +115,12 @@ private:
 class TCPLink : public LinkInterface
 {
     Q_OBJECT
+    
     friend class TCPLinkUnitTest;
     friend class TCPConfiguration;
-public:
-    TCPLink(TCPConfiguration* config);
-    ~TCPLink();
+    friend class LinkManager;
     
+public:
     QTcpSocket* getSocket(void) { return _socket; }
     
     void signalBytesWritten(void);
@@ -159,9 +159,13 @@ protected:
     virtual void run(void);
     
 private:
+    // Links are only created/destroyed by LinkManager so constructor/destructor is not public
+    TCPLink(TCPConfiguration* config);
+    ~TCPLink();
+    
     // From LinkInterface
-    bool _connect(void);
-    bool _disconnect(void);
+    virtual bool _connect(void);
+    virtual bool _disconnect(void);
 
     bool _hardwareConnect();
     void _restartConnection();

src/comm/UDPLink.h

@@ -143,11 +143,11 @@ private:
 class UDPLink : public LinkInterface
 {
     Q_OBJECT
+    
     friend class UDPConfiguration;
+    friend class LinkManager;
+    
 public:
-    UDPLink(UDPConfiguration* config);
-    ~UDPLink();
-
     void requestReset() { }
     bool isConnected() const;
     QString getName() const;
@@ -192,6 +192,10 @@ protected:
     int                 _id;
 
 private:
+    // Links are only created/destroyed by LinkManager so constructor/destructor is not public
+    UDPLink(UDPConfiguration* config);
+    ~UDPLink();
+    
     // From LinkInterface
     virtual bool _connect(void);
     virtual bool _disconnect(void);

src/qgcunittest/LinkManagerTest.cc

@@ -78,7 +78,7 @@ void LinkManagerTest::_add_test(void)
     Q_ASSERT(_linkMgr->getLinks().count() == 0);
     
     MockLink* link = new MockLink();
-    _linkMgr->addLink(link);
+    _linkMgr->_addLink(link);
     
     QList<LinkInterface*> links = _linkMgr->getLinks();
     QCOMPARE(links.count(), 1);
@@ -91,8 +91,8 @@ void LinkManagerTest::_delete_test(void)
     Q_ASSERT(_linkMgr->getLinks().count() == 0);
     
     MockLink* link = new MockLink();
-    _linkMgr->addLink(link);
-    _linkMgr->deleteLink(link);
+    _linkMgr->_addLink(link);
+    _linkMgr->_deleteLink(link);
     
     QCOMPARE(_linkMgr->getLinks().count(), 0);
 }
@@ -104,7 +104,7 @@ void LinkManagerTest::_addSignals_test(void)
     Q_ASSERT(_multiSpy->checkNoSignals() == true);
     
     MockLink* link = new MockLink();
-    _linkMgr->addLink(link);
+    _linkMgr->_addLink(link);
     
     QCOMPARE(_multiSpy->checkOnlySignalByMask(newLinkSignalMask), true);
     QSignalSpy* spy = _multiSpy->getSpyByIndex(newLinkSignalIndex);
@@ -125,10 +125,10 @@ void LinkManagerTest::_deleteSignals_test(void)
     Q_ASSERT(_multiSpy->checkNoSignals() == true);
     
     MockLink* link = new MockLink();
-    _linkMgr->addLink(link);
+    _linkMgr->_addLink(link);
     _multiSpy->clearAllSignals();
     
-    _linkMgr->deleteLink(link);
+    _linkMgr->_deleteLink(link);
     
     QCOMPARE(_multiSpy->checkOnlySignalByMask(linkDeletedSignalMask), true);
     QSignalSpy* spy = _multiSpy->getSpyByIndex(linkDeletedSignalIndex);

src/qgcunittest/MainWindowTest.cc

@@ -67,7 +67,7 @@ void MainWindowTest::_connectWindowClose_test(MAV_AUTOPILOT autopilot)
     MockLink* link = new MockLink();
     Q_CHECK_PTR(link);
     link->setAutopilotType(autopilot);
-    LinkManager::instance()->addLink(link);
+    LinkManager::instance()->_addLink(link);
     linkMgr->connectLink(link);
     QTest::qWait(5000); // Give enough time for UI to settle and heartbeats to go through
     

src/qgcunittest/MavlinkLogTest.cc

@@ -140,7 +140,7 @@ void MavlinkLogTest::_connectLog_test(void)
     
     MockLink* link = new MockLink();
     Q_CHECK_PTR(link);
-    LinkManager::instance()->addLink(link);
+    LinkManager::instance()->_addLink(link);
     linkMgr->connectLink(link);
     QTest::qWait(5000); // Give enough time for UI to settle and heartbeats to go through
     

src/uas/UAS.cc

@@ -380,8 +380,7 @@ bool UAS::getSelected() const
 
 void UAS::receiveMessage(LinkInterface* link, mavlink_message_t message)
 {
-    if (!links.contains(link))
-    {
+    if (!_containsLink(link)) {
         addLink(link);
         //        qDebug() << __FILE__ << __LINE__ << "ADDED LINK!" << link->getName();
     }
@@ -873,8 +872,6 @@ void UAS::receiveMessage(LinkInterface* link, mavlink_message_t message)
             }
             positionLock = true;
             isGlobalPositionKnown = true;
-            //TODO fix this hack for forwarding of global position for patch antenna tracking
-            //forwardMessage(message);
         }
             break;
         case MAVLINK_MSG_ID_GPS_RAW_INT:
@@ -1725,49 +1722,21 @@ void UAS::sendMessage(mavlink_message_t message)
         return;
     }
 
-    if (links.count() < 1) {
+    if (_links.count() < 1) {
         qDebug() << "NO LINK AVAILABLE TO SEND!";
     }
 
     // Emit message on all links that are currently connected
-    foreach (LinkInterface* link, links) {
+    foreach (SharedLinkInterface sharedLink, _links) {
+        LinkInterface* link = sharedLink.data();
+        Q_ASSERT(link);
+        
         if (link->isConnected()) {
             sendMessage(link, message);
         }
     }
 }
 
-/**
-* Forward a message to all links that are currently connected.
-* @param message that is to be forwarded
-*/
-void UAS::forwardMessage(mavlink_message_t message)
-{
-    // Emit message on all links that are currently connected
-    QList<LinkInterface*>link_list = LinkManager::instance()->getLinks();
-
-    foreach(LinkInterface* link, link_list)
-    {
-        if (link)
-        {
-            SerialLink* serial = dynamic_cast<SerialLink*>(link);
-            if(serial != 0)
-            {
-                for(int i=0; i<links.size(); i++)
-                {
-                    if(serial != links.at(i))
-                    {
-                        if (link->isConnected()) {
-                            qDebug()<<"Antenna tracking: Forwarding Over link: "<<serial->getName()<<" "<<serial;
-                            sendMessage(serial, message);
-                        }
-                    }
-                }
-            }
-        }
-    }
-}
-
 /**
 * Send a message to the link that is connected.
 * @param link that the message will be sent to
@@ -3274,9 +3243,9 @@ const QString& UAS::getShortMode() const
 */
 void UAS::addLink(LinkInterface* link)
 {
-    if (!links.contains(link))
+    if (!_containsLink(link))
     {
-        links.append(link);
+        _links.append(LinkManager::instance()->sharedPointerForLink(link));
         qCDebug(UASLog) << "addLink:" << QString("%1").arg((ulong)link, 0, 16);
         connect(link, SIGNAL(destroyed(QObject*)), this, SLOT(removeLink(QObject*)));
     }
@@ -3285,16 +3254,16 @@ void UAS::addLink(LinkInterface* link)
 void UAS::removeLink(QObject* object)
 {
     qCDebug(UASLog) << "removeLink:" << QString("%1").arg((ulong)object, 0, 16);
-    qCDebug(UASLog) << "link count:" << links.count();
+    qCDebug(UASLog) << "link count:" << _links.count();
 
-    // Do not dynamic cast or de-reference QObject, since object is either in destructor or may have already
-    // been destroyed.
+    LinkInterface* link = dynamic_cast<LinkInterface*>(object);
 
-    LinkInterface* link = (LinkInterface*)object;
-
-    int index = links.indexOf(link);
-    Q_ASSERT(index != -1);
-    links.removeAt(index);
+    for (int i=0; i<_links.count(); i++) {
+        if (_links[i].data() == link) {
+            _links.removeAt(i);
+            break;
+        }
+    }
 }
 
 /**
@@ -3302,7 +3271,13 @@ void UAS::removeLink(QObject* object)
 */
 QList<LinkInterface*> UAS::getLinks()
 {
-    return links;
+    QList<LinkInterface*> list;
+    
+    foreach (SharedLinkInterface sharedLink, _links) {
+        list << sharedLink.data();
+    }
+    
+    return list;
 }
 
 /**
@@ -3429,3 +3404,14 @@ void UAS::unsetRCToParameterMap()
         sendMessage(message);
     }
 }
+
+bool UAS::_containsLink(LinkInterface* link)
+{
+    foreach (SharedLinkInterface sharedLink, _links) {
+        if (sharedLink.data() == link) {
+            return true;
+        }
+    }
+
+    return false;
+}

src/uas/UAS.h

@@ -341,7 +341,11 @@ protected: //COMMENTS FOR TEST UNIT
     /// LINK ID AND STATUS
     int uasId;                    ///< Unique system ID
     QMap<int, QString> components;///< IDs and names of all detected onboard components
-    QList<LinkInterface*> links;  ///< List of links this UAS can be reached by
+    
+    /// List of all links associated with this UAS. We keep SharedLinkInterface objects which are QSharedPointer's in order to
+    /// maintain reference counts across threads. This way Link deletion works correctly.
+    QList<SharedLinkInterface> _links;
+    
     QList<int> unknownPackets;    ///< Packet IDs which are unknown and have been received
     MAVLinkProtocol* mavlink;     ///< Reference to the MAVLink instance
     CommStatus commStatus;        ///< Communication status
@@ -812,9 +816,6 @@ public slots:
     /** @brief Send a message over all links this UAS can be reached with (!= all links) */
     void sendMessage(mavlink_message_t message);
 
-    /** @brief Temporary Hack for sending packets to patch Antenna. Send a message over all serial links except for this UAS's */
-    void forwardMessage(mavlink_message_t message);
-
     /** @brief Set this UAS as the system currently in focus, e.g. in the main display widgets */
     void setSelected();
 
@@ -951,6 +952,9 @@ protected slots:
     void writeSettings();
     /** @brief Read settings from disk */
     void readSettings();
+    
+private:
+    bool _containsLink(LinkInterface* link);
 };
 
 

src/ui/MainWindow.cc

@@ -642,15 +642,7 @@ void MainWindow::normalActionItemCallback()
 void MainWindow::closeEvent(QCloseEvent *event)
 {
     // Disallow window close if there are active connections
-    bool foundConnections = false;
-    foreach(LinkInterface* link, LinkManager::instance()->getLinks()) {
-        if (link->isConnected()) {
-            foundConnections = true;
-            break;
-        }
-    }
-
-    if (foundConnections) {
+    if (LinkManager::instance()->anyConnectedLinks()) {
         QGCMessageBox::StandardButton button =
             QGCMessageBox::warning(
                 tr("QGroundControl close"),
@@ -658,9 +650,7 @@ void MainWindow::closeEvent(QCloseEvent *event)
                 QMessageBox::Yes | QMessageBox::Cancel,
                 QMessageBox::Cancel);
         if (button == QMessageBox::Yes) {
-            foreach(LinkInterface* link, LinkManager::instance()->getLinks()) {
-                LinkManager::instance()->disconnectLink(link);
-            }
+            LinkManager::instance()->disconnectAll();
         } else {
             event->ignore();
             return;
@@ -669,11 +659,10 @@ void MainWindow::closeEvent(QCloseEvent *event)
 
     // This will process any remaining flight log save dialogs
     qgcApp()->processEvents(QEventLoop::ExcludeUserInputEvents);
+    
     // Should not be any active connections
-    foreach(LinkInterface* link, LinkManager::instance()->getLinks()) {
-        Q_UNUSED(link);
-        Q_ASSERT(!link->isConnected());
-    }
+    Q_ASSERT(!LinkManager::instance()->anyConnectedLinks());
+    
     _storeCurrentViewState();
     storeSettings();
     UASManager::instance()->storeSettings();
@@ -1355,11 +1344,7 @@ void MainWindow::restoreLastUsedConnection()
     if(settings.contains(key)) {
         QString connection = settings.value(key).toString();
         // Create a link for it
-        LinkInterface* link = LinkManager::instance()->createLink(connection);
-        if(link) {
-            // Connect it
-            LinkManager::instance()->connectLink(link);
-        }
+        LinkManager::instance()->createConnectedLink(connection);
     }
 }
 

src/ui/QGCLinkConfiguration.cc

@@ -104,10 +104,8 @@ void QGCLinkConfiguration::on_connectLinkButton_clicked()
                     LinkManager::instance()->disconnectLink(link);
                 }
             } else {
-                LinkInterface* link = LinkManager::instance()->createLink(config);
+                LinkInterface* link = LinkManager::instance()->createConnectedLink(config);
                 if(link) {
-                    // Connect it
-                    LinkManager::instance()->connectLink(link);
                     // Now go hunting for the parent so we can shut this down
                     QWidget* pQw = parentWidget();
                     while(pQw) {

src/ui/QGCMAVLinkLogPlayer.cc

@@ -246,18 +246,7 @@ void QGCMAVLinkLogPlayer::updatePositionSliderUi(float percent)
 void QGCMAVLinkLogPlayer::_selectLogFileForPlayback(void)
 {
     // Disallow replay when any links are connected
-    
-    bool foundConnection = false;
-    LinkManager* linkMgr = LinkManager::instance();
-    QList<LinkInterface*> links = linkMgr->getLinks();
-    foreach(LinkInterface* link, links) {
-        if (link->isConnected()) {
-            foundConnection = true;
-            break;
-        }
-    }
-    
-    if (foundConnection) {
+    if (LinkManager::instance()->anyConnectedLinks()) {
         QGCMessageBox::information(tr("Log Replay"), tr("You must close all connections prior to replaying a log."));
         return;
     }
@@ -326,7 +315,7 @@ bool QGCMAVLinkLogPlayer::loadLogFile(const QString& file)
     // If there's an existing MAVLinkSimulationLink() being used for an old file,
     // we replace it.
     if (logLink) {
-        LinkManager::instance()->deleteLink(logLink);
+        LinkManager::instance()->_deleteLink(logLink);
     }
     logLink = new MAVLinkSimulationLink("");
 

src/ui/QGCToolBar.cc

@@ -676,10 +676,8 @@ void QGCToolBar::_connectButtonClicked(bool checked)
             // Get the configuration name
             QString confName = _linkCombo->currentText();
             // Create a link for it
-            LinkInterface* link = _linkMgr->createLink(confName);
+            LinkInterface* link = _linkMgr->createConnectedLink(confName);
             if(link) {
-                // Connect it
-                _linkMgr->connectLink(link);
                 // Save last used connection
                 MainWindow::instance()->saveLastUsedConnection(confName);
             }

src/ui/toolbar/MainToolBar.cc

@@ -163,10 +163,8 @@ void MainToolBar::onConnect(QString conf)
             // We don't want the combo box updating under our feet
             LinkManager::instance()->suspendConfigurationUpdates(true);
             // Create a link
-            LinkInterface* link = LinkManager::instance()->createLink(_currentConfig);
+            LinkInterface* link = LinkManager::instance()->createConnectedLink(_currentConfig);
             if(link) {
-                // Connect it
-                LinkManager::instance()->connectLink(link);
                 // Save last used connection
                 MainWindow::instance()->saveLastUsedConnection(_currentConfig);
             }

src/ui/uas/UASListWidget.cc

@@ -118,7 +118,7 @@ void UASListWidget::updateStatus()
         LinkInterface* link = i.key();
 
         // Paranoid sanity check
-        if (!LinkManager::instance()->getLinks().contains(link))
+        if (!LinkManager::instance()->containsLink(link))
             continue;
 
         if (!link)