Fix for [OP-1078]: GCS segfaults if you close it after playing a log file

ground/openpilotgcs/src/plugins/logging/logfile.cpp

@@ -3,7 +3,11 @@
 #include <QtGlobal>
 
 LogFile::LogFile(QObject *parent) :
-    QIODevice(parent)
+    QIODevice(parent),
+    lastTimeStamp(0),
+    lastPlayed(0),
+    timeOffset(0),
+    playbackSpeed(1.0)
 {
     connect(&timer, SIGNAL(timeout()), this, SLOT(timerFired()));
 }
@@ -97,7 +101,7 @@ void LogFile::timerFired()
         // TODO: going back in time will be a problem
         while ((lastPlayed + ((time - timeOffset) * playbackSpeed) > lastTimeStamp)) {
             lastPlayed += ((time - timeOffset) * playbackSpeed);
-            if (file.bytesAvailable() < 4) {
+            if (file.bytesAvailable() < sizeof(dataSize)) {
                 stopReplay();
                 return;
             }
@@ -109,6 +113,7 @@ void LogFile::timerFired()
                 stopReplay();
                 return;
             }
+
             if (file.bytesAvailable() < dataSize) {
                 stopReplay();
                 return;
@@ -117,9 +122,10 @@ void LogFile::timerFired()
             mutex.lock();
             dataBuffer.append(file.read(dataSize));
             mutex.unlock();
+
             emit readyRead();
 
-            if (file.bytesAvailable() < 4) {
+            if (file.bytesAvailable() < sizeof(lastTimeStamp)) {
                 stopReplay();
                 return;
             }
@@ -127,7 +133,7 @@ void LogFile::timerFired()
             int save = lastTimeStamp;
             file.read((char *)&lastTimeStamp, sizeof(lastTimeStamp));
             // some validity checks
-            if (lastTimeStamp < save // logfile goies back in time
+            if (lastTimeStamp < save // logfile goes back in time
                 || (lastTimeStamp - save) > (60 * 60 * 1000)) { // gap of more than 60 minutes)
                 qDebug() << "Error: Logfile corrupted! Unlikely timestamp " << lastTimeStamp << " after " << save << "\n";
                 stopReplay();
@@ -148,7 +154,6 @@ bool LogFile::startReplay()
     myTime.restart();
     timeOffset    = 0;
     lastPlayed    = 0;
-    playbackSpeed = 1;
     file.read((char *)&lastTimeStamp, sizeof(lastTimeStamp));
     timer.setInterval(10);
     timer.start();

ground/openpilotgcs/src/plugins/logging/logfile.h

@@ -34,7 +34,8 @@ public:
 public slots:
     void setReplaySpeed(double val)
     {
-        playbackSpeed = val; qDebug() << playbackSpeed;
+        playbackSpeed = val;
+        qDebug() << "Playback speed is now" << playbackSpeed;
     };
     void pauseReplay();
     void resumeReplay();

ground/openpilotgcs/src/plugins/logging/loggingplugin.cpp

@@ -44,7 +44,9 @@
 #include "uavobjectmanager.h"
 
 
-LoggingConnection::LoggingConnection()
+LoggingConnection::LoggingConnection(LoggingPlugin *loggingPlugin):
+loggingPlugin(loggingPlugin),
+m_deviceOpened(false)
 {}
 
 LoggingConnection::~LoggingConnection()
@@ -68,11 +70,9 @@ QList <Core::IConnection::device> LoggingConnection::availableDevices()
 
 QIODevice *LoggingConnection::openDevice(const QString &deviceName)
 {
-    Q_UNUSED(deviceName)
+    loggingPlugin->stopLogging();
+    closeDevice(deviceName);
 
-    if (logFile.isOpen()) {
-        logFile.close();
-    }
     QString fileName = QFileDialog::getOpenFileName(NULL, tr("Open file"), QString(""), tr("OpenPilot Log (*.opl)"));
     if (!fileName.isNull()) {
         startReplay(fileName);
@@ -112,6 +112,11 @@ QString LoggingConnection::shortName()
 }
 
 
+LoggingThread::~LoggingThread()
+{
+    stopLogging();
+}
+
 /**
  * Sets the file to use for logging and takes the parent plugin
  * to connect to stop logging signal
@@ -282,16 +287,19 @@ void LoggingThread::transactionCompleted(UAVObject *obj, bool success)
  ********************************/
 
 
-LoggingPlugin::LoggingPlugin() : state(IDLE)
+LoggingPlugin::LoggingPlugin() :
+state(IDLE),
+loggingThread(NULL),
+logConnection(new LoggingConnection(this)),
+mf(NULL),
+cmd(NULL)
 {
-    logConnection = new LoggingConnection();
+
 }
 
 LoggingPlugin::~LoggingPlugin()
 {
-    if (loggingThread) {
+    delete loggingThread;
-        delete loggingThread;
-    }
 
     // Don't delete it, the plugin manager will do it:
     // delete logConnection;
@@ -307,7 +315,6 @@ bool LoggingPlugin::initialize(const QStringList & args, QString *errMsg)
 
     loggingThread = NULL;
 
-
     // Add Menu entry
     Core::ActionManager *am   = Core::ICore::instance()->actionManager();
     Core::ActionContainer *ac = am->actionContainer(Core::Constants::M_TOOLS);
@@ -406,7 +413,7 @@ void LoggingPlugin::loggingStopped()
 
     emit stateChanged("IDLE");
 
-    free(loggingThread);
+    delete loggingThread;
     loggingThread = NULL;
 }
 

ground/openpilotgcs/src/plugins/logging/loggingplugin.h

@@ -53,7 +53,7 @@ class LoggingConnection
     : public Core::IConnection {
     Q_OBJECT
 public:
-    LoggingConnection();
+    LoggingConnection(LoggingPlugin *loggingPlugin);
     virtual ~LoggingConnection();
 
     virtual QList <Core::IConnection::device> availableDevices();
@@ -75,6 +75,7 @@ public:
 
 private:
     LogFile logFile;
+    LoggingPlugin *loggingPlugin;
 
 
 protected slots:
@@ -89,6 +90,8 @@ protected:
 class LoggingThread : public QThread {
     Q_OBJECT
 public:
+    virtual ~LoggingThread();
+
     bool openFile(QString file, LoggingPlugin *parent);
 
 private slots:
@@ -115,6 +118,8 @@ class LoggingPlugin : public ExtensionSystem::IPlugin {
     Q_OBJECT
     Q_PLUGIN_METADATA(IID "OpenPilot.Logging")
 
+    friend class LoggingConnection;
+
 public:
     LoggingPlugin();
     ~LoggingPlugin();