SingleApplication: Refactor startup code

3rdparty/singleapplication/singleapplication_p.cpp

@@ -80,27 +80,24 @@ SingleApplicationPrivateClass::SingleApplicationPrivateClass(SingleApplicationCl
 
 SingleApplicationPrivateClass::~SingleApplicationPrivateClass() {
 
-  if (socket_ != nullptr) {
+  if (socket_ != nullptr && socket_->isOpen()) {
     socket_->close();
-    delete socket_;
-    socket_ = nullptr;
   }
 
   if (memory_ != nullptr) {
     memory_->lock();
-    InstancesInfo *instance = static_cast<InstancesInfo*>(memory_->data());
     if (server_ != nullptr) {
       server_->close();
-      delete server_;
+      InstancesInfo *instance = static_cast<InstancesInfo*>(memory_->data());
       instance->primary = false;
       instance->primaryPid = -1;
       instance->primaryUser[0] = '\0';
       instance->checksum = blockChecksum();
     }
     memory_->unlock();
-
-    delete memory_;
-    memory_ = nullptr;
+    if (memory_->isAttached()) {
+      memory_->detach();
+    }
   }
 
 }
@@ -203,7 +200,7 @@ void SingleApplicationPrivateClass::startPrimary() {
   // Successful creation means that no main process exists
   // So we start a QLocalServer to listen for connections
   QLocalServer::removeServer(blockServerName_);
-  server_ = new QLocalServer();
+  server_ = new QLocalServer(this);
 
   // Restrict access to the socket according to the SingleApplication::Mode::User flag on User level or no restrictions
   if (options_ & SingleApplicationClass::Mode::User) {
@@ -230,16 +227,16 @@ void SingleApplicationPrivateClass::startSecondary() {
 
 bool SingleApplicationPrivateClass::connectToPrimary(const int timeout, const ConnectionType connectionType) {
 
-  QElapsedTimer time;
-  time.start();
-
   // Connect to the Local Server of the Primary Instance if not already connected.
   if (socket_ == nullptr) {
-    socket_ = new QLocalSocket();
+    socket_ = new QLocalSocket(this);
   }
 
   if (socket_->state() == QLocalSocket::ConnectedState) return true;
 
+  QElapsedTimer time;
+  time.start();
+
   if (socket_->state() != QLocalSocket::ConnectedState) {
 
     forever {
@@ -261,7 +258,7 @@ bool SingleApplicationPrivateClass::connectToPrimary(const int timeout, const Co
     }
   }
 
-  // Initialisation message according to the SingleApplication protocol
+  // Initialization message according to the SingleApplication protocol
   QByteArray initMsg;
   QDataStream writeStream(&initMsg, QIODevice::WriteOnly);
   writeStream.setVersion(QDataStream::Qt_5_8);

3rdparty/singleapplication/singleapplication_p.h

@@ -45,25 +45,13 @@ class QLocalServer;
 class QLocalSocket;
 class QSharedMemory;
 
-struct InstancesInfo {
-  bool primary;
-  quint32 secondary;
-  qint64 primaryPid;
-  char primaryUser[128];
-  quint16 checksum;
-};
-
-struct ConnectionInfo {
-  explicit ConnectionInfo() : msgLen(0), instanceId(0), stage(0) {}
-  quint64 msgLen;
-  quint32 instanceId;
-  quint8 stage;
-};
-
 class SingleApplicationPrivateClass : public QObject {
   Q_OBJECT
 
  public:
+  explicit SingleApplicationPrivateClass(SingleApplicationClass *ptr);
+  ~SingleApplicationPrivateClass() override;
+
   enum ConnectionType : quint8 {
     InvalidConnection = 0,
     NewInstance = 1,
@@ -74,12 +62,25 @@ class SingleApplicationPrivateClass : public QObject {
     StageInitHeader = 0,
     StageInitBody = 1,
     StageConnectedHeader = 2,
-    StageConnectedBody = 3,
+    StageConnectedBody = 3
   };
   Q_DECLARE_PUBLIC(SingleApplicationClass)
 
-  explicit SingleApplicationPrivateClass(SingleApplicationClass *ptr);
-  ~SingleApplicationPrivateClass() override;
+  struct InstancesInfo {
+    explicit InstancesInfo() : primary(false), secondary(0), primaryPid(0), checksum(0) {}
+    bool primary;
+    quint32 secondary;
+    qint64 primaryPid;
+    char primaryUser[128];
+    quint16 checksum;
+  };
+
+  struct ConnectionInfo {
+    explicit ConnectionInfo() : msgLen(0), instanceId(0), stage(0) {}
+    quint64 msgLen;
+    quint32 instanceId;
+    quint8 stage;
+  };
 
   static QString getUsername();
   void genBlockServerName();

3rdparty/singleapplication/singleapplication_t.cpp

@@ -34,6 +34,8 @@
 #include <cstdlib>
 #include <limits>
 
+#include <boost/scope_exit.hpp>
+
 #include <QtGlobal>
 #include <QThread>
 #include <QSharedMemory>
@@ -77,108 +79,125 @@ SingleApplicationClass::SingleApplicationClass(int &argc, char *argv[], const bo
 
 #ifdef Q_OS_UNIX
   // By explicitly attaching it and then deleting it we make sure that the memory is deleted even after the process has crashed on Unix.
-#if QT_VERSION >= QT_VERSION_CHECK(6, 6, 0)
-  d->memory_ = new QSharedMemory(QNativeIpcKey(d->blockServerName_));
-#else
-  d->memory_ = new QSharedMemory(d->blockServerName_);
-#endif
-  d->memory_->attach();
-  delete d->memory_;
+  {
+#  if QT_VERSION >= QT_VERSION_CHECK(6, 6, 0)
+    std::unique_ptr<QSharedMemory> memory = std::make_unique<QSharedMemory>(QNativeIpcKey(d->blockServerName_));
+#  else
+    std::unique_ptr<QSharedMemory> memory = std::make_unique<QSharedMemory>(d->blockServerName_);
+#  endif
+    if (memory->attach()) {
+      memory->detach();
+    }
+  }
 #endif
 
   // Guarantee thread safe behaviour with a shared memory block.
 #if QT_VERSION >= QT_VERSION_CHECK(6, 6, 0)
-  d->memory_ = new QSharedMemory(QNativeIpcKey(d->blockServerName_));
+  QSharedMemory *memory = new QSharedMemory(QNativeIpcKey(d->blockServerName_), this);
 #else
-  d->memory_ = new QSharedMemory(d->blockServerName_);
+  QSharedMemory *memory = new QSharedMemory(d->blockServerName_, this);
 #endif
+  d->memory_ = memory;
+
+  bool primary = false;
 
   // Create a shared memory block
-  if (d->memory_->create(sizeof(InstancesInfo))) {
-    // Initialize the shared memory block
-    if (!d->memory_->lock()) {
-      qCritical() << "SingleApplication: Unable to lock memory block after create.";
-      abortSafely();
+  if (d->memory_->create(sizeof(SingleApplicationPrivateClass::InstancesInfo))) {
+    primary = true;
+  }
+  else if (d->memory_->error() == QSharedMemory::AlreadyExists) {
+    if (!d->memory_->attach()) {
+      qCritical() << "SingleApplication: Unable to attach to shared memory block:" << d->memory_->error() << d->memory_->errorString();
+      return;
     }
-    d->initializeMemoryBlock();
   }
   else {
-    if (d->memory_->error() == QSharedMemory::AlreadyExists) {
-      // Attempt to attach to the memory segment
-      if (!d->memory_->attach()) {
-        qCritical() << "SingleApplication: Unable to attach to shared memory block.";
-        abortSafely();
-      }
-      if (!d->memory_->lock()) {
-        qCritical() << "SingleApplication: Unable to lock memory block after attach.";
-        abortSafely();
-      }
-    }
-    else {
-      qCritical() << "SingleApplication: Unable to create block.";
-      abortSafely();
-    }
+    qCritical() << "SingleApplication: Unable to create shared memory block:" << d->memory_->error() << d->memory_->errorString();
+    return;
   }
 
-  InstancesInfo *instance = static_cast<InstancesInfo*>(d->memory_->data());
+  bool locked = false;
+
+  BOOST_SCOPE_EXIT((memory)(&locked)) {
+    if (locked && !memory->unlock()) {
+      qWarning() << "SingleApplication: Unable to unlock shared memory block:" << memory->error() << memory->errorString();
+      return;
+    }
+  }BOOST_SCOPE_EXIT_END
+
+  if (!d->memory_->lock()) {
+    qCritical() << "SingleApplication: Unable to lock shared memory block:" << d->memory_->error() << d->memory_->errorString();
+    return;
+  }
+  locked = true;
+
+  if (primary) {
+    // Initialize the shared memory block
+    d->initializeMemoryBlock();
+  }
+
+  SingleApplicationPrivateClass::InstancesInfo *instance = static_cast<SingleApplicationPrivateClass::InstancesInfo*>(d->memory_->data());
   QElapsedTimer time;
   time.start();
 
-  // Make sure the shared memory block is initialised and in consistent state
-  forever {
-    // If the shared memory block's checksum is valid continue
-    if (d->blockChecksum() == instance->checksum) break;
+  // Make sure the shared memory block is initialized and in a consistent state
+  while (d->blockChecksum() != instance->checksum) {
 
-    // If more than 5s have elapsed, assume the primary instance crashed and assume it's position
+    // If more than 5 seconds have elapsed, assume the primary instance crashed and assume its position
     if (time.elapsed() > 5000) {
-      qWarning() << "SingleApplication: Shared memory block has been in an inconsistent state from more than 5s. Assuming primary instance failure.";
+      qWarning() << "SingleApplication: Shared memory block has been in an inconsistent state from more than 5 seconds. Assuming primary instance failure.";
       d->initializeMemoryBlock();
     }
 
     // Otherwise wait for a random period and try again.
-    // The random sleep here limits the probability of a collision between two racing apps and allows the app to initialise faster
-    if (!d->memory_->unlock()) {
-      qDebug() << "SingleApplication: Unable to unlock memory for random wait.";
-      qDebug() << d->memory_->errorString();
+    // The random sleep here limits the probability of a collision between two racing apps and allows the app to initialize faster
+    if (locked) {
+      if (d->memory_->unlock()) {
+        locked = false;
+      }
+      else {
+        qCritical() << "SingleApplication: Unable to unlock shared memory block for random wait:" << memory->error() << memory->errorString();
+        return;
+      }
     }
+
     SingleApplicationPrivateClass::randomSleep();
+
     if (!d->memory_->lock()) {
-      qCritical() << "SingleApplication: Unable to lock memory after random wait.";
-      abortSafely();
+      qCritical() << "SingleApplication: Unable to lock shared memory block after random wait:" << memory->error() << memory->errorString();
+      return;
     }
+    locked = true;
+
   }
 
-  if (!instance->primary) {
+  if (instance->primary) {
+    // Check if another instance can be started
+    if (allowSecondary) {
+      d->startSecondary();
+      if (d->options_ & Mode::SecondaryNotification) {
+        d->connectToPrimary(timeout, SingleApplicationPrivateClass::SecondaryInstance);
+      }
+    }
+  }
+  else {
     d->startPrimary();
-    if (!d->memory_->unlock()) {
-      qDebug() << "SingleApplication: Unable to unlock memory after primary start.";
-      qDebug() << d->memory_->errorString();
-    }
-    return;
+    primary = true;
   }
 
-  // Check if another instance can be started
-  if (allowSecondary) {
-    d->startSecondary();
-    if (d->options_ & Mode::SecondaryNotification) {
-      d->connectToPrimary(timeout, SingleApplicationPrivateClass::SecondaryInstance);
+  if (locked) {
+    if (d->memory_->unlock()) {
+      locked = false;
     }
-    if (!d->memory_->unlock()) {
-      qDebug() << "SingleApplication: Unable to unlock memory after secondary start.";
-      qDebug() << d->memory_->errorString();
+    else {
+      qWarning() << "SingleApplication: Unable to unlock shared memory block:" << memory->error() << memory->errorString();
     }
-    return;
   }
 
-  if (!d->memory_->unlock()) {
-    qDebug() << "SingleApplication: Unable to unlock memory at end of execution.";
-    qDebug() << d->memory_->errorString();
+  if (!primary && !allowSecondary) {
+    d->connectToPrimary(timeout, SingleApplicationPrivateClass::NewInstance);
   }
 
-  d->connectToPrimary(timeout, SingleApplicationPrivateClass::NewInstance);
-
-  delete d;
-
 }
 
 SingleApplicationClass::~SingleApplicationClass() {
@@ -308,22 +327,3 @@ bool SingleApplicationClass::sendMessage(const QByteArray &message, const int ti
   return d->writeConfirmedMessage(timeout, message);
 
 }
-
-/**
- * Cleans up the shared memory block and exits with a failure.
- * This function halts program execution.
- */
-void SingleApplicationClass::abortSafely() {
-
-#if defined(SINGLEAPPLICATION)
-  Q_D(SingleApplication);
-#elif defined(SINGLECOREAPPLICATION)
-  Q_D(SingleCoreApplication);
-#endif
-
-  qCritical() << "SingleApplication: " << d->memory_->error() << d->memory_->errorString();
-  delete d;
-
-  ::exit(EXIT_FAILURE);
-
-}

3rdparty/singleapplication/singleapplication_t.h

@@ -102,7 +102,7 @@ class SingleApplicationClass : public ApplicationClass {  // clazy:exclude=ctor-
    * instance and the secondary instance.
    * @note The timeout is just a hint for the maximum time of blocking
    * operations. It does not guarantee that the SingleApplication
-   * initialisation will be completed in given time, though is a good hint.
+   * initialization will be completed in given time, though is a good hint.
    * Usually 4*timeout would be the worst case (fail) scenario.
    */
   explicit SingleApplicationClass(int &argc, char *argv[], const bool allowSecondary = false, const Options options = Mode::User, const int timeout = 1000);