From a654bfa692ca3fc89ddcd0057d371f954f8edcb1 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 1 Jun 2017 11:20:36 -0700 Subject: [PATCH 1/7] Update server-console to watch interface via pid rather than marker --- interface/src/main.cpp | 2 +- libraries/networking/src/SandboxUtils.cpp | 8 ++++---- libraries/networking/src/SandboxUtils.h | 2 +- server-console/src/main.js | 22 ++++++++++++++++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 63738d2d91..a22130377a 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -199,7 +199,7 @@ int main(int argc, const char* argv[]) { bool serverContentPathOptionIsSet = parser.isSet(serverContentPathOption); QString serverContentPath = serverContentPathOptionIsSet ? parser.value(serverContentPathOption) : QString(); if (runServer) { - SandboxUtils::runLocalSandbox(serverContentPath, true, RUNNING_MARKER_FILENAME, noUpdater); + SandboxUtils::runLocalSandbox(serverContentPath, true, noUpdater); } Application app(argc, const_cast(argv), startupTime, runningMarkerExisted); diff --git a/libraries/networking/src/SandboxUtils.cpp b/libraries/networking/src/SandboxUtils.cpp index d816f7ebee..c62bd8f982 100644 --- a/libraries/networking/src/SandboxUtils.cpp +++ b/libraries/networking/src/SandboxUtils.cpp @@ -52,9 +52,8 @@ bool readStatus(QByteArray statusData) { return false; } -void runLocalSandbox(QString contentPath, bool autoShutdown, QString runningMarkerName, bool noUpdater) { +void runLocalSandbox(QString contentPath, bool autoShutdown, bool noUpdater) { QString serverPath = "./server-console/server-console.exe"; - qCDebug(networking) << "Running marker path is: " << runningMarkerName; qCDebug(networking) << "Server path is: " << serverPath; qCDebug(networking) << "autoShutdown: " << autoShutdown; qCDebug(networking) << "noUpdater: " << noUpdater; @@ -74,8 +73,9 @@ void runLocalSandbox(QString contentPath, bool autoShutdown, QString runningMark } if (autoShutdown) { - QString interfaceRunningStateFile = RunningMarker::getMarkerFilePath(runningMarkerName); - args << "--shutdownWatcher" << interfaceRunningStateFile; + auto pid = qApp->applicationPid(); + qCDebug(networking) << "autoShutdown pid is" << pid; + args << "--watchProcessShutdown" << QString::number(pid); } if (noUpdater) { diff --git a/libraries/networking/src/SandboxUtils.h b/libraries/networking/src/SandboxUtils.h index 42484b8edf..370b28e1b0 100644 --- a/libraries/networking/src/SandboxUtils.h +++ b/libraries/networking/src/SandboxUtils.h @@ -21,7 +21,7 @@ namespace SandboxUtils { QNetworkReply* getStatus(); bool readStatus(QByteArray statusData); - void runLocalSandbox(QString contentPath, bool autoShutdown, QString runningMarkerName, bool noUpdater); + void runLocalSandbox(QString contentPath, bool autoShutdown, bool noUpdater); }; #endif // hifi_SandboxUtils_h diff --git a/server-console/src/main.js b/server-console/src/main.js index 98bda9a10f..725c6ca0c8 100644 --- a/server-console/src/main.js +++ b/server-console/src/main.js @@ -821,6 +821,15 @@ for (var key in trayIcons) { const notificationIcon = path.join(__dirname, '../resources/console-notification.png'); +function isProcessRunning(pid) { + try { + running = process.kill(pid, 0); + return true; + } catch (e) { + } + return false; +} + function onContentLoaded() { // Disable splash window for now. // maybeShowSplash(); @@ -882,6 +891,19 @@ function onContentLoaded() { startInterface(); } + if (argv.watchProcessShutdown) { + let pid = argv.watchProcessShutdown; + console.log("Watching process: ", pid); + let watchProcessInterval = setInterval(function() { + let isRunning = isProcessRunning(pid); + if (!isRunning) { + log.debug("Watched process is no longer running, shutting down"); + clearTimeout(watchProcessInterval); + forcedShutdown(); + } + }, 5000); + } + // If we were launched with the shutdownWatcher option, then we need to watch for the interface app // shutting down. The interface app will regularly update a running state file which we will check. // If the file doesn't exist or stops updating for a significant amount of time, we will shut down. From 34630f839f05d773db1e4d8f6947e1ea08cad84c Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 1 Jun 2017 12:48:42 -0700 Subject: [PATCH 2/7] Fix isProcessRunning error in strict mode --- server-console/src/main.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server-console/src/main.js b/server-console/src/main.js index 725c6ca0c8..55a45a4991 100644 --- a/server-console/src/main.js +++ b/server-console/src/main.js @@ -823,8 +823,7 @@ const notificationIcon = path.join(__dirname, '../resources/console-notification function isProcessRunning(pid) { try { - running = process.kill(pid, 0); - return true; + return process.kill(pid, 0); } catch (e) { } return false; From 37321814b6c2bc5164128a08a21a78bd028478d8 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 1 Jun 2017 18:13:27 -0700 Subject: [PATCH 3/7] Rename watchProcessShutdown to shutdownWith --- libraries/networking/src/SandboxUtils.cpp | 3 +-- server-console/src/main.js | 14 +++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/libraries/networking/src/SandboxUtils.cpp b/libraries/networking/src/SandboxUtils.cpp index c62bd8f982..f6c7634ce5 100644 --- a/libraries/networking/src/SandboxUtils.cpp +++ b/libraries/networking/src/SandboxUtils.cpp @@ -74,8 +74,7 @@ void runLocalSandbox(QString contentPath, bool autoShutdown, bool noUpdater) { if (autoShutdown) { auto pid = qApp->applicationPid(); - qCDebug(networking) << "autoShutdown pid is" << pid; - args << "--watchProcessShutdown" << QString::number(pid); + args << "--shutdownWith" << QString::number(pid); } if (noUpdater) { diff --git a/server-console/src/main.js b/server-console/src/main.js index 55a45a4991..6667a570c8 100644 --- a/server-console/src/main.js +++ b/server-console/src/main.js @@ -890,18 +890,18 @@ function onContentLoaded() { startInterface(); } - if (argv.watchProcessShutdown) { - let pid = argv.watchProcessShutdown; - console.log("Watching process: ", pid); - let watchProcessInterval = setInterval(function() { + // If we were launched with the shutdownWith option, then we need to shutdown when that process (pid) + // is no longer running. + if (argv.shutdownWith) { + let pid = argv.shutdownWith; + console.log("Shutting down with process: ", pid); + let checkProcessInterval = setInterval(function() { let isRunning = isProcessRunning(pid); if (!isRunning) { log.debug("Watched process is no longer running, shutting down"); - clearTimeout(watchProcessInterval); + clearTimeout(checkProcessInterval); forcedShutdown(); } - }, 5000); - } // If we were launched with the shutdownWatcher option, then we need to watch for the interface app // shutting down. The interface app will regularly update a running state file which we will check. From ccd473ad0cfccefa4833b03f544f0a5f2b7faf81 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 1 Jun 2017 18:13:40 -0700 Subject: [PATCH 4/7] Remove shutdownWatcher option inside sandbox --- server-console/src/main.js | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/server-console/src/main.js b/server-console/src/main.js index 6667a570c8..46cc472e04 100644 --- a/server-console/src/main.js +++ b/server-console/src/main.js @@ -902,32 +902,6 @@ function onContentLoaded() { clearTimeout(checkProcessInterval); forcedShutdown(); } - - // If we were launched with the shutdownWatcher option, then we need to watch for the interface app - // shutting down. The interface app will regularly update a running state file which we will check. - // If the file doesn't exist or stops updating for a significant amount of time, we will shut down. - if (argv.shutdownWatcher) { - log.debug("Shutdown watcher requested... argv.shutdownWatcher:", argv.shutdownWatcher); - var MAX_TIME_SINCE_EDIT = 5000; // 5 seconds between updates - var firstAttemptToCheck = new Date().getTime(); - var shutdownWatchInterval = setInterval(function(){ - var stats = fs.stat(argv.shutdownWatcher, function(err, stats) { - if (err) { - var sinceFirstCheck = new Date().getTime() - firstAttemptToCheck; - if (sinceFirstCheck > MAX_TIME_SINCE_EDIT) { - log.debug("Running state file is missing, assume interface has shutdown... shutting down snadbox."); - forcedShutdown(); - clearTimeout(shutdownWatchInterval); - } - } else { - var sinceEdit = new Date().getTime() - stats.mtime.getTime(); - if (sinceEdit > MAX_TIME_SINCE_EDIT) { - log.debug("Running state of interface hasn't updated in MAX time... shutting down."); - forcedShutdown(); - clearTimeout(shutdownWatchInterval); - } - } - }); }, 1000); } } From 3888385a7294556c617283baa839dd2e99474b0f Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 1 Jun 2017 18:19:13 -0700 Subject: [PATCH 5/7] Remove the runningMarker timer --- interface/src/main.cpp | 5 +--- libraries/shared/src/RunningMarker.cpp | 35 +------------------------- libraries/shared/src/RunningMarker.h | 12 +-------- 3 files changed, 3 insertions(+), 49 deletions(-) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index a22130377a..83cac6d9bb 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -190,7 +190,7 @@ int main(int argc, const char* argv[]) { int exitCode; { - RunningMarker runningMarker(nullptr, RUNNING_MARKER_FILENAME); + RunningMarker runningMarker(RUNNING_MARKER_FILENAME); bool runningMarkerExisted = runningMarker.fileExists(); runningMarker.writeRunningMarkerFile(); @@ -204,9 +204,6 @@ int main(int argc, const char* argv[]) { Application app(argc, const_cast(argv), startupTime, runningMarkerExisted); - // Now that the main event loop is setup, launch running marker thread - runningMarker.startRunningMarker(); - // If we failed the OpenGLVersion check, log it. if (override) { auto accountManager = DependencyManager::get(); diff --git a/libraries/shared/src/RunningMarker.cpp b/libraries/shared/src/RunningMarker.cpp index 0c1fd06df8..cb7b39320c 100644 --- a/libraries/shared/src/RunningMarker.cpp +++ b/libraries/shared/src/RunningMarker.cpp @@ -13,44 +13,16 @@ #include #include -#include -#include -#include "NumericalConstants.h" #include "PathUtils.h" -RunningMarker::RunningMarker(QObject* parent, QString name) : - _parent(parent), +RunningMarker::RunningMarker(QString name) : _name(name) { } -void RunningMarker::startRunningMarker() { - static const int RUNNING_STATE_CHECK_IN_MSECS = MSECS_PER_SECOND; - - // start the nodeThread so its event loop is running - _runningMarkerThread = new QThread(_parent); - _runningMarkerThread->setObjectName("Running Marker Thread"); - _runningMarkerThread->start(); - - writeRunningMarkerFile(); // write the first file, even before timer - - _runningMarkerTimer = new QTimer(); - QObject::connect(_runningMarkerTimer, &QTimer::timeout, [=](){ - writeRunningMarkerFile(); - }); - _runningMarkerTimer->start(RUNNING_STATE_CHECK_IN_MSECS); - - // put the time on the thread - _runningMarkerTimer->moveToThread(_runningMarkerThread); -} - RunningMarker::~RunningMarker() { deleteRunningMarkerFile(); - QMetaObject::invokeMethod(_runningMarkerTimer, "stop", Qt::BlockingQueuedConnection); - _runningMarkerThread->quit(); - _runningMarkerTimer->deleteLater(); - _runningMarkerThread->deleteLater(); } bool RunningMarker::fileExists() const { @@ -77,8 +49,3 @@ void RunningMarker::deleteRunningMarkerFile() { QString RunningMarker::getFilePath() const { return QStandardPaths::writableLocation(QStandardPaths::DataLocation) + "/" + _name; } - -QString RunningMarker::getMarkerFilePath(QString name) { - return QStandardPaths::writableLocation(QStandardPaths::DataLocation) + "/" + name; -} - diff --git a/libraries/shared/src/RunningMarker.h b/libraries/shared/src/RunningMarker.h index f9c8e72d37..ae7c550660 100644 --- a/libraries/shared/src/RunningMarker.h +++ b/libraries/shared/src/RunningMarker.h @@ -12,21 +12,14 @@ #ifndef hifi_RunningMarker_h #define hifi_RunningMarker_h -#include #include -class QThread; -class QTimer; - class RunningMarker { public: - RunningMarker(QObject* parent, QString name); + RunningMarker(QString name); ~RunningMarker(); - void startRunningMarker(); - QString getFilePath() const; - static QString getMarkerFilePath(QString name); bool fileExists() const; @@ -34,10 +27,7 @@ public: void deleteRunningMarkerFile(); private: - QObject* _parent { nullptr }; QString _name; - QThread* _runningMarkerThread { nullptr }; - QTimer* _runningMarkerTimer { nullptr }; }; #endif // hifi_RunningMarker_h From f10f717ea205917873b721a4caa23c1758b340a8 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Fri, 2 Jun 2017 08:58:35 -0700 Subject: [PATCH 6/7] Add clarifying comment to use of kill in isProcessRunning --- server-console/src/main.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server-console/src/main.js b/server-console/src/main.js index 46cc472e04..408a17bd56 100644 --- a/server-console/src/main.js +++ b/server-console/src/main.js @@ -823,6 +823,9 @@ const notificationIcon = path.join(__dirname, '../resources/console-notification function isProcessRunning(pid) { try { + // Sending a signal of 0 is effectively a NOOP. + // If sending the signal is successful, kill will return true. + // If the process is not running, an exception will be thrown. return process.kill(pid, 0); } catch (e) { } From 2146f96091a4b0462eb61d0ee3e33a47b72ee61d Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Fri, 2 Jun 2017 13:51:39 -0700 Subject: [PATCH 7/7] Replace static use of qApp with QCoreApplication --- libraries/networking/src/SandboxUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/SandboxUtils.cpp b/libraries/networking/src/SandboxUtils.cpp index f6c7634ce5..4a348b0662 100644 --- a/libraries/networking/src/SandboxUtils.cpp +++ b/libraries/networking/src/SandboxUtils.cpp @@ -73,7 +73,7 @@ void runLocalSandbox(QString contentPath, bool autoShutdown, bool noUpdater) { } if (autoShutdown) { - auto pid = qApp->applicationPid(); + auto pid = QCoreApplication::applicationPid(); args << "--shutdownWith" << QString::number(pid); }