refactor(daemon): Separate arg parsing from program control flow

src/apps/deskflow-daemon/deskflow-daemon.cpp

@@ -21,12 +21,13 @@
 
 #endif
 
+#include <QCommandLineParser>
 #include <QCoreApplication>
 #include <QThread>
 
 using namespace deskflow::core;
 
-void handleError(const char *message);
+void handleError(const char *message = "Unrecognized error.");
 
 int main(int argc, char **argv)
 {
@@ -43,24 +44,40 @@ int main(int argc, char **argv)
   Log log;
   EventQueue events;
 
-  auto &daemon = DaemonApp::instance();
+  // Daemon deliberately does not have a parent, as it will be moved to a new thread.
-  DaemonApp::InitResult initResult;
+  DaemonApp daemon(events);
-  try {
+
-    initResult = daemon.init(&events, argc, argv);
+  QCoreApplication app(argc, argv);
-  } catch (std::exception &e) {
+  QCoreApplication::setApplicationName(QStringLiteral("%1 Daemon").arg(kAppName));
-    handleError(e.what());
+
-    return kExitFailed;
+  QCommandLineParser parser;
-  } catch (...) {
+  parser.addHelpOption();
-    handleError("Unrecognized error.");
+  parser.addVersionOption();
-    return kExitFailed;
+
+  const auto foregroundOption = QCommandLineOption({"f", "foreground"}, "Run in the foreground (show console)");
+  parser.addOption(foregroundOption);
+
+  const auto installOption = QCommandLineOption({"i", "install"}, "Install as a Windows service");
+  parser.addOption(installOption);
+
+  const auto uninstallOption = QCommandLineOption({"u", "uninstall"}, "Uninstall the Windows service");
+  parser.addOption(uninstallOption);
+
+  parser.process(app);
+
+  if (parser.isSet(foregroundOption)) {
+    daemon.setForeground();
   }
 
-  // Important: Log the app name and version number to the log file after the daemon app init
+  // Depends on whether foreground option was set.
-  // because the file log outputter is created there. Logging before would only log to stdout
+  daemon.initLogging();
-  // which is not useful for troubleshooting Windows services.
+
+  // Important: Log the app name and version number to the log file daemon app has initialized
+  // logging as it creates the file logger. Logging before would only log to stdout which is not
+  // useful for troubleshooting Windows services.
   // It's important to write the version number to the log file so we can be certain the old daemon
   // was uninstalled, since sometimes Windows services can get stuck and fail to be removed.
-  LOG_PRINT("%s Daemon v%s", kAppName, kDisplayVersion);
+  LOG_PRINT("%s v%s", QCoreApplication::applicationName().toStdString().c_str(), kDisplayVersion);
 
   // Default log level to system setting (found in Registry).
   if (std::string logLevel = ARCH->setting("LogLevel"); logLevel != "") {
@@ -68,72 +85,43 @@ int main(int argc, char **argv)
     LOG_DEBUG("log level: %s", logLevel.c_str());
   }
 
+  try {
+
 #if SYSAPI_WIN32
-  // Show warning if not running as admin as daemon will behave differently.
+    // Show warning if not running as admin as daemon will behave differently.
-  if (!ArchMiscWindows::isProcessElevated()) {
+    if (!ArchMiscWindows::isProcessElevated()) {
-    LOG_WARN("not running as admin, some features may not work");
+      LOG_WARN("not running as admin, some features may not work");
-  }
+    }
 #endif
 
-  switch (initResult) {
+    if (parser.isSet(installOption)) {
-    using enum DaemonApp::InitResult;
+      daemon.install();
+      return kExitSuccess;
+    } else if (parser.isSet(uninstallOption)) {
+      daemon.uninstall();
+      return kExitSuccess;
+    }
 
-  case StartDaemon: {
+    const auto ipcServer = new ipc::DaemonIpcServer(&app, DaemonApp::logFilename().c_str()); // NOSONAR - Qt managed
-    LOG_INFO("starting daemon");
+    ipcServer->listen();
-    QCoreApplication app(argc, argv);
+    daemon.connectIpcServer(ipcServer);
 
     QThread daemonThread;
-    daemon.moveToThread(&daemonThread);
-
-    QObject::connect(&daemonThread, &QThread::started, [&daemon, &daemonThread]() {
-      LOG_DEBUG("daemon thread started");
-      daemon.run();
-      daemonThread.quit();
-      LOG_DEBUG("daemon thread finished");
-    });
     QObject::connect(&daemonThread, &QThread::finished, &app, &QCoreApplication::quit);
+    daemon.run(daemonThread);
 
-    ipc::DaemonIpcServer ipcServer(&app, QString::fromStdString(daemon.logFilename()));
-
-    // Use direct connection as the daemon app is on it's own thread, and so is on a different event loop.
-    QObject::connect(
-        &ipcServer, &ipc::DaemonIpcServer::logLevelChanged, &daemon, &DaemonApp::saveLogLevel, //
-        Qt::DirectConnection
-    );
-    QObject::connect(
-        &ipcServer, &ipc::DaemonIpcServer::elevateModeChanged, &daemon, &DaemonApp::setElevate, //
-        Qt::DirectConnection
-    );
-    QObject::connect(
-        &ipcServer, &ipc::DaemonIpcServer::commandChanged, &daemon, &DaemonApp::setCommand, //
-        Qt::DirectConnection
-    );
-    QObject::connect(
-        &ipcServer, &ipc::DaemonIpcServer::startProcessRequested, &daemon, &DaemonApp::applyWatchdogCommand, //
-        Qt::DirectConnection
-    );
-    QObject::connect(
-        &ipcServer, &ipc::DaemonIpcServer::stopProcessRequested, &daemon, &DaemonApp::clearWatchdogCommand, //
-        Qt::DirectConnection
-    );
-    QObject::connect(
-        &ipcServer, &ipc::DaemonIpcServer::clearSettingsRequested, &daemon, &DaemonApp::clearSettings, //
-        Qt::DirectConnection
-    );
-
-    daemonThread.start();
     const auto exitCode = QCoreApplication::exec();
     daemonThread.wait();
 
     LOG_DEBUG("daemon exited, code: %d", exitCode);
     return exitCode;
-  }
 
-  case FatalError:
+  } catch (std::exception &e) {
+    handleError(e.what());
+    return kExitFailed;
+  } catch (...) {
+    handleError();
     return kExitFailed;
-
-  default:
-    return kExitSuccess;
   }
 }
 

src/lib/deskflow/DaemonApp.cpp

@@ -7,14 +7,16 @@
 #include "deskflow/DaemonApp.h"
 
 #include "arch/XArch.h"
+#include "base/IEventQueue.h"
 #include "base/Log.h"
 #include "base/log_outputters.h"
 #include "common/constants.h"
 #include "deskflow/App.h"
+#include "deskflow/ipc/DaemonIpcServer.h"
 
 #if SYSAPI_WIN32
 
-#include "arch/win32/ArchMiscWindows.h"
+#include "arch/win32/ArchMiscWindows.h" // IWYU pragma: keep
 #include "deskflow/Screen.h"
 #include "platform/MSWindowsDebugOutputter.h"
 #include "platform/MSWindowsEventQueueBuffer.h"
@@ -27,7 +29,6 @@
 
 #include <filesystem>
 #include <iostream>
-#include <memory>
 #include <string>
 
 using namespace std;
@@ -39,38 +40,12 @@ void showHelp(int argc, char **argv) // NOSONAR - CLI args
   std::cout << "Usage: " << binName << " [-f|--foreground] [--install] [--uninstall]" << std::endl;
 }
 
-DaemonApp::DaemonApp()
+DaemonApp::DaemonApp(IEventQueue &events) : m_events(events)
 {
-  m_fileLogOutputter = new FileLogOutputter(logFilename().c_str()); // NOSONAR - Adopted by `Log`
-  CLOG->insert(m_fileLogOutputter);
 }
 
 DaemonApp::~DaemonApp() = default;
 
-void DaemonApp::run()
-{
-  if (m_foreground) {
-    LOG_DEBUG("running daemon in foreground");
-    mainLoop();
-  } else {
-    LOG_DEBUG("running daemon in background (daemonizing)");
-    ARCH->daemonize(kAppName, [this](int, const char **) { return daemonLoop(); });
-  }
-}
-
-int DaemonApp::daemonLoop()
-{
-#if SYSAPI_WIN32
-  return ArchMiscWindows::runDaemon([this]() {
-    mainLoop();
-    return kExitSuccess;
-  });
-#elif SYSAPI_UNIX
-  mainLoop();
-  return kExitSuccess;
-#endif
-}
-
 void DaemonApp::saveLogLevel(const QString &logLevel) const
 {
   LOG_DEBUG("log level changed: %s", logLevel.toUtf8().constData());
@@ -115,7 +90,7 @@ void DaemonApp::applyWatchdogCommand() const
   LOG_DEBUG("applying watchdog command");
 
 #if SYSAPI_WIN32
-  m_watchdog->setProcessConfig(m_command, m_elevate);
+  m_pWatchdog->setProcessConfig(m_command, m_elevate);
 #else
   LOG_ERR("applying watchdog command not implemented on this platform");
 #endif
@@ -129,7 +104,7 @@ void DaemonApp::clearWatchdogCommand()
   setCommand("");
 
 #if SYSAPI_WIN32
-  m_watchdog->setProcessConfig("", false);
+  m_pWatchdog->setProcessConfig("", false);
 #else
   LOG_ERR("clearing watchdog command not implemented on this platform");
 #endif
@@ -141,84 +116,104 @@ void DaemonApp::clearSettings() const
   ARCH->clearSettings();
 }
 
-DaemonApp::InitResult DaemonApp::init(IEventQueue *events, int argc, char **argv) // NOSONAR - CLI args
+void DaemonApp::connectIpcServer(const ipc::DaemonIpcServer *ipcServer) const
 {
-  using enum InitResult;
+  // Use direct connection as this object is on it's own thread,
+  // and so is on a different event loop to the main Qt loop.
+  QObject::connect(
+      ipcServer, &ipc::DaemonIpcServer::logLevelChanged, this, &DaemonApp::saveLogLevel, //
+      Qt::DirectConnection
+  );
+  QObject::connect(
+      ipcServer, &ipc::DaemonIpcServer::elevateModeChanged, this, &DaemonApp::setElevate, //
+      Qt::DirectConnection
+  );
+  QObject::connect(
+      ipcServer, &ipc::DaemonIpcServer::commandChanged, this, &DaemonApp::setCommand, //
+      Qt::DirectConnection
+  );
+  QObject::connect(
+      ipcServer, &ipc::DaemonIpcServer::startProcessRequested, this, &DaemonApp::applyWatchdogCommand, //
+      Qt::DirectConnection
+  );
+  QObject::connect(
+      ipcServer, &ipc::DaemonIpcServer::stopProcessRequested, this, &DaemonApp::clearWatchdogCommand, //
+      Qt::DirectConnection
+  );
+  QObject::connect(
+      ipcServer, &ipc::DaemonIpcServer::clearSettingsRequested, this, &DaemonApp::clearSettings, //
+      Qt::DirectConnection
+  );
+}
 
-  if (events == nullptr) {
+void DaemonApp::install() const
-    throw XDeskflow("event queue not set");
+{
-  }
+  LOG_NOTE("installing windows daemon");
+  ARCH->installDaemon();
+}
 
-  m_events = events;
+void DaemonApp::uninstall() const
+{
+  LOG_NOTE("uninstalling windows daemon");
+  ARCH->uninstallDaemon();
+}
 
-  for (int i = 1; i < argc; ++i) {
+void DaemonApp::run(QThread &daemonThread)
-    string arg(argv[i]);
+{
+  LOG_NOTE("starting daemon");
 
-    if (arg == "-h" || arg == "--help") {
+  // Important: Move the daemon app to the daemon thread before creating any more Qt objects
-      showConsole();
+  // owned by the daemon app, as they will be created on the daemon thread.
-      showHelp(argc, argv);
+  moveToThread(&daemonThread);
-      return ShowHelp;
+
-    } else if (arg == "-f" || arg == "--foreground") {
+  QObject::connect(&daemonThread, &QThread::started, [this, &daemonThread]() {
-      showConsole();
+    LOG_DEBUG("daemon thread started");
-      m_foreground = true;
+
+    if (m_foreground) {
+      LOG_DEBUG("running daemon in foreground");
+      mainLoop();
+    } else {
+      LOG_DEBUG("running daemon in background (daemonizing)");
+      ARCH->daemonize(kAppName, [this](int, const char **) { return daemonLoop(); });
     }
-#if SYSAPI_WIN32
+
-    else if (arg == "--install" || arg == "/install") {
+    daemonThread.quit();
-      LOG((CLOG_NOTE "installing windows daemon"));
+    LOG_DEBUG("daemon thread finished");
-      ARCH->installDaemon();
+  });
-      return Installed;
-    } else if (arg == "--uninstall" || arg == "/uninstall") {
-      LOG((CLOG_NOTE "uninstalling windows daemon"));
-      try {
-        ARCH->uninstallDaemon();
-      } catch (XArch &e) {
-        std::string message = e.what();
-        if (message.find("The service has not been started") != std::string::npos) {
-          // HACK: this message happens intermittently, not sure where from but
-          // it's quite misleading for the user. they thing something has gone
-          // horribly wrong, but it's just the service manager reporting a false
-          // positive (the service has actually shut down in most cases).
-          LOG_DEBUG("ignoring service start error on uninstall: %s", message.c_str());
-        } else {
-          throw e;
-        }
-      }
-      return Uninstalled;
-    }
-#endif
-    else {
-      LOG_ERR("unknown argument: %s", arg.c_str());
-      return ArgsError;
-    }
-  }
 
 #if SYSAPI_WIN32
-  if (!m_foreground) {
+  m_pWatchdog = std::make_unique<MSWindowsWatchdog>(m_foreground, *m_pFileLogOutputter);
-    // Only use MS debug outputter when the process is daemonized, since stdout won't be accessible
-    // in that case, but is accessible when running in the foreground.
-    CLOG->insert(new MSWindowsDebugOutputter()); // NOSONAR - Adopted by `Log`
-  }
-
-  m_watchdog = std::make_unique<MSWindowsWatchdog>(m_foreground);
-  m_watchdog->setFileLogOutputter(m_fileLogOutputter);
 
   std::string command = ARCH->setting("Command");
   bool elevate = ARCH->setting("Elevate") == "1";
   if (!command.empty()) {
     LOG_DEBUG("using last known command: %s", command.c_str());
-    m_watchdog->setProcessConfig(command, elevate);
+    m_pWatchdog->setProcessConfig(command, elevate);
   }
 #endif
 
-  return StartDaemon;
+  LOG_DEBUG("starting daemon thread");
+  daemonThread.start();
 }
 
-void DaemonApp::mainLoop()
+int DaemonApp::daemonLoop()
 {
-  if (m_events == nullptr) {
+#if SYSAPI_WIN32
-    LOG((CLOG_CRIT "event queue not set for main loop"));
+  // Runs the daemon through the Windows service controller, which controls the program lifecycle.
-    return;
+  return ArchMiscWindows::runDaemon([this]() { return mainLoop(); });
+#elif SYSAPI_UNIX
+  return mainLoop();
+#endif
+}
+
+int DaemonApp::mainLoop()
+{
+#if SYSAPI_WIN32
+  if (m_pWatchdog == nullptr) {
+    LOG_ERR("watchdog not initialized");
+    return kExitFailed;
   }
+#endif
 
   DAEMON_RUNNING(true);
 
@@ -227,14 +222,14 @@ void DaemonApp::mainLoop()
     // Install the platform event queue to handle service stop events.
     // This must be done on the same thread as the event loop, otherwise the service stop
     // request will not add the quit event to the event queue, and the service won't stop.
-    m_events->adoptBuffer(new MSWindowsEventQueueBuffer(m_events));
+    m_events.adoptBuffer(new MSWindowsEventQueueBuffer(&m_events));
 
     LOG_DEBUG("starting watchdog threads");
-    m_watchdog->startAsync();
+    m_pWatchdog->startAsync();
 #endif
 
     LOG_INFO("daemon is running");
-    m_events->loop();
+    m_events.loop();
     LOG_INFO("daemon is stopping");
   } catch (std::exception &e) { // NOSONAR - Catching all exceptions
     LOG((CLOG_CRIT "daemon error: %s", e.what()));
@@ -245,7 +240,7 @@ void DaemonApp::mainLoop()
 #if SYSAPI_WIN32
   try {
     LOG_DEBUG("stopping process watchdog");
-    m_watchdog->stop();
+    m_pWatchdog->stop();
   } catch (std::exception &e) { // NOSONAR - Catching all exceptions
     LOG((CLOG_CRIT "stop watchdog error: %s", e.what()));
   } catch (...) { // NOSONAR - Catching remaining exceptions
@@ -254,6 +249,7 @@ void DaemonApp::mainLoop()
 #endif
 
   DAEMON_RUNNING(false);
+  return kExitSuccess;
 }
 
 std::string DaemonApp::logFilename()
@@ -269,6 +265,26 @@ std::string DaemonApp::logFilename()
   return logFilename;
 }
 
+void DaemonApp::setForeground()
+{
+  m_foreground = true;
+  showConsole();
+}
+
+void DaemonApp::initLogging()
+{
+#if SYSAPI_WIN32
+  if (!m_foreground) {
+    // Only use MS debug outputter when the process is daemonized, since stdout won't be accessible
+    // in that case, but is accessible when running in the foreground.
+    CLOG->insert(new MSWindowsDebugOutputter()); // NOSONAR - Adopted by `Log`
+  }
+#endif
+
+  m_pFileLogOutputter = new FileLogOutputter(logFilename().c_str()); // NOSONAR - Adopted by `Log`
+  CLOG->insert(m_pFileLogOutputter);
+}
+
 void DaemonApp::showConsole()
 {
 #if SYSAPI_WIN32

src/lib/deskflow/DaemonApp.h

@@ -8,10 +8,10 @@
 
 #include "common/common.h"
 
-#include <memory>
 #include <string>
 
 #include <QObject>
+#include <QThread>
 
 class Event;
 class IEventQueue;
@@ -44,40 +44,40 @@ public:
     FatalError,
   };
 
-  InitResult init(IEventQueue *events, int argc, char **argv);
+  explicit DaemonApp(IEventQueue &events);
-  void run();
+  ~DaemonApp() override;
-  void mainLoop();
+
+  InitResult init(int argc, char **argv);
+  void install() const;
+  void uninstall() const;
+  void run(QThread &daemonThread);
+  void setForeground();
+  void initLogging();
+  void connectIpcServer(const deskflow::core::ipc::DaemonIpcServer *ipcServer) const;
+
+  static std::string logFilename();
+
+private:
+  void daemonize();
+  void handleError(const char *message);
+  void handleIpcMessage(const Event &e, void *);
+  int mainLoop();
+  int daemonLoop();
   void saveLogLevel(const QString &logLevel) const;
   void setElevate(bool elevate);
   void setCommand(const QString &command);
   void applyWatchdogCommand() const;
   void clearWatchdogCommand();
   void clearSettings() const;
-  std::string logFilename();
-  int daemonLoop();
-
-  static DaemonApp &instance()
-  {
-    static DaemonApp instance; // NOSONAR - Meyers' Singleton
-    return instance;
-  }
-
-private:
-  explicit DaemonApp();
-  ~DaemonApp() override;
-
-  void daemonize();
-  void handleError(const char *message);
-  void handleIpcMessage(const Event &e, void *);
 
   static void showConsole();
 
 #if SYSAPI_WIN32
-  std::unique_ptr<MSWindowsWatchdog> m_watchdog;
+  std::unique_ptr<MSWindowsWatchdog> m_pWatchdog;
 #endif
 
-  IEventQueue *m_events = nullptr;
+  IEventQueue &m_events;
-  FileLogOutputter *m_fileLogOutputter = nullptr;
+  FileLogOutputter *m_pFileLogOutputter = nullptr;
   deskflow::core::ipc::DaemonIpcServer *m_ipcServer = nullptr;
   std::string m_command = "";
   bool m_elevate = false;

src/lib/deskflow/ipc/DaemonIpcServer.cpp

@@ -21,6 +21,15 @@ DaemonIpcServer::DaemonIpcServer(QObject *parent, const QString &logFilename)
     : QObject(parent),
       m_logFilename(logFilename),
       m_server{new QLocalServer(this)} // NOSONAR - Qt memory
+{
+}
+
+DaemonIpcServer::~DaemonIpcServer()
+{
+  m_server->close();
+}
+
+void DaemonIpcServer::listen()
 {
   // Daemon runs as system, but GUI runs as regular user, so we need to allow world access.
   m_server->setSocketOptions(QLocalServer::WorldAccessOption);
@@ -34,11 +43,6 @@ DaemonIpcServer::DaemonIpcServer(QObject *parent, const QString &logFilename)
   }
 }
 
-DaemonIpcServer::~DaemonIpcServer()
-{
-  m_server->close();
-}
-
 void DaemonIpcServer::handleNewConnection()
 {
   QLocalSocket *clientSocket = m_server->nextPendingConnection();

src/lib/deskflow/ipc/DaemonIpcServer.h

@@ -22,6 +22,8 @@ public:
   explicit DaemonIpcServer(QObject *parent, const QString &logFilename);
   ~DaemonIpcServer() override;
 
+  void listen();
+
 signals:
   void logLevelChanged(const QString &logLevel);
   void elevateModeChanged(bool elevate);

src/lib/platform/MSWindowsWatchdog.cpp

@@ -79,7 +79,9 @@ HANDLE openProcessForKill(const PROCESSENTRY32 &entry)
 // MSWindowsWatchdog
 //
 
-MSWindowsWatchdog::MSWindowsWatchdog(bool foreground) : m_foreground(foreground)
+MSWindowsWatchdog::MSWindowsWatchdog(bool foreground, FileLogOutputter &fileLogOutputter)
+    : m_fileLogOutputter(fileLogOutputter),
+      m_foreground(foreground)
 {
   initSasFunc();
   initOutputReadPipe();
@@ -263,11 +265,6 @@ bool MSWindowsWatchdog::isProcessRunning()
   return exitCode == STILL_ACTIVE;
 }
 
-void MSWindowsWatchdog::setFileLogOutputter(FileLogOutputter *outputter)
-{
-  m_fileLogOutputter = outputter;
-}
-
 void MSWindowsWatchdog::startProcess()
 {
   if (m_command.empty()) {
@@ -369,10 +366,7 @@ void MSWindowsWatchdog::outputLoop(void *)
 
       // strip out windows \r chars to prevent extra lines in log file.
       std::string output = trimOutputBuffer(buffer);
-
+      m_fileLogOutputter.write(kPRINT, output.c_str());
-      if (m_fileLogOutputter != nullptr) {
-        m_fileLogOutputter->write(kPRINT, output.c_str());
-      }
 
 #if SYSAPI_WIN32
       if (m_foreground) {

src/lib/platform/MSWindowsWatchdog.h

@@ -37,7 +37,7 @@ class MSWindowsWatchdog
   };
 
 public:
-  explicit MSWindowsWatchdog(bool foreground);
+  explicit MSWindowsWatchdog(bool foreground, FileLogOutputter &fileLogOutputter);
   ~MSWindowsWatchdog() = default;
 
   /**
@@ -60,15 +60,6 @@ public:
    */
   bool isProcessRunning();
 
-  /**
-   * @brief Set the file log outputter.
-   *
-   * Outputter is not adopted by the watchdog, so the caller must manage the memory.
-   *
-   * Standard out/error from the launched core process is written to the file log outputter.
-   */
-  void setFileLogOutputter(FileLogOutputter *outputter);
-
 private:
   /**
    * @brief Monitor the process state and start/stop the process as necessary.
@@ -157,7 +148,7 @@ private:
   bool m_elevateProcess = false;
   MSWindowsSession m_session;
   int m_startFailures = 0;
-  FileLogOutputter *m_fileLogOutputter = nullptr;
+  FileLogOutputter &m_fileLogOutputter;
   bool m_foreground = false;
   std::string m_activeDesktop = "";
   std::unique_ptr<deskflow::platform::MSWindowsProcess> m_process;