refactor: Exclude current process when killing existing process from Windows daemon

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

@@ -6,8 +6,8 @@
  */
 
 #include "arch/Arch.h"
-#include "base/EventQueue.h"
 #include "base/Log.h"
+#include "common/constants.h"
 #include "deskflow/DaemonApp.h"
 
 #include "arch/Arch.h"
@@ -43,13 +43,18 @@ int main(int argc, char **argv)
 
   QCoreApplication app(argc, argv);
 
-  // Must be on the heap, as we're moving it to a thread.
-  DaemonApp *pDaemon = new DaemonApp(&events);
+  const auto pDaemon = &DaemonApp::instance();
+  QObject::connect(pDaemon, &DaemonApp::mainLoopFinished, &app, &QCoreApplication::quit);
+  QObject::connect(pDaemon, &DaemonApp::fatalErrorOccurred, &app, &QCoreApplication::quit);
   QObject::connect(pDaemon, &DaemonApp::serviceInstalled, &app, &QCoreApplication::quit);
   QObject::connect(pDaemon, &DaemonApp::serviceUninstalled, &app, &QCoreApplication::quit);
-  pDaemon->init(argc, argv);
+  pDaemon->init(&events, argc, argv);
 
-  QThread *thread = new QThread();
+  // Thread must be on the heap, as the thread needs to be deleted only when the loop finishes,
+  // and not when we go out of scope.
+  // Deliberately do not set ownership of the thread to the daemon, as we want to delete the thread
+  // when the loop finishes, and not when the daemon is deleted.
+  QThread *thread = new QThread(); // NOSONAR
   pDaemon->moveToThread(thread);
   QObject::connect(thread, &QThread::started, pDaemon, &DaemonApp::run);
   QObject::connect(thread, &QThread::finished, pDaemon, &QObject::deleteLater);

src/lib/deskflow/DaemonApp.cpp

@@ -81,11 +81,9 @@ bool isServerCommandLine(const std::vector<std::string> &cmd)
 
 } // namespace
 
-DaemonApp *DaemonApp::s_instance = nullptr;
-
 int mainLoopStatic()
 {
-  DaemonApp::s_instance->mainLoop(true);
+  DaemonApp::instance().mainLoop(true);
   return kExitSuccess;
 }
 
@@ -101,19 +99,14 @@ int winMainLoopStatic(int, const char **)
 }
 #endif
 
-DaemonApp::DaemonApp(IEventQueue *events) : m_ipcServer{new ipc::DaemonIpcServer(this)}
+DaemonApp::DaemonApp() : m_ipcServer{new ipc::DaemonIpcServer(this)}
 {
-  s_instance = this;
-
   connect(m_ipcServer, &ipc::DaemonIpcServer::elevateModeChanged, this, &DaemonApp::handleElevateModeChanged);
   connect(m_ipcServer, &ipc::DaemonIpcServer::commandChanged, this, &DaemonApp::handleCommandChanged);
   connect(m_ipcServer, &ipc::DaemonIpcServer::restartRequested, this, &DaemonApp::handleRestartRequested);
 }
 
-DaemonApp::~DaemonApp()
-{
-  s_instance = nullptr;
-}
+DaemonApp::~DaemonApp() = default;
 
 void DaemonApp::run()
 {
@@ -132,6 +125,8 @@ void DaemonApp::run()
     ARCH->daemonize(kAppName, unixMainLoopStatic);
 #endif
   }
+
+  Q_EMIT mainLoopFinished();
 }
 
 void DaemonApp::handleElevateModeChanged(int mode)
@@ -156,8 +151,14 @@ void DaemonApp::handleRestartRequested()
 #endif
 }
 
-void DaemonApp::init(int argc, char **argv)
+void DaemonApp::init(IEventQueue *events, int argc, char **argv) // NOSONAR
 {
+  if (events == nullptr) {
+    throw XDeskflow("event queue not set");
+  }
+
+  m_events = events;
+
   bool isUninstalling = false;
   try {
     // default log level to system setting.
@@ -186,7 +187,7 @@ void DaemonApp::init(int argc, char **argv)
         stringstream ss;
         ss << "Unrecognized argument: " << arg;
         foregroundError(ss.str().c_str());
-        Q_EMIT fatalError();
+        Q_EMIT fatalErrorOccurred();
       }
     }
 
@@ -208,18 +209,22 @@ void DaemonApp::init(int argc, char **argv)
     } else {
       foregroundError(message.c_str());
     }
-    Q_EMIT fatalError();
+    Q_EMIT fatalErrorOccurred();
   } catch (std::exception &e) {
     foregroundError(e.what());
-    Q_EMIT fatalError();
+    Q_EMIT fatalErrorOccurred();
   } catch (...) {
     foregroundError("Unrecognized error.");
-    Q_EMIT fatalError();
+    Q_EMIT fatalErrorOccurred();
   }
 }
 
 void DaemonApp::mainLoop(bool logToFile, bool foreground)
 {
+  if (m_events == nullptr) {
+    throw XDeskflow("event queue not set");
+  }
+
   try {
     DAEMON_RUNNING(true);
 

src/lib/deskflow/DaemonApp.h

@@ -37,18 +37,26 @@ class DaemonApp : public QObject
   Q_OBJECT
 
 public:
-  DaemonApp(IEventQueue *events);
-  ~DaemonApp();
-  void init(int argc, char **argv);
+  void init(IEventQueue *events, int argc, char **argv);
   void run();
   void mainLoop(bool logToFile, bool foreground = false);
 
+  static DaemonApp &instance()
+  {
+    static DaemonApp instance; // NOSONAR - Meyers' Singleton
+    return instance;
+  }
+
 signals:
-  void fatalError();
+  void mainLoopFinished();
+  void fatalErrorOccurred();
   void serviceInstalled();
   void serviceUninstalled();
 
 private:
+  explicit DaemonApp();
+  ~DaemonApp() override;
+
   void daemonize();
   void foregroundError(const char *message);
   std::string logFilename();
@@ -59,9 +67,6 @@ private slots:
   void handleCommandChanged(const QString &command);
   void handleRestartRequested();
 
-public:
-  static DaemonApp *s_instance;
-
 #if SYSAPI_WIN32
   std::unique_ptr<MSWindowsWatchdog> m_watchdog;
 #endif

src/lib/platform/MSWindowsWatchdog.cpp

@@ -422,15 +422,36 @@ void MSWindowsWatchdog::outputLoop(void *)
   }
 }
 
+HANDLE openProcessForKill(PROCESSENTRY32 entry)
+{
+  // pid 0 is 'system idle process'
+  if (entry.th32ProcessID == 0)
+    return nullptr;
+
+  if (_stricmp(entry.szExeFile, "deskflow-client.exe") != 0 && //
+      _stricmp(entry.szExeFile, "deskflow-server.exe") != 0 && //
+      _stricmp(entry.szExeFile, "deskflow-core.exe") != 0) {
+    return nullptr;
+  }
+
+  HANDLE handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, entry.th32ProcessID);
+  if (handle == nullptr) {
+    LOG((CLOG_ERR "could not open process handle for kill"));
+    throw XArch(new XArchEvalWindows);
+  }
+
+  // only shut down if not current process (daemon is now the same unified binary).
+  if (entry.th32ProcessID == GetCurrentProcessId()) {
+    CloseHandle(handle);
+    return nullptr;
+  }
+
+  return handle;
+}
+
 void MSWindowsWatchdog::shutdownExistingProcesses()
 {
-  LOG_INFO("daemon shutting down existing processes");
-
-  if (m_process != nullptr) {
-    m_process->shutdown();
-    m_process.reset();
-    m_processStarted = false;
-  }
+  LOG_DEBUG("shutting down existing processes");
 
   // first we need to take a snapshot of the running processes
   HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, CURRENT_PROCESS_ID);
@@ -454,17 +475,11 @@ void MSWindowsWatchdog::shutdownExistingProcesses()
   DWORD pid = 0;
   while (gotEntry) {
 
-    // make sure we're not checking the system process
-    if (entry.th32ProcessID != 0) {
-
-      if (_stricmp(entry.szExeFile, "deskflow-client.exe") == 0 ||
-          _stricmp(entry.szExeFile, "deskflow-server.exe") == 0 ||
-          _stricmp(entry.szExeFile, "deskflow-core.exe") == 0) {
-
-        HANDLE handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, entry.th32ProcessID);
-        deskflow::platform::MSWindowsProcess::shutdown(handle, entry.th32ProcessID);
-        CloseHandle(handle);
-      }
+    HANDLE handle = openProcessForKill(entry);
+    if (handle) {
+      LOG((CLOG_INFO "shutting down process, name=%s, pid=%d", entry.szExeFile, entry.th32ProcessID));
+      deskflow::platform::MSWindowsProcess::shutdown(handle, entry.th32ProcessID);
+      CloseHandle(handle);
     }
 
     // now move on to the next entry (if we're not at the end)