From e8ea9f53ee246db062bcfb8452f50a6f791a6d9f Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Wed, 5 Mar 2025 15:40:23 +0000 Subject: [PATCH] refactor(daemon): Separate arg parsing from program control flow --- src/apps/deskflow-daemon/deskflow-daemon.cpp | 120 +++++------ src/lib/deskflow/DaemonApp.cpp | 202 ++++++++++--------- src/lib/deskflow/DaemonApp.h | 46 ++--- src/lib/deskflow/ipc/DaemonIpcServer.cpp | 14 +- src/lib/deskflow/ipc/DaemonIpcServer.h | 2 + src/lib/platform/MSWindowsWatchdog.cpp | 14 +- src/lib/platform/MSWindowsWatchdog.h | 13 +- 7 files changed, 203 insertions(+), 208 deletions(-) diff --git a/src/apps/deskflow-daemon/deskflow-daemon.cpp b/src/apps/deskflow-daemon/deskflow-daemon.cpp index b6788ab80..6a1f710ae 100644 --- a/src/apps/deskflow-daemon/deskflow-daemon.cpp +++ b/src/apps/deskflow-daemon/deskflow-daemon.cpp @@ -21,12 +21,13 @@ #endif +#include #include #include 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(); - DaemonApp::InitResult initResult; - try { - initResult = daemon.init(&events, argc, argv); - } catch (std::exception &e) { - handleError(e.what()); - return kExitFailed; - } catch (...) { - handleError("Unrecognized error."); - return kExitFailed; + // Daemon deliberately does not have a parent, as it will be moved to a new thread. + DaemonApp daemon(events); + + QCoreApplication app(argc, argv); + QCoreApplication::setApplicationName(QStringLiteral("%1 Daemon").arg(kAppName)); + + QCommandLineParser parser; + parser.addHelpOption(); + parser.addVersionOption(); + + 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 - // because the file log outputter is created there. Logging before would only log to stdout - // which is not useful for troubleshooting Windows services. + // Depends on whether foreground option was set. + daemon.initLogging(); + + // 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. - if (!ArchMiscWindows::isProcessElevated()) { - LOG_WARN("not running as admin, some features may not work"); - } + // Show warning if not running as admin as daemon will behave differently. + if (!ArchMiscWindows::isProcessElevated()) { + LOG_WARN("not running as admin, some features may not work"); + } #endif - switch (initResult) { - using enum DaemonApp::InitResult; + if (parser.isSet(installOption)) { + daemon.install(); + return kExitSuccess; + } else if (parser.isSet(uninstallOption)) { + daemon.uninstall(); + return kExitSuccess; + } - case StartDaemon: { - LOG_INFO("starting daemon"); - QCoreApplication app(argc, argv); + const auto ipcServer = new ipc::DaemonIpcServer(&app, DaemonApp::logFilename().c_str()); // NOSONAR - Qt managed + ipcServer->listen(); + 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; } } diff --git a/src/lib/deskflow/DaemonApp.cpp b/src/lib/deskflow/DaemonApp.cpp index 5fc34bbe9..0a0bf153e 100644 --- a/src/lib/deskflow/DaemonApp.cpp +++ b/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 #include -#include #include 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) { - throw XDeskflow("event queue not set"); - } +void DaemonApp::install() const +{ + 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) { - string arg(argv[i]); +void DaemonApp::run(QThread &daemonThread) +{ + LOG_NOTE("starting daemon"); - if (arg == "-h" || arg == "--help") { - showConsole(); - showHelp(argc, argv); - return ShowHelp; - } else if (arg == "-f" || arg == "--foreground") { - showConsole(); - m_foreground = true; + // Important: Move the daemon app to the daemon thread before creating any more Qt objects + // owned by the daemon app, as they will be created on the daemon thread. + moveToThread(&daemonThread); + + QObject::connect(&daemonThread, &QThread::started, [this, &daemonThread]() { + LOG_DEBUG("daemon thread started"); + + 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") { - LOG((CLOG_NOTE "installing windows daemon")); - 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; - } - } + + daemonThread.quit(); + LOG_DEBUG("daemon thread finished"); + }); #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` - } - - m_watchdog = std::make_unique(m_foreground); - m_watchdog->setFileLogOutputter(m_fileLogOutputter); + m_pWatchdog = std::make_unique(m_foreground, *m_pFileLogOutputter); 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) { - LOG((CLOG_CRIT "event queue not set for main loop")); - return; +#if SYSAPI_WIN32 + // Runs the daemon through the Windows service controller, which controls the program lifecycle. + 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 diff --git a/src/lib/deskflow/DaemonApp.h b/src/lib/deskflow/DaemonApp.h index 669d8a1b2..3490d7be9 100644 --- a/src/lib/deskflow/DaemonApp.h +++ b/src/lib/deskflow/DaemonApp.h @@ -8,10 +8,10 @@ #include "common/common.h" -#include #include #include +#include class Event; class IEventQueue; @@ -44,40 +44,40 @@ public: FatalError, }; - InitResult init(IEventQueue *events, int argc, char **argv); - void run(); - void mainLoop(); + explicit DaemonApp(IEventQueue &events); + ~DaemonApp() override; + + 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 m_watchdog; + std::unique_ptr m_pWatchdog; #endif - IEventQueue *m_events = nullptr; - FileLogOutputter *m_fileLogOutputter = nullptr; + IEventQueue &m_events; + FileLogOutputter *m_pFileLogOutputter = nullptr; deskflow::core::ipc::DaemonIpcServer *m_ipcServer = nullptr; std::string m_command = ""; bool m_elevate = false; diff --git a/src/lib/deskflow/ipc/DaemonIpcServer.cpp b/src/lib/deskflow/ipc/DaemonIpcServer.cpp index 83f5420da..179a2a1e1 100644 --- a/src/lib/deskflow/ipc/DaemonIpcServer.cpp +++ b/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(); diff --git a/src/lib/deskflow/ipc/DaemonIpcServer.h b/src/lib/deskflow/ipc/DaemonIpcServer.h index eb540e2ee..1bc95716c 100644 --- a/src/lib/deskflow/ipc/DaemonIpcServer.h +++ b/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); diff --git a/src/lib/platform/MSWindowsWatchdog.cpp b/src/lib/platform/MSWindowsWatchdog.cpp index fb811e526..788efa3ed 100644 --- a/src/lib/platform/MSWindowsWatchdog.cpp +++ b/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); - - if (m_fileLogOutputter != nullptr) { - m_fileLogOutputter->write(kPRINT, output.c_str()); - } + m_fileLogOutputter.write(kPRINT, output.c_str()); #if SYSAPI_WIN32 if (m_foreground) { diff --git a/src/lib/platform/MSWindowsWatchdog.h b/src/lib/platform/MSWindowsWatchdog.h index a9464224c..6c4603580 100644 --- a/src/lib/platform/MSWindowsWatchdog.h +++ b/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 m_process;