From d75d204ff6ef190411410bf4b2a3915485e4e393 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 7 May 2015 17:03:43 -0700 Subject: [PATCH 01/39] standardize cleanup of ThreadedAssignments --- assignment-client/src/AssignmentClient.cpp | 69 ++++++++++++------- assignment-client/src/AssignmentClient.h | 4 +- assignment-client/src/AssignmentThread.cpp | 19 ----- assignment-client/src/AssignmentThread.h | 26 ------- assignment-client/src/audio/AudioMixer.cpp | 2 +- assignment-client/src/avatars/AvatarMixer.cpp | 3 +- .../src/octree/OctreeQueryNode.cpp | 4 +- .../src/octree/OctreeQueryNode.h | 4 +- .../src/octree/OctreeSendThread.cpp | 11 +-- .../src/octree/OctreeSendThread.h | 3 +- assignment-client/src/octree/OctreeServer.cpp | 8 +-- .../networking/src/ThreadedAssignment.cpp | 62 +++++++++-------- libraries/networking/src/ThreadedAssignment.h | 6 +- 13 files changed, 90 insertions(+), 131 deletions(-) delete mode 100644 assignment-client/src/AssignmentThread.cpp delete mode 100644 assignment-client/src/AssignmentThread.h diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 39dcf4b29c..5702f26500 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -9,6 +9,8 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // +#include + #include #include #include @@ -30,15 +32,12 @@ #include #include "AssignmentFactory.h" -#include "AssignmentThread.h" #include "AssignmentClient.h" const QString ASSIGNMENT_CLIENT_TARGET_NAME = "assignment-client"; const long long ASSIGNMENT_REQUEST_INTERVAL_MSECS = 1 * 1000; -SharedAssignmentPointer AssignmentClient::_currentAssignment; - int hifiSockAddrMeta = qRegisterMetaType("HifiSockAddr"); AssignmentClient::AssignmentClient(int ppid, Assignment::Type requestAssignmentType, QString assignmentPool, @@ -115,12 +114,22 @@ AssignmentClient::AssignmentClient(int ppid, Assignment::Type requestAssignmentT void AssignmentClient::stopAssignmentClient() { - qDebug() << "Exiting."; + qDebug() << "Forced stop of assignment-client."; + _requestTimer.stop(); _statsTimerACM.stop(); + if (_currentAssignment) { - _currentAssignment->aboutToQuit(); + // grab the thread for the current assignment QThread* currentAssignmentThread = _currentAssignment->thread(); + + // ask the current assignment to stop + QMetaObject::invokeMethod(_currentAssignment, "stop", Qt::BlockingQueuedConnection); + + // ask the current assignment to delete itself on its thread + _currentAssignment->deleteLater(); + + // wait on the thread from that assignment - it will be gone once the current assignment deletes currentAssignmentThread->quit(); currentAssignmentThread->wait(); } @@ -129,12 +138,9 @@ void AssignmentClient::stopAssignmentClient() { void AssignmentClient::aboutToQuit() { stopAssignmentClient(); + // clear the log handler so that Qt doesn't call the destructor on LogHandler - qInstallMessageHandler(0); - // clear out pointer to the assignment so the destructor gets called. if we don't do this here, - // it will get destroyed along with all the other "static" stuff. various static member variables - // will be destroyed first and things go wrong. - _currentAssignment.clear(); + qInstallMessageHandler(0); } @@ -203,7 +209,7 @@ void AssignmentClient::readPendingDatagrams() { if (nodeList->packetVersionAndHashMatch(receivedPacket)) { if (packetTypeForPacket(receivedPacket) == PacketTypeCreateAssignment) { // construct the deployed assignment from the packet data - _currentAssignment = SharedAssignmentPointer(AssignmentFactory::unpackAssignment(receivedPacket)); + _currentAssignment = AssignmentFactory::unpackAssignment(receivedPacket); if (_currentAssignment) { qDebug() << "Received an assignment -" << *_currentAssignment; @@ -216,13 +222,23 @@ void AssignmentClient::readPendingDatagrams() { qDebug() << "Destination IP for assignment is" << nodeList->getDomainHandler().getIP().toString(); // start the deployed assignment - AssignmentThread* workerThread = new AssignmentThread(_currentAssignment, this); - workerThread->setObjectName("worker"); + QThread* workerThread = new QThread; + qDebug() << "The thread is" << workerThread; + workerThread->setObjectName("ThreadedAssignment Worker"); - connect(workerThread, &QThread::started, _currentAssignment.data(), &ThreadedAssignment::run); - connect(_currentAssignment.data(), &ThreadedAssignment::finished, workerThread, &QThread::quit); - connect(_currentAssignment.data(), &ThreadedAssignment::finished, - this, &AssignmentClient::assignmentCompleted); + connect(workerThread, &QThread::started, _currentAssignment, &ThreadedAssignment::run); + + // once the ThreadedAssignment says it is finished - we ask it to deleteLater + connect(_currentAssignment, &ThreadedAssignment::finished, _currentAssignment, + &ThreadedAssignment::deleteLater); + + // once it is deleted, we take down the worker thread + connect(_currentAssignment, &ThreadedAssignment::destroyed, workerThread, &QThread::quit); + + // once the worker thread says it is done, we consider the assignment completed + connect(workerThread, &QThread::finished, this, &AssignmentClient::assignmentCompleted); + + // have the worker thread remove itself once it is done connect(workerThread, &QThread::finished, workerThread, &QThread::deleteLater); _currentAssignment->moveToThread(workerThread); @@ -232,7 +248,7 @@ void AssignmentClient::readPendingDatagrams() { // let the assignment handle the incoming datagrams for its duration disconnect(&nodeList->getNodeSocket(), 0, this, 0); - connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, _currentAssignment.data(), + connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, _currentAssignment, &ThreadedAssignment::readPendingDatagrams); // Starts an event loop, and emits workerThread->started() @@ -281,20 +297,23 @@ void AssignmentClient::handleAuthenticationRequest() { } void AssignmentClient::assignmentCompleted() { + + // we expect that to be here the previous assignment has completely cleaned up + assert(_currentAssignment.isNull()); + + // reset our current assignment pointer to NULL now that it has been deleted + _currentAssignment = NULL; + // reset the logging target to the the CHILD_TARGET_NAME LogHandler::getInstance().setTargetName(ASSIGNMENT_CLIENT_TARGET_NAME); - qDebug("Assignment finished or never started - waiting for new assignment."); + qDebug() << "Assignment finished or never started - waiting for new assignment."; auto nodeList = DependencyManager::get(); // have us handle incoming NodeList datagrams again - disconnect(&nodeList->getNodeSocket(), 0, _currentAssignment.data(), 0); - connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, this, &AssignmentClient::readPendingDatagrams); - - // clear our current assignment shared pointer now that we're done with it - // if the assignment thread is still around it has its own shared pointer to the assignment - _currentAssignment.clear(); + disconnect(&nodeList->getNodeSocket(), 0, _currentAssignment, 0); + connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, this, &AssignmentClient::readPendingDatagrams); // reset our NodeList by switching back to unassigned and clearing the list nodeList->setOwnerType(NodeType::Unassigned); diff --git a/assignment-client/src/AssignmentClient.h b/assignment-client/src/AssignmentClient.h index 1ffb862dd3..005652b7ef 100644 --- a/assignment-client/src/AssignmentClient.h +++ b/assignment-client/src/AssignmentClient.h @@ -13,6 +13,7 @@ #define hifi_AssignmentClient_h #include +#include #include "ThreadedAssignment.h" @@ -24,7 +25,6 @@ public: AssignmentClient(int ppid, Assignment::Type requestAssignmentType, QString assignmentPool, QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort); - static const SharedAssignmentPointer& getCurrentAssignment() { return _currentAssignment; } private slots: void sendAssignmentRequest(); @@ -40,7 +40,7 @@ public slots: private: void setUpStatsToMonitor(int ppid); Assignment _requestAssignment; - static SharedAssignmentPointer _currentAssignment; + QPointer _currentAssignment; QString _assignmentServerHostname; HifiSockAddr _assignmentServerSocket; QSharedMemory* _localASPortSharedMem; // memory shared with domain server diff --git a/assignment-client/src/AssignmentThread.cpp b/assignment-client/src/AssignmentThread.cpp deleted file mode 100644 index 848fa7afae..0000000000 --- a/assignment-client/src/AssignmentThread.cpp +++ /dev/null @@ -1,19 +0,0 @@ -// -// AssignmentThread.cpp -// assignment-client/src -// -// Created by Stephen Birarda on 2014. -// Copyright 2014 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#include "AssignmentThread.h" - -AssignmentThread::AssignmentThread(const SharedAssignmentPointer& assignment, QObject* parent) : - QThread(parent), - _assignment(assignment) -{ - -} diff --git a/assignment-client/src/AssignmentThread.h b/assignment-client/src/AssignmentThread.h deleted file mode 100644 index 848c4614ba..0000000000 --- a/assignment-client/src/AssignmentThread.h +++ /dev/null @@ -1,26 +0,0 @@ -// -// AssignmentThread.h -// assignment-client/src -// -// Created by Stephen Birarda on 2014. -// Copyright 2014 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#ifndef hifi_AssignmentThread_h -#define hifi_AssignmentThread_h - -#include - -#include - -class AssignmentThread : public QThread { -public: - AssignmentThread(const SharedAssignmentPointer& assignment, QObject* parent); -private: - SharedAssignmentPointer _assignment; -}; - -#endif // hifi_AssignmentThread_h diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 94439cb18b..852b7c5576 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -854,7 +854,7 @@ void AudioMixer::run() { ++_numStatFrames; QCoreApplication::processEvents(); - + if (_isFinished) { break; } diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 961cbecebb..01894293dc 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -52,6 +52,7 @@ AvatarMixer::~AvatarMixer() { if (_broadcastTimer) { _broadcastTimer->deleteLater(); } + _broadcastThread.quit(); _broadcastThread.wait(); } @@ -458,7 +459,7 @@ void AvatarMixer::run() { nodeList->linkedDataCreateCallback = attachAvatarDataToNode; // setup the timer that will be fired on the broadcast thread - _broadcastTimer = new QTimer(); + _broadcastTimer = new QTimer; _broadcastTimer->setInterval(AVATAR_DATA_SEND_INTERVAL_MSECS); _broadcastTimer->moveToThread(&_broadcastThread); diff --git a/assignment-client/src/octree/OctreeQueryNode.cpp b/assignment-client/src/octree/OctreeQueryNode.cpp index 2d8d8d357e..40da610a72 100644 --- a/assignment-client/src/octree/OctreeQueryNode.cpp +++ b/assignment-client/src/octree/OctreeQueryNode.cpp @@ -91,8 +91,8 @@ void OctreeQueryNode::sendThreadFinished() { } } -void OctreeQueryNode::initializeOctreeSendThread(const SharedAssignmentPointer& myAssignment, const SharedNodePointer& node) { - _octreeSendThread = new OctreeSendThread(myAssignment, node); +void OctreeQueryNode::initializeOctreeSendThread(OctreeServer* myServer, const SharedNodePointer& node) { + _octreeSendThread = new OctreeSendThread(myServer, node); // we want to be notified when the thread finishes connect(_octreeSendThread, &GenericThread::finished, this, &OctreeQueryNode::sendThreadFinished); diff --git a/assignment-client/src/octree/OctreeQueryNode.h b/assignment-client/src/octree/OctreeQueryNode.h index d7b660f18a..ea0f3240aa 100644 --- a/assignment-client/src/octree/OctreeQueryNode.h +++ b/assignment-client/src/octree/OctreeQueryNode.h @@ -22,11 +22,11 @@ #include #include #include -#include // for SharedAssignmentPointer #include "SentPacketHistory.h" #include class OctreeSendThread; +class OctreeServer; class OctreeQueryNode : public OctreeQuery { Q_OBJECT @@ -89,7 +89,7 @@ public: OctreeSceneStats stats; - void initializeOctreeSendThread(const SharedAssignmentPointer& myAssignment, const SharedNodePointer& node); + void initializeOctreeSendThread(OctreeServer* myServer, const SharedNodePointer& node); bool isOctreeSendThreadInitalized() { return _octreeSendThread; } void dumpOutOfView(); diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 4e61793910..f629ef7e72 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -21,9 +21,8 @@ quint64 startSceneSleepTime = 0; quint64 endSceneSleepTime = 0; -OctreeSendThread::OctreeSendThread(const SharedAssignmentPointer& myAssignment, const SharedNodePointer& node) : - _myAssignment(myAssignment), - _myServer(static_cast(myAssignment.data())), +OctreeSendThread::OctreeSendThread(OctreeServer* myServer, const SharedNodePointer& node) : + _myServer(myServer), _node(node), _nodeUUID(node->getUUID()), _packetData(), @@ -53,7 +52,6 @@ OctreeSendThread::~OctreeSendThread() { OctreeServer::stopTrackingThread(this); _node.clear(); - _myAssignment.clear(); } void OctreeSendThread::setIsShuttingDown() { @@ -66,11 +64,6 @@ bool OctreeSendThread::process() { return false; // exit early if we're shutting down } - // check that our server and assignment is still valid - if (!_myServer || !_myAssignment) { - return false; // exit early if it's not, it means the server is shutting down - } - OctreeServer::didProcess(this); quint64 start = usecTimestampNow(); diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index d0507fc83d..3316546913 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -26,7 +26,7 @@ class OctreeServer; class OctreeSendThread : public GenericThread { Q_OBJECT public: - OctreeSendThread(const SharedAssignmentPointer& myAssignment, const SharedNodePointer& node); + OctreeSendThread(OctreeServer* myServer, const SharedNodePointer& node); virtual ~OctreeSendThread(); void setIsShuttingDown(); @@ -43,7 +43,6 @@ protected: virtual bool process(); private: - SharedAssignmentPointer _myAssignment; OctreeServer* _myServer; SharedNodePointer _node; QUuid _nodeUUID; diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index 9abace0c5b..b994443b83 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -847,13 +847,7 @@ void OctreeServer::readPendingDatagram(const QByteArray& receivedPacket, const H nodeList->updateNodeWithDataFromPacket(matchingNode, receivedPacket); OctreeQueryNode* nodeData = (OctreeQueryNode*)matchingNode->getLinkedData(); if (nodeData && !nodeData->isOctreeSendThreadInitalized()) { - - // NOTE: this is an important aspect of the proper ref counting. The send threads/node data need to - // know that the OctreeServer/Assignment will not get deleted on it while it's still active. The - // solution is to get the shared pointer for the current assignment. We need to make sure this is the - // same SharedAssignmentPointer that was ref counted by the assignment client. - SharedAssignmentPointer sharedAssignment = AssignmentClient::getCurrentAssignment(); - nodeData->initializeOctreeSendThread(sharedAssignment, matchingNode); + nodeData->initializeOctreeSendThread(this, matchingNode); } } } else if (packetType == PacketTypeOctreeDataNack) { diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index 43bcce4530..595ff0df6c 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -27,40 +27,42 @@ ThreadedAssignment::ThreadedAssignment(const QByteArray& packet) : } void ThreadedAssignment::setFinished(bool isFinished) { - _isFinished = isFinished; + if (_isFinished != isFinished) { + _isFinished = isFinished; - if (_isFinished) { - if (_domainServerTimer) { - _domainServerTimer->stop(); - delete _domainServerTimer; - _domainServerTimer = nullptr; - } - if (_statsTimer) { - _statsTimer->stop(); - delete _statsTimer; - _statsTimer = nullptr; - } + if (_isFinished) { - aboutToFinish(); - - auto nodeList = DependencyManager::get(); - - // if we have a datagram processing thread, quit it and wait on it to make sure that - // the node socket is back on the same thread as the NodeList - - if (_datagramProcessingThread) { - // tell the datagram processing thread to quit and wait until it is done, then return the node socket to the NodeList - _datagramProcessingThread->quit(); - _datagramProcessingThread->wait(); + qDebug() << "Somebody set us finished"; + + if (_domainServerTimer) { + _domainServerTimer->stop(); + } + + if (_statsTimer) { + _statsTimer->stop(); + } + + aboutToFinish(); - // set node socket parent back to NodeList - nodeList->getNodeSocket().setParent(nodeList.data()); + auto nodeList = DependencyManager::get(); + + // if we have a datagram processing thread, quit it and wait on it to make sure that + // the node socket is back on the same thread as the NodeList + + if (_datagramProcessingThread) { + // tell the datagram processing thread to quit and wait until it is done, then return the node socket to the NodeList + _datagramProcessingThread->quit(); + _datagramProcessingThread->wait(); + + // set node socket parent back to NodeList + nodeList->getNodeSocket().setParent(nodeList.data()); + } + + // move the NodeList back to the QCoreApplication instance's thread + nodeList->moveToThread(QCoreApplication::instance()->thread()); + + emit finished(); } - - // move the NodeList back to the QCoreApplication instance's thread - nodeList->moveToThread(QCoreApplication::instance()->thread()); - - emit finished(); } } diff --git a/libraries/networking/src/ThreadedAssignment.h b/libraries/networking/src/ThreadedAssignment.h index 590c2f56ca..4e425be429 100644 --- a/libraries/networking/src/ThreadedAssignment.h +++ b/libraries/networking/src/ThreadedAssignment.h @@ -20,6 +20,7 @@ class ThreadedAssignment : public Assignment { Q_OBJECT public: ThreadedAssignment(const QByteArray& packet); + ~ThreadedAssignment() { stop(); } void setFinished(bool isFinished); virtual void aboutToFinish() { }; @@ -32,11 +33,6 @@ public slots: virtual void readPendingDatagrams() = 0; virtual void sendStatsPacket(); -public slots: - virtual void aboutToQuit() { - QMetaObject::invokeMethod(this, "stop"); - } - signals: void finished(); From 4c8c24eb00d62f9d4e18841b9ad71a6b288c9bb5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 7 May 2015 17:04:45 -0700 Subject: [PATCH 02/39] remove an extra debug --- assignment-client/src/AssignmentClient.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 5702f26500..80b8b2137d 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -223,7 +223,6 @@ void AssignmentClient::readPendingDatagrams() { // start the deployed assignment QThread* workerThread = new QThread; - qDebug() << "The thread is" << workerThread; workerThread->setObjectName("ThreadedAssignment Worker"); connect(workerThread, &QThread::started, _currentAssignment, &ThreadedAssignment::run); From a3ce02181e0835f160b73d7af86d69ef14aba0dd Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 7 May 2015 17:06:04 -0700 Subject: [PATCH 03/39] fix ThreadedAssignment setFinished debug --- libraries/networking/src/ThreadedAssignment.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index 595ff0df6c..ee608411ca 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -32,7 +32,7 @@ void ThreadedAssignment::setFinished(bool isFinished) { if (_isFinished) { - qDebug() << "Somebody set us finished"; + qDebug() << "ThreadedAssignment::setFinished(true) called - finishing up."; if (_domainServerTimer) { _domainServerTimer->stop(); From 3e0029e6e6225abb2c1e0c3c445ad66b246c0872 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 7 May 2015 17:29:31 -0700 Subject: [PATCH 04/39] use .data on QPointer for slot connection --- assignment-client/src/AssignmentClient.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 80b8b2137d..f8c995a999 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -228,11 +228,11 @@ void AssignmentClient::readPendingDatagrams() { connect(workerThread, &QThread::started, _currentAssignment, &ThreadedAssignment::run); // once the ThreadedAssignment says it is finished - we ask it to deleteLater - connect(_currentAssignment, &ThreadedAssignment::finished, _currentAssignment, + connect(_currentAssignment.data(), &ThreadedAssignment::finished, _currentAssignment, &ThreadedAssignment::deleteLater); // once it is deleted, we take down the worker thread - connect(_currentAssignment, &ThreadedAssignment::destroyed, workerThread, &QThread::quit); + connect(_currentAssignment.data(), &ThreadedAssignment::destroyed, workerThread, &QThread::quit); // once the worker thread says it is done, we consider the assignment completed connect(workerThread, &QThread::finished, this, &AssignmentClient::assignmentCompleted); @@ -247,7 +247,7 @@ void AssignmentClient::readPendingDatagrams() { // let the assignment handle the incoming datagrams for its duration disconnect(&nodeList->getNodeSocket(), 0, this, 0); - connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, _currentAssignment, + connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, _currentAssignment.data(), &ThreadedAssignment::readPendingDatagrams); // Starts an event loop, and emits workerThread->started() From bab96a77117bc206657179fdd7546125d8fe34c0 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 09:29:29 -0700 Subject: [PATCH 05/39] more connect call fixes in AssignmentClient --- assignment-client/src/AssignmentClient.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index f8c995a999..9fa519b0d6 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -225,10 +225,10 @@ void AssignmentClient::readPendingDatagrams() { QThread* workerThread = new QThread; workerThread->setObjectName("ThreadedAssignment Worker"); - connect(workerThread, &QThread::started, _currentAssignment, &ThreadedAssignment::run); + connect(workerThread, &QThread::started, _currentAssignment.data(), &ThreadedAssignment::run); // once the ThreadedAssignment says it is finished - we ask it to deleteLater - connect(_currentAssignment.data(), &ThreadedAssignment::finished, _currentAssignment, + connect(_currentAssignment.data(), &ThreadedAssignment::finished, _currentAssignment.data(), &ThreadedAssignment::deleteLater); // once it is deleted, we take down the worker thread From 0f71e93c7f95197362ea736d9c22af45349e810d Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 10:12:14 -0700 Subject: [PATCH 06/39] ensure no new OST creation on OctreeServer shutdown --- assignment-client/src/octree/OctreeServer.cpp | 9 ++++++++- libraries/networking/src/NodeList.cpp | 5 ++++- libraries/networking/src/ThreadedAssignment.cpp | 11 ++++++++--- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index b994443b83..41b46c0947 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -1216,6 +1216,11 @@ void OctreeServer::aboutToFinish() { qDebug() << qPrintable(_safeServerName) << "server STARTING about to finish..."; qDebug() << qPrintable(_safeServerName) << "inform Octree Inbound Packet Processor that we are shutting down..."; + // we're going down - set the NodeList linkedDataCallback to NULL so we do not create any more OctreeSendThreads + // this ensures that when we forceNodeShutdown below for each node we don't get any more newly connecting nodes + auto nodeList = DependencyManager::get(); + nodeList->linkedDataCreateCallback = NULL; + if (_octreeInboundPacketProcessor) { _octreeInboundPacketProcessor->terminating(); } @@ -1224,7 +1229,9 @@ void OctreeServer::aboutToFinish() { _jurisdictionSender->terminating(); } - DependencyManager::get()->eachNode([this](const SharedNodePointer& node) { + // force a shutdown of all of our OctreeSendThreads - at this point it has to be impossible for a + // linkedDataCreateCallback to be called for a new node + nodeList->eachNode([this](const SharedNodePointer& node) { qDebug() << qPrintable(_safeServerName) << "server about to finish while node still connected node:" << *node; forceNodeShutdown(node); }); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 819c46cdf0..b30afb91de 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -150,7 +150,10 @@ void NodeList::timePingReply(const QByteArray& packet, const SharedNodePointer& void NodeList::processNodeData(const HifiSockAddr& senderSockAddr, const QByteArray& packet) { switch (packetTypeForPacket(packet)) { case PacketTypeDomainList: { - processDomainServerList(packet); + if (_domainHandler.isConnected()) { + // only process a list from domain-server if we think we're connected + processDomainServerList(packet); + } break; } case PacketTypeDomainServerRequireDTLS: { diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index ee608411ca..f2b4018495 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -41,16 +41,21 @@ void ThreadedAssignment::setFinished(bool isFinished) { if (_statsTimer) { _statsTimer->stop(); } - - aboutToFinish(); + // stop processing datagrams from the node socket + // this ensures we won't process a domain list while we are going down auto nodeList = DependencyManager::get(); + disconnect(&nodeList->getNodeSocket(), 0, this, 0); + + // call our virtual aboutToFinish method - this gives the ThreadedAssignment subclass a chance to cleanup + aboutToFinish(); // if we have a datagram processing thread, quit it and wait on it to make sure that // the node socket is back on the same thread as the NodeList if (_datagramProcessingThread) { - // tell the datagram processing thread to quit and wait until it is done, then return the node socket to the NodeList + // tell the datagram processing thread to quit and wait until it is done, + // then return the node socket to the NodeList _datagramProcessingThread->quit(); _datagramProcessingThread->wait(); From b080dfc1cf948738e72a2cf143c63df5e1ff7dc7 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 10:16:39 -0700 Subject: [PATCH 07/39] change comment for AC readyRead connect --- assignment-client/src/AssignmentClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 9fa519b0d6..f163c57d02 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -310,7 +310,7 @@ void AssignmentClient::assignmentCompleted() { auto nodeList = DependencyManager::get(); - // have us handle incoming NodeList datagrams again + // have us handle incoming NodeList datagrams again, and make sure our ThreadedAssignment isn't handling them disconnect(&nodeList->getNodeSocket(), 0, _currentAssignment, 0); connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, this, &AssignmentClient::readPendingDatagrams); From be58347aecdd902c05327f652432c5b88ab3e2a3 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 10:40:04 -0700 Subject: [PATCH 08/39] consider assignment completed when worker thread is gone --- assignment-client/src/AssignmentClient.cpp | 9 ++++----- libraries/networking/src/NodeList.cpp | 5 +++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index f163c57d02..fe845fca4a 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -231,15 +231,15 @@ void AssignmentClient::readPendingDatagrams() { connect(_currentAssignment.data(), &ThreadedAssignment::finished, _currentAssignment.data(), &ThreadedAssignment::deleteLater); - // once it is deleted, we take down the worker thread + // once it is deleted, we quit the worker thread connect(_currentAssignment.data(), &ThreadedAssignment::destroyed, workerThread, &QThread::quit); - // once the worker thread says it is done, we consider the assignment completed - connect(workerThread, &QThread::finished, this, &AssignmentClient::assignmentCompleted); - // have the worker thread remove itself once it is done connect(workerThread, &QThread::finished, workerThread, &QThread::deleteLater); + // once the worker thread says it is done, we consider the assignment completed + connect(workerThread, &QThread::destroyed, this, &AssignmentClient::assignmentCompleted); + _currentAssignment->moveToThread(workerThread); // move the NodeList to the thread used for the _current assignment @@ -311,7 +311,6 @@ void AssignmentClient::assignmentCompleted() { auto nodeList = DependencyManager::get(); // have us handle incoming NodeList datagrams again, and make sure our ThreadedAssignment isn't handling them - disconnect(&nodeList->getNodeSocket(), 0, _currentAssignment, 0); connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, this, &AssignmentClient::readPendingDatagrams); // reset our NodeList by switching back to unassigned and clearing the list diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index b30afb91de..5244c44eee 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -150,8 +150,9 @@ void NodeList::timePingReply(const QByteArray& packet, const SharedNodePointer& void NodeList::processNodeData(const HifiSockAddr& senderSockAddr, const QByteArray& packet) { switch (packetTypeForPacket(packet)) { case PacketTypeDomainList: { - if (_domainHandler.isConnected()) { - // only process a list from domain-server if we think we're connected + if (!_domainHandler.getSockAddr().isNull()) { + // only process a list from domain-server if we're talking to a domain + // TODO: how do we make sure this is actually the domain we want the list from (DTLS probably) processDomainServerList(packet); } break; From fe1b8cc52b0e4335528973cb1365d9831bc4990f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 11:04:22 -0700 Subject: [PATCH 09/39] fix entity tree reset / add OST assert for myServer --- assignment-client/src/Agent.cpp | 4 ++++ assignment-client/src/octree/OctreeSendThread.cpp | 3 +++ 2 files changed, 7 insertions(+) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 098460ecac..ffeb5a79e0 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -224,5 +224,9 @@ void Agent::run() { void Agent::aboutToFinish() { _scriptEngine.stop(); + + // our entity tree is going to go away so tell that to the EntityScriptingInterface + DependencyManager::get()->setEntityTree(NULL); + NetworkAccessManager::getInstance().clearAccessCache(); } diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index f629ef7e72..c979017696 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -68,6 +68,9 @@ bool OctreeSendThread::process() { quint64 start = usecTimestampNow(); + // we'd better have a server at this point, or we're in trouble + assert(_myServer); + // don't do any send processing until the initial load of the octree is complete... if (_myServer->isInitialLoadComplete()) { if (_node) { From 3d06a86670fc6482cde53297b1b5c6afe921cec4 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 11:57:25 -0700 Subject: [PATCH 10/39] drop packets on floor when shutting down --- .../src/octree/OctreeQueryNode.cpp | 10 ++- .../src/octree/OctreeSendThread.cpp | 5 ++ assignment-client/src/octree/OctreeServer.cpp | 61 +++++++++++-------- assignment-client/src/octree/OctreeServer.h | 4 +- libraries/shared/src/GenericThread.cpp | 3 + 5 files changed, 53 insertions(+), 30 deletions(-) diff --git a/assignment-client/src/octree/OctreeQueryNode.cpp b/assignment-client/src/octree/OctreeQueryNode.cpp index 40da610a72..05697b4ba3 100644 --- a/assignment-client/src/octree/OctreeQueryNode.cpp +++ b/assignment-client/src/octree/OctreeQueryNode.cpp @@ -9,11 +9,15 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include "PacketHeaders.h" -#include "SharedUtil.h" #include "OctreeQueryNode.h" + #include #include + +#include +#include +#include + #include "OctreeSendThread.h" OctreeQueryNode::OctreeQueryNode() : @@ -92,7 +96,7 @@ void OctreeQueryNode::sendThreadFinished() { } void OctreeQueryNode::initializeOctreeSendThread(OctreeServer* myServer, const SharedNodePointer& node) { - _octreeSendThread = new OctreeSendThread(myServer, node); + _octreeSendThread = new OctreeSendThread(myServer, node); // we want to be notified when the thread finishes connect(_octreeSendThread, &GenericThread::finished, this, &OctreeQueryNode::sendThreadFinished); diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index c979017696..3a3eac161b 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -30,9 +30,14 @@ OctreeSendThread::OctreeSendThread(OctreeServer* myServer, const SharedNodePoint _isShuttingDown(false) { QString safeServerName("Octree"); + + // set our QThread object name so we can identify this thread while debugging + setObjectName(QString("Octree Send Thread (%1)").arg(uuidStringWithoutCurlyBraces(node->getUUID()))); + if (_myServer) { safeServerName = _myServer->getMyServerName(); } + qDebug() << qPrintable(safeServerName) << "server [" << _myServer << "]: client connected " "- starting sending thread [" << this << "]"; diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index 41b46c0947..d2c6cfc4cd 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -836,36 +836,42 @@ void OctreeServer::parsePayload() { void OctreeServer::readPendingDatagram(const QByteArray& receivedPacket, const HifiSockAddr& senderSockAddr) { auto nodeList = DependencyManager::get(); + + // If we know we're shutting down we just drop these packets on the floor. + // This stops us from initializing send threads we just shut down. - if (nodeList->packetVersionAndHashMatch(receivedPacket)) { - PacketType packetType = packetTypeForPacket(receivedPacket); - SharedNodePointer matchingNode = nodeList->sendingNodeForPacket(receivedPacket); - if (packetType == getMyQueryMessageType()) { - // If we got a query packet, then we're talking to an agent, and we - // need to make sure we have it in our nodeList. - if (matchingNode) { - nodeList->updateNodeWithDataFromPacket(matchingNode, receivedPacket); - OctreeQueryNode* nodeData = (OctreeQueryNode*)matchingNode->getLinkedData(); - if (nodeData && !nodeData->isOctreeSendThreadInitalized()) { - nodeData->initializeOctreeSendThread(this, matchingNode); + if (!_isShuttingDown) { + if (nodeList->packetVersionAndHashMatch(receivedPacket)) { + PacketType packetType = packetTypeForPacket(receivedPacket); + SharedNodePointer matchingNode = nodeList->sendingNodeForPacket(receivedPacket); + if (packetType == getMyQueryMessageType()) { + // If we got a query packet, then we're talking to an agent, and we + // need to make sure we have it in our nodeList. + if (matchingNode) { + nodeList->updateNodeWithDataFromPacket(matchingNode, receivedPacket); + + OctreeQueryNode* nodeData = (OctreeQueryNode*) matchingNode->getLinkedData(); + if (nodeData && !nodeData->isOctreeSendThreadInitalized()) { + nodeData->initializeOctreeSendThread(this, matchingNode); + } } - } - } else if (packetType == PacketTypeOctreeDataNack) { - // If we got a nack packet, then we're talking to an agent, and we - // need to make sure we have it in our nodeList. - if (matchingNode) { - OctreeQueryNode* nodeData = (OctreeQueryNode*)matchingNode->getLinkedData(); - if (nodeData) { - nodeData->parseNackPacket(receivedPacket); + } else if (packetType == PacketTypeOctreeDataNack) { + // If we got a nack packet, then we're talking to an agent, and we + // need to make sure we have it in our nodeList. + if (matchingNode) { + OctreeQueryNode* nodeData = (OctreeQueryNode*)matchingNode->getLinkedData(); + if (nodeData) { + nodeData->parseNackPacket(receivedPacket); + } } + } else if (packetType == PacketTypeJurisdictionRequest) { + _jurisdictionSender->queueReceivedPacket(matchingNode, receivedPacket); + } else if (_octreeInboundPacketProcessor && getOctree()->handlesEditPacketType(packetType)) { + _octreeInboundPacketProcessor->queueReceivedPacket(matchingNode, receivedPacket); + } else { + // let processNodeData handle it. + DependencyManager::get()->processNodeData(senderSockAddr, receivedPacket); } - } else if (packetType == PacketTypeJurisdictionRequest) { - _jurisdictionSender->queueReceivedPacket(matchingNode, receivedPacket); - } else if (_octreeInboundPacketProcessor && getOctree()->handlesEditPacketType(packetType)) { - _octreeInboundPacketProcessor->queueReceivedPacket(matchingNode, receivedPacket); - } else { - // let processNodeData handle it. - DependencyManager::get()->processNodeData(senderSockAddr, receivedPacket); } } } @@ -1214,6 +1220,9 @@ void OctreeServer::forceNodeShutdown(SharedNodePointer node) { void OctreeServer::aboutToFinish() { qDebug() << qPrintable(_safeServerName) << "server STARTING about to finish..."; + + _isShuttingDown = true; + qDebug() << qPrintable(_safeServerName) << "inform Octree Inbound Packet Processor that we are shutting down..."; // we're going down - set the NodeList linkedDataCallback to NULL so we do not create any more OctreeSendThreads diff --git a/assignment-client/src/octree/OctreeServer.h b/assignment-client/src/octree/OctreeServer.h index 41cd3259cf..8b8a0185e8 100644 --- a/assignment-client/src/octree/OctreeServer.h +++ b/assignment-client/src/octree/OctreeServer.h @@ -146,12 +146,14 @@ protected: QString getStatusLink(); void setupDatagramProcessingThread(); - + int _argc; const char** _argv; char** _parsedArgV; QJsonObject _settings; + bool _isShuttingDown = false; + HTTPManager* _httpManager; int _statusPort; QString _statusHost; diff --git a/libraries/shared/src/GenericThread.cpp b/libraries/shared/src/GenericThread.cpp index 4be253f045..3068ca2ab6 100644 --- a/libraries/shared/src/GenericThread.cpp +++ b/libraries/shared/src/GenericThread.cpp @@ -32,6 +32,9 @@ void GenericThread::initialize(bool isThreaded) { if (_isThreaded) { _thread = new QThread(this); + // match the thread name to our object name + _thread->setObjectName(objectName()); + // when the worker thread is started, call our engine's run.. connect(_thread, SIGNAL(started()), this, SLOT(threadRoutine())); From 69143d8677a814297a8eaae25b667d73bdf059c4 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 11:57:38 -0700 Subject: [PATCH 11/39] add a script for server load testing --- .../utilities/diagnostics/loadTestServers.js | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 examples/utilities/diagnostics/loadTestServers.js diff --git a/examples/utilities/diagnostics/loadTestServers.js b/examples/utilities/diagnostics/loadTestServers.js new file mode 100644 index 0000000000..789273c5e0 --- /dev/null +++ b/examples/utilities/diagnostics/loadTestServers.js @@ -0,0 +1,68 @@ +// +// loadTestServers.js +// examples/utilities/diagnostics +// +// Created by Stephen Birarda on 05/08/15. +// Copyright 2015 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// +// This script is made for load testing HF servers. It connects to the HF servers and sends/receives data. +// Run this on an assignment-client. +// + +var count = 0; +var yawDirection = -1; +var yaw = 45; +var yawMax = 70; +var yawMin = 20; +var vantagePoint = {x: 5000, y: 500, z: 5000}; + +var isLocal = false; + +// set up our EntityViewer with a position and orientation +var orientation = Quat.fromPitchYawRollDegrees(0, yaw, 0); + +EntityViewer.setPosition(vantagePoint); +EntityViewer.setOrientation(orientation); +EntityViewer.queryOctree(); + +Agent.isListeningToAudioStream = true; +Agent.isAvatar = true; + +function getRandomInt(min, max) { + return Math.floor(Math.random() * (max - min + 1)) + min; +} + +function keepLooking(deltaTime) { + count++; + + if (count % getRandomInt(5, 15) == 0) { + yaw += yawDirection; + orientation = Quat.fromPitchYawRollDegrees(0, yaw, 0); + + if (yaw > yawMax || yaw < yawMin) { + yawDirection = yawDirection * -1; + } + + EntityViewer.setOrientation(orientation); + EntityViewer.queryOctree(); + + } + + // approximately every second, consider stopping + if (count % 60 == 0) { + print("considering stop.... elementCount:" + EntityViewer.getOctreeElementsCount()); + + var stopProbability = 0.05; // 5% chance of stopping + + if (Math.random() < stopProbability) { + print("stopping.... elementCount:" + EntityViewer.getOctreeElementsCount()); + Script.stop(); + } + } +} + +// register the call back so it fires before each data send +Script.update.connect(keepLooking); From df6ee4c7a537b4c74df55187f14b6087cf0b0486 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 12:10:13 -0700 Subject: [PATCH 12/39] immediately terminate instead of attempting network stop --- assignment-client/src/AssignmentClientMonitor.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 12f5abc796..bb1e56e886 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -85,17 +85,7 @@ void AssignmentClientMonitor::waitOnChildren(int msecs) { void AssignmentClientMonitor::stopChildProcesses() { auto nodeList = DependencyManager::get(); - nodeList->eachNode([&](const SharedNodePointer& node) { - qDebug() << "asking child" << node->getUUID() << "to exit."; - node->activateLocalSocket(); - QByteArray diePacket = byteArrayWithPopulatedHeader(PacketTypeStopNode); - nodeList->writeUnverifiedDatagram(diePacket, *node->getActiveSocket()); - }); - - // try to give all the children time to shutdown - waitOnChildren(WAIT_FOR_CHILD_MSECS); - - // ask more firmly + // ask child processes to terminate QMutableListIterator i(_childProcesses); while (i.hasNext()) { QProcess* childProcess = i.next(); From 99a9f2df90d1d08d11c7ff18cef61df94fad18e8 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 14:02:19 -0700 Subject: [PATCH 13/39] don't allow an AC monitor to take over other local ACs --- assignment-client/src/AssignmentClient.cpp | 38 ++++++++++--------- assignment-client/src/AssignmentClient.h | 9 +++-- assignment-client/src/AssignmentClientApp.cpp | 15 ++++---- assignment-client/src/AssignmentClientApp.h | 2 +- .../src/AssignmentClientMonitor.cpp | 16 +++----- libraries/networking/src/LimitedNodeList.h | 4 +- .../networking/src/ThreadedAssignment.cpp | 4 +- 7 files changed, 42 insertions(+), 46 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index fe845fca4a..69b9934feb 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -40,11 +40,11 @@ const long long ASSIGNMENT_REQUEST_INTERVAL_MSECS = 1 * 1000; int hifiSockAddrMeta = qRegisterMetaType("HifiSockAddr"); -AssignmentClient::AssignmentClient(int ppid, Assignment::Type requestAssignmentType, QString assignmentPool, - QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort) : +AssignmentClient::AssignmentClient(Assignment::Type requestAssignmentType, QString assignmentPool, + QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort, + quint16 assignmentMonitorPort) : _assignmentServerHostname(DEFAULT_ASSIGNMENT_SERVER_HOSTNAME), - _localASPortSharedMem(NULL), - _localACMPortSharedMem(NULL) + _localASPortSharedMem(NULL) { LogUtils::init(); @@ -107,9 +107,16 @@ AssignmentClient::AssignmentClient(int ppid, Assignment::Type requestAssignmentT // Create Singleton objects on main thread NetworkAccessManager::getInstance(); - - // Hook up a timer to send this child's status to the Monitor once per second - setUpStatsToMonitor(ppid); + + // did we get an assignment-client monitor port? + if (assignmentMonitorPort > 0) { + _assignmentClientMonitorSocket = HifiSockAddr(DEFAULT_ASSIGNMENT_CLIENT_MONITOR_HOSTNAME, assignmentMonitorPort); + + qDebug() << "Assignment-client monitor socket is" << _assignmentClientMonitorSocket; + + // Hook up a timer to send this child's status to the Monitor once per second + setUpStatsToMonitor(); + } } @@ -144,15 +151,7 @@ void AssignmentClient::aboutToQuit() { } -void AssignmentClient::setUpStatsToMonitor(int ppid) { - // Figure out the address to send out stats to - quint16 localMonitorServerPort = DEFAULT_ASSIGNMENT_CLIENT_MONITOR_PORT; - auto nodeList = DependencyManager::get(); - - nodeList->getLocalServerPortFromSharedMemory(QString(ASSIGNMENT_CLIENT_MONITOR_LOCAL_PORT_SMEM_KEY) + "-" + - QString::number(ppid), _localACMPortSharedMem, localMonitorServerPort); - _assignmentClientMonitorSocket = HifiSockAddr(DEFAULT_ASSIGNMENT_CLIENT_MONITOR_HOSTNAME, localMonitorServerPort, true); - +void AssignmentClient::setUpStatsToMonitor() { // send a stats packet every 1 seconds connect(&_statsTimerACM, &QTimer::timeout, this, &AssignmentClient::sendStatsPacketToACM); _statsTimerACM.start(1000); @@ -208,6 +207,9 @@ void AssignmentClient::readPendingDatagrams() { if (nodeList->packetVersionAndHashMatch(receivedPacket)) { if (packetTypeForPacket(receivedPacket) == PacketTypeCreateAssignment) { + + qDebug() << "Received a PacketTypeCreateAssignment - attempting to unpack."; + // construct the deployed assignment from the packet data _currentAssignment = AssignmentFactory::unpackAssignment(receivedPacket); @@ -258,8 +260,8 @@ void AssignmentClient::readPendingDatagrams() { } else if (packetTypeForPacket(receivedPacket) == PacketTypeStopNode) { if (senderSockAddr.getAddress() == QHostAddress::LocalHost || senderSockAddr.getAddress() == QHostAddress::LocalHostIPv6) { - qDebug() << "Network told me to exit."; - emit stopAssignmentClient(); + qDebug() << "AssignmentClientMonitor at" << senderSockAddr << "requested stop via PacketTypeStopNode."; + stopAssignmentClient(); } else { qDebug() << "Got a stop packet from other than localhost."; } diff --git a/assignment-client/src/AssignmentClient.h b/assignment-client/src/AssignmentClient.h index 005652b7ef..465a497f95 100644 --- a/assignment-client/src/AssignmentClient.h +++ b/assignment-client/src/AssignmentClient.h @@ -23,8 +23,9 @@ class AssignmentClient : public QObject { Q_OBJECT public: - AssignmentClient(int ppid, Assignment::Type requestAssignmentType, QString assignmentPool, - QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort); + AssignmentClient(Assignment::Type requestAssignmentType, QString assignmentPool, + QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort, + quint16 assignmentMonitorPort); private slots: void sendAssignmentRequest(); @@ -38,13 +39,13 @@ public slots: void aboutToQuit(); private: - void setUpStatsToMonitor(int ppid); + void setUpStatsToMonitor(); + Assignment _requestAssignment; QPointer _currentAssignment; QString _assignmentServerHostname; HifiSockAddr _assignmentServerSocket; QSharedMemory* _localASPortSharedMem; // memory shared with domain server - QSharedMemory* _localACMPortSharedMem; // memory shared with assignment client monitor QTimer _requestTimer; // timer for requesting and assignment QTimer _statsTimerACM; // timer for sending stats to assignment client monitor diff --git a/assignment-client/src/AssignmentClientApp.cpp b/assignment-client/src/AssignmentClientApp.cpp index 2de349ca4e..cd6e49ed09 100644 --- a/assignment-client/src/AssignmentClientApp.cpp +++ b/assignment-client/src/AssignmentClientApp.cpp @@ -79,9 +79,8 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : const QCommandLineOption maxChildsOption(ASSIGNMENT_MAX_FORKS_OPTION, "maximum number of children", "child-count"); parser.addOption(maxChildsOption); - const QCommandLineOption ppidOption(PARENT_PID_OPTION, "parent's process id", "pid"); - parser.addOption(ppidOption); - + const QCommandLineOption monitorPortOption(ASSIGNMENT_CLIENT_MONITOR_PORT_OPTION, "assignment-client monitor port", "port"); + parser.addOption(monitorPortOption); if (!parser.parse(QCoreApplication::arguments())) { qCritical() << parser.errorText() << endl; @@ -113,9 +112,9 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : maxForks = parser.value(maxChildsOption).toInt(); } - int ppid = 0; - if (parser.isSet(ppidOption)) { - ppid = parser.value(ppidOption).toInt(); + unsigned short monitorPort = 0; + if (parser.isSet(monitorPortOption)) { + monitorPort = parser.value(monitorPortOption).toUShort(); } if (!numForks && minForks) { @@ -191,8 +190,8 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : connect(this, &QCoreApplication::aboutToQuit, &monitor, &AssignmentClientMonitor::aboutToQuit); exec(); } else { - AssignmentClient client(ppid, requestAssignmentType, assignmentPool, - walletUUID, assignmentServerHostname, assignmentServerPort); + AssignmentClient client(requestAssignmentType, assignmentPool, + walletUUID, assignmentServerHostname, assignmentServerPort, monitorPort); connect(this, &QCoreApplication::aboutToQuit, &client, &AssignmentClient::aboutToQuit); exec(); } diff --git a/assignment-client/src/AssignmentClientApp.h b/assignment-client/src/AssignmentClientApp.h index 7f75f63755..bbc60256a7 100644 --- a/assignment-client/src/AssignmentClientApp.h +++ b/assignment-client/src/AssignmentClientApp.h @@ -23,7 +23,7 @@ const QString CUSTOM_ASSIGNMENT_SERVER_PORT_OPTION = "p"; const QString ASSIGNMENT_NUM_FORKS_OPTION = "n"; const QString ASSIGNMENT_MIN_FORKS_OPTION = "min"; const QString ASSIGNMENT_MAX_FORKS_OPTION = "max"; -const QString PARENT_PID_OPTION = "ppid"; +const QString ASSIGNMENT_CLIENT_MONITOR_PORT_OPTION = "monitor-port"; class AssignmentClientApp : public QCoreApplication { diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index bb1e56e886..9e6c3ad568 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -48,15 +48,10 @@ AssignmentClientMonitor::AssignmentClientMonitor(const unsigned int numAssignmen // create a NodeList so we can receive stats from children DependencyManager::registerInheritance(); auto addressManager = DependencyManager::set(); - auto nodeList = DependencyManager::set(DEFAULT_ASSIGNMENT_CLIENT_MONITOR_PORT); + auto nodeList = DependencyManager::set(); connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, this, &AssignmentClientMonitor::readPendingDatagrams); - qint64 pid = QCoreApplication::applicationPid (); - - nodeList->putLocalPortIntoSharedMemory(QString(ASSIGNMENT_CLIENT_MONITOR_LOCAL_PORT_SMEM_KEY) + "-" + QString::number(pid), - this, nodeList->getNodeSocket().localPort()); - // use QProcess to fork off a process for each of the child assignment clients for (unsigned int i = 0; i < _numAssignmentClientForks; i++) { spawnChildClient(); @@ -139,10 +134,10 @@ void AssignmentClientMonitor::spawnChildClient() { _childArguments.append(QString::number(_requestAssignmentType)); } - // tell children which shared memory key to use - qint64 pid = QCoreApplication::applicationPid (); - _childArguments.append("--" + PARENT_PID_OPTION); - _childArguments.append(QString::number(pid)); + // tell children which assignment monitor port to use + // for now they simply talk to us on localhost + _childArguments.append("--" + ASSIGNMENT_CLIENT_MONITOR_PORT_OPTION); + _childArguments.append(QString::number(DependencyManager::get()->getLocalSockAddr().getPort())); // make sure that the output from the child process appears in our output assignmentClient->setProcessChannelMode(QProcess::ForwardedChannels); @@ -183,6 +178,7 @@ void AssignmentClientMonitor::checkSpares() { qDebug() << "asking child" << aSpareId << "to exit."; SharedNodePointer childNode = nodeList->nodeWithUUID(aSpareId); childNode->activateLocalSocket(); + QByteArray diePacket = byteArrayWithPopulatedHeader(PacketTypeStopNode); nodeList->writeUnverifiedDatagram(diePacket, childNode); } diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 6ebe30c930..424921d3b9 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -50,10 +50,8 @@ const unsigned short STUN_SERVER_PORT = 3478; const QString DOMAIN_SERVER_LOCAL_PORT_SMEM_KEY = "domain-server.local-port"; const QString DOMAIN_SERVER_LOCAL_HTTP_PORT_SMEM_KEY = "domain-server.local-http-port"; const QString DOMAIN_SERVER_LOCAL_HTTPS_PORT_SMEM_KEY = "domain-server.local-https-port"; -const QString ASSIGNMENT_CLIENT_MONITOR_LOCAL_PORT_SMEM_KEY = "assignment-client-monitor.local-port"; -const char DEFAULT_ASSIGNMENT_CLIENT_MONITOR_HOSTNAME[] = "localhost"; -const unsigned short DEFAULT_ASSIGNMENT_CLIENT_MONITOR_PORT = 40104; +const QHostAddress DEFAULT_ASSIGNMENT_CLIENT_MONITOR_HOSTNAME = QHostAddress::LocalHost; const QString USERNAME_UUID_REPLACEMENT_STATS_KEY = "$username"; diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index f2b4018495..992ccf5575 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -46,10 +46,10 @@ void ThreadedAssignment::setFinished(bool isFinished) { // this ensures we won't process a domain list while we are going down auto nodeList = DependencyManager::get(); disconnect(&nodeList->getNodeSocket(), 0, this, 0); - + // call our virtual aboutToFinish method - this gives the ThreadedAssignment subclass a chance to cleanup aboutToFinish(); - + // if we have a datagram processing thread, quit it and wait on it to make sure that // the node socket is back on the same thread as the NodeList From 0a295bada135834c321711414859f6426194dade Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 14:32:16 -0700 Subject: [PATCH 14/39] add debugging to catch AC kill hang --- assignment-client/src/Agent.cpp | 2 +- assignment-client/src/Agent.h | 1 + assignment-client/src/AssignmentClient.cpp | 3 +++ libraries/networking/src/ThreadedAssignment.h | 2 +- libraries/shared/src/LogHandler.cpp | 2 +- 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index ffeb5a79e0..ec77d745f8 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -224,7 +224,7 @@ void Agent::run() { void Agent::aboutToFinish() { _scriptEngine.stop(); - + // our entity tree is going to go away so tell that to the EntityScriptingInterface DependencyManager::get()->setEntityTree(NULL); diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index 415b14c0da..289aa35d8a 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -36,6 +36,7 @@ class Agent : public ThreadedAssignment { Q_PROPERTY(float lastReceivedAudioLoudness READ getLastReceivedAudioLoudness) public: Agent(const QByteArray& packet); + ~Agent() { qDebug() << "Agent is dead"; } void setIsAvatar(bool isAvatar) { QMetaObject::invokeMethod(&_scriptEngine, "setIsAvatar", Q_ARG(bool, isAvatar)); } bool isAvatar() const { return _scriptEngine.isAvatar(); } diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 69b9934feb..f307f3a4bd 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -136,6 +136,9 @@ void AssignmentClient::stopAssignmentClient() { // ask the current assignment to delete itself on its thread _currentAssignment->deleteLater(); + // when this thread is destroyed we don't need to run our assignment complete method + disconnect(currentAssignmentThread, &QThread::destroyed, this, &AssignmentClient::assignmentCompleted); + // wait on the thread from that assignment - it will be gone once the current assignment deletes currentAssignmentThread->quit(); currentAssignmentThread->wait(); diff --git a/libraries/networking/src/ThreadedAssignment.h b/libraries/networking/src/ThreadedAssignment.h index 4e425be429..d20c44c7a0 100644 --- a/libraries/networking/src/ThreadedAssignment.h +++ b/libraries/networking/src/ThreadedAssignment.h @@ -20,7 +20,7 @@ class ThreadedAssignment : public Assignment { Q_OBJECT public: ThreadedAssignment(const QByteArray& packet); - ~ThreadedAssignment() { stop(); } + ~ThreadedAssignment() { stop(); qDebug() << "ThreadedAssignment dtor called"; } void setFinished(bool isFinished); virtual void aboutToFinish() { }; diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index 60cddb5cfe..64ded81a52 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -24,7 +24,7 @@ LogHandler& LogHandler::getInstance() { } LogHandler::LogHandler() : - _shouldOutputPID(false) + _shouldOutputPID(true) { // setup our timer to flush the verbose logs every 5 seconds QTimer* logFlushTimer = new QTimer(this); From aca71474d8d7e85a8a641587174da0c21bd05bdb Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 14:34:34 -0700 Subject: [PATCH 15/39] add debugging to AC monitor to capture hang --- assignment-client/src/AssignmentClient.cpp | 2 +- assignment-client/src/AssignmentClient.h | 2 +- .../src/AssignmentClientMonitor.cpp | 19 ++++++++++++++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index f307f3a4bd..cd5b1bb20b 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -150,7 +150,7 @@ void AssignmentClient::aboutToQuit() { stopAssignmentClient(); // clear the log handler so that Qt doesn't call the destructor on LogHandler - qInstallMessageHandler(0); + qInstallMessageHandler(0); } diff --git a/assignment-client/src/AssignmentClient.h b/assignment-client/src/AssignmentClient.h index 465a497f95..ede1a9bbf6 100644 --- a/assignment-client/src/AssignmentClient.h +++ b/assignment-client/src/AssignmentClient.h @@ -26,7 +26,7 @@ public: AssignmentClient(Assignment::Type requestAssignmentType, QString assignmentPool, QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort, quint16 assignmentMonitorPort); - + ~AssignmentClient() { qDebug() << "AC object is going down"; } private slots: void sendAssignmentRequest(); void readPendingDatagrams(); diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 9e6c3ad568..f204157363 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -70,9 +70,18 @@ void AssignmentClientMonitor::waitOnChildren(int msecs) { QMutableListIterator i(_childProcesses); while (i.hasNext()) { QProcess* childProcess = i.next(); - bool finished = childProcess->waitForFinished(msecs); - if (finished) { + + qDebug() << "The current state of the process is" << childProcess->state(); + + if (childProcess->state() == QProcess::NotRunning) { i.remove(); + } else { + qDebug() << "Waiting on" << childProcess->processId() << "for" << msecs; + + bool finished = childProcess->waitForFinished(msecs); + if (finished) { + i.remove(); + } } } } @@ -84,19 +93,23 @@ void AssignmentClientMonitor::stopChildProcesses() { QMutableListIterator i(_childProcesses); while (i.hasNext()) { QProcess* childProcess = i.next(); + qDebug() << "Asking" << childProcess->processId() << "to terminate"; childProcess->terminate(); } // try to give all the children time to shutdown + qDebug() << "Waiting on children after terminate"; waitOnChildren(WAIT_FOR_CHILD_MSECS); // ask even more firmly QMutableListIterator j(_childProcesses); while (j.hasNext()) { QProcess* childProcess = j.next(); + qDebug() << "Asking" << childProcess->processId() << "to die"; childProcess->kill(); } - + + qDebug() << "Waiting on children after kill"; waitOnChildren(WAIT_FOR_CHILD_MSECS); } From 692c3497cd149c66e12be6e7a419a9b31121c163 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 14:45:46 -0700 Subject: [PATCH 16/39] return the actual exit code from the application --- assignment-client/src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/main.cpp b/assignment-client/src/main.cpp index 81a12526bf..e28e596ae4 100644 --- a/assignment-client/src/main.cpp +++ b/assignment-client/src/main.cpp @@ -14,5 +14,5 @@ int main(int argc, char* argv[]) { AssignmentClientApp app(argc, argv); - return 0; + return app.exec(); } From 8307553d1b75a6f4c5c8afb6851771f002fd295b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 14:53:30 -0700 Subject: [PATCH 17/39] don't call exec from ACApp ctor --- assignment-client/src/AssignmentClientApp.cpp | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/assignment-client/src/AssignmentClientApp.cpp b/assignment-client/src/AssignmentClientApp.cpp index cd6e49ed09..796e8ed3b0 100644 --- a/assignment-client/src/AssignmentClientApp.cpp +++ b/assignment-client/src/AssignmentClientApp.cpp @@ -161,12 +161,11 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : if (argumentVariantMap.contains(ASSIGNMENT_WALLET_DESTINATION_ID_OPTION)) { assignmentServerPort = argumentVariantMap.value(CUSTOM_ASSIGNMENT_SERVER_PORT_OPTION).toString().toUInt(); } + if (parser.isSet(assignmentServerPortOption)) { assignmentServerPort = parser.value(assignmentServerPortOption).toInt(); } - - if (parser.isSet(numChildsOption)) { if (minForks && minForks > numForks) { qCritical() << "--min can't be more than -n"; @@ -185,14 +184,17 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : DependencyManager::registerInheritance(); if (numForks || minForks || maxForks) { - AssignmentClientMonitor monitor(numForks, minForks, maxForks, requestAssignmentType, assignmentPool, - walletUUID, assignmentServerHostname, assignmentServerPort); - connect(this, &QCoreApplication::aboutToQuit, &monitor, &AssignmentClientMonitor::aboutToQuit); - exec(); + AssignmentClientMonitor* monitor = new AssignmentClientMonitor(numForks, minForks, maxForks, + requestAssignmentType, assignmentPool, + walletUUID, assignmentServerHostname, + assignmentServerPort); + monitor->setParent(this); + connect(this, &QCoreApplication::aboutToQuit, monitor, &AssignmentClientMonitor::aboutToQuit); } else { - AssignmentClient client(requestAssignmentType, assignmentPool, - walletUUID, assignmentServerHostname, assignmentServerPort, monitorPort); - connect(this, &QCoreApplication::aboutToQuit, &client, &AssignmentClient::aboutToQuit); - exec(); + AssignmentClient* client = new AssignmentClient(requestAssignmentType, assignmentPool, + walletUUID, assignmentServerHostname, + assignmentServerPort, monitorPort); + client->setParent(this); + connect(this, &QCoreApplication::aboutToQuit, client, &AssignmentClient::aboutToQuit); } } From 897ed84cca8bc9913325ad807fb7f3cfbf4f0fc8 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 14:56:49 -0700 Subject: [PATCH 18/39] remove debug calls used to find ACApp stall --- assignment-client/src/Agent.h | 1 - assignment-client/src/AssignmentClient.h | 1 - assignment-client/src/AssignmentClientMonitor.cpp | 8 -------- libraries/networking/src/ThreadedAssignment.h | 2 +- 4 files changed, 1 insertion(+), 11 deletions(-) diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index 289aa35d8a..415b14c0da 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -36,7 +36,6 @@ class Agent : public ThreadedAssignment { Q_PROPERTY(float lastReceivedAudioLoudness READ getLastReceivedAudioLoudness) public: Agent(const QByteArray& packet); - ~Agent() { qDebug() << "Agent is dead"; } void setIsAvatar(bool isAvatar) { QMetaObject::invokeMethod(&_scriptEngine, "setIsAvatar", Q_ARG(bool, isAvatar)); } bool isAvatar() const { return _scriptEngine.isAvatar(); } diff --git a/assignment-client/src/AssignmentClient.h b/assignment-client/src/AssignmentClient.h index ede1a9bbf6..acd952122c 100644 --- a/assignment-client/src/AssignmentClient.h +++ b/assignment-client/src/AssignmentClient.h @@ -26,7 +26,6 @@ public: AssignmentClient(Assignment::Type requestAssignmentType, QString assignmentPool, QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort, quint16 assignmentMonitorPort); - ~AssignmentClient() { qDebug() << "AC object is going down"; } private slots: void sendAssignmentRequest(); void readPendingDatagrams(); diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index f204157363..8ed40e4311 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -71,13 +71,9 @@ void AssignmentClientMonitor::waitOnChildren(int msecs) { while (i.hasNext()) { QProcess* childProcess = i.next(); - qDebug() << "The current state of the process is" << childProcess->state(); - if (childProcess->state() == QProcess::NotRunning) { i.remove(); } else { - qDebug() << "Waiting on" << childProcess->processId() << "for" << msecs; - bool finished = childProcess->waitForFinished(msecs); if (finished) { i.remove(); @@ -93,23 +89,19 @@ void AssignmentClientMonitor::stopChildProcesses() { QMutableListIterator i(_childProcesses); while (i.hasNext()) { QProcess* childProcess = i.next(); - qDebug() << "Asking" << childProcess->processId() << "to terminate"; childProcess->terminate(); } // try to give all the children time to shutdown - qDebug() << "Waiting on children after terminate"; waitOnChildren(WAIT_FOR_CHILD_MSECS); // ask even more firmly QMutableListIterator j(_childProcesses); while (j.hasNext()) { QProcess* childProcess = j.next(); - qDebug() << "Asking" << childProcess->processId() << "to die"; childProcess->kill(); } - qDebug() << "Waiting on children after kill"; waitOnChildren(WAIT_FOR_CHILD_MSECS); } diff --git a/libraries/networking/src/ThreadedAssignment.h b/libraries/networking/src/ThreadedAssignment.h index d20c44c7a0..c4df616aaa 100644 --- a/libraries/networking/src/ThreadedAssignment.h +++ b/libraries/networking/src/ThreadedAssignment.h @@ -20,7 +20,7 @@ class ThreadedAssignment : public Assignment { Q_OBJECT public: ThreadedAssignment(const QByteArray& packet); - ~ThreadedAssignment() { stop(); qDebug() << "ThreadedAssignment dtor called"; } + ~ThreadedAssignment() { stop(); } void setFinished(bool isFinished); virtual void aboutToFinish() { }; From 78a1bcdcaefb1a8c689d8bc68235bc108d17fb7f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:01:00 -0700 Subject: [PATCH 19/39] add helpful debug for AC monitor cleanup --- assignment-client/src/AssignmentClientMonitor.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 8ed40e4311..b6bf255bce 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -74,6 +74,7 @@ void AssignmentClientMonitor::waitOnChildren(int msecs) { if (childProcess->state() == QProcess::NotRunning) { i.remove(); } else { + qDebug() << "Waiting on child process" << childProcess->processId() << "to finish."; bool finished = childProcess->waitForFinished(msecs); if (finished) { i.remove(); @@ -89,6 +90,7 @@ void AssignmentClientMonitor::stopChildProcesses() { QMutableListIterator i(_childProcesses); while (i.hasNext()) { QProcess* childProcess = i.next(); + qDebug() << "Attempting to terminate child process" << childProcess->processId(); childProcess->terminate(); } @@ -99,6 +101,7 @@ void AssignmentClientMonitor::stopChildProcesses() { QMutableListIterator j(_childProcesses); while (j.hasNext()) { QProcess* childProcess = j.next(); + qDebug() << "Attempting to kill child process" << childProcess->processId(); childProcess->kill(); } From e092b8c537d7c7c97d3e27a4fe5dbade2155fe6c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:04:54 -0700 Subject: [PATCH 20/39] give each child one second to clean up --- assignment-client/src/AssignmentClientMonitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index b6bf255bce..b3817f646b 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -23,7 +23,7 @@ const QString ASSIGNMENT_CLIENT_MONITOR_TARGET_NAME = "assignment-client-monitor"; -const int WAIT_FOR_CHILD_MSECS = 500; +const int WAIT_FOR_CHILD_MSECS = 1000; AssignmentClientMonitor::AssignmentClientMonitor(const unsigned int numAssignmentClientForks, const unsigned int minAssignmentClientForks, From 15fdcb4fe0bb2b65dd9001c23a1c06eebeceb859 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:07:59 -0700 Subject: [PATCH 21/39] fix non-block, add exit code debug --- assignment-client/src/AssignmentClientMonitor.cpp | 2 +- assignment-client/src/main.cpp | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index b3817f646b..a4015264ca 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -73,7 +73,7 @@ void AssignmentClientMonitor::waitOnChildren(int msecs) { if (childProcess->state() == QProcess::NotRunning) { i.remove(); - } else { + } else if (msecs > 0) { qDebug() << "Waiting on child process" << childProcess->processId() << "to finish."; bool finished = childProcess->waitForFinished(msecs); if (finished) { diff --git a/assignment-client/src/main.cpp b/assignment-client/src/main.cpp index e28e596ae4..1c4426fc4d 100644 --- a/assignment-client/src/main.cpp +++ b/assignment-client/src/main.cpp @@ -9,10 +9,12 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // - #include "AssignmentClientApp.h" +#include + int main(int argc, char* argv[]) { AssignmentClientApp app(argc, argv); - return app.exec(); + int acReturn = app.exec(); + qDebug() << "assignment-client process" << app.applicationPid() << "exiting with status code" << acReturn; } From fe2775ca948b839e70cbb26927327e092e7a5adb Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:10:37 -0700 Subject: [PATCH 22/39] print out state before wait --- assignment-client/src/AssignmentClientMonitor.cpp | 2 ++ assignment-client/src/main.cpp | 3 +++ 2 files changed, 5 insertions(+) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index a4015264ca..25b54572f1 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -71,6 +71,8 @@ void AssignmentClientMonitor::waitOnChildren(int msecs) { while (i.hasNext()) { QProcess* childProcess = i.next(); + qDebug() << "The current state of process" << childProcess->processId() << "is" << childProcess->state(); + if (childProcess->state() == QProcess::NotRunning) { i.remove(); } else if (msecs > 0) { diff --git a/assignment-client/src/main.cpp b/assignment-client/src/main.cpp index 1c4426fc4d..5d08848b8b 100644 --- a/assignment-client/src/main.cpp +++ b/assignment-client/src/main.cpp @@ -15,6 +15,9 @@ int main(int argc, char* argv[]) { AssignmentClientApp app(argc, argv); + int acReturn = app.exec(); qDebug() << "assignment-client process" << app.applicationPid() << "exiting with status code" << acReturn; + + return acReturn; } From 4c93e395adc7e7579ddfb9476f3718ffa6fd126e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:22:58 -0700 Subject: [PATCH 23/39] ask QCoreApplication to quit when PacketTypeStopNode received --- assignment-client/src/AssignmentClient.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index cd5b1bb20b..8f02c6c36d 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -264,7 +264,8 @@ void AssignmentClient::readPendingDatagrams() { if (senderSockAddr.getAddress() == QHostAddress::LocalHost || senderSockAddr.getAddress() == QHostAddress::LocalHostIPv6) { qDebug() << "AssignmentClientMonitor at" << senderSockAddr << "requested stop via PacketTypeStopNode."; - stopAssignmentClient(); + + QCoreApplication::quit(); } else { qDebug() << "Got a stop packet from other than localhost."; } From 07d8a6a8ae17c5e445c814d824c7fabd82b09ebd Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:23:22 -0700 Subject: [PATCH 24/39] remove debug for current state of ACM child process --- assignment-client/src/AssignmentClientMonitor.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 25b54572f1..a4015264ca 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -71,8 +71,6 @@ void AssignmentClientMonitor::waitOnChildren(int msecs) { while (i.hasNext()) { QProcess* childProcess = i.next(); - qDebug() << "The current state of process" << childProcess->processId() << "is" << childProcess->state(); - if (childProcess->state() == QProcess::NotRunning) { i.remove(); } else if (msecs > 0) { From b5d0a31ad44705621a353c03f1dddcd647bdc54b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:25:49 -0700 Subject: [PATCH 25/39] remove clear access cache on Agent stop --- assignment-client/src/Agent.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index ec77d745f8..3c99f0a209 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -227,6 +227,4 @@ void Agent::aboutToFinish() { // our entity tree is going to go away so tell that to the EntityScriptingInterface DependencyManager::get()->setEntityTree(NULL); - - NetworkAccessManager::getInstance().clearAccessCache(); } From 8359a1a492d5b3d77bab012fbd695482277f324f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:30:11 -0700 Subject: [PATCH 26/39] add debug for completion of Agent script-engine stop --- libraries/script-engine/src/ScriptEngine.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index ac2c212001..49ea585f83 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -633,6 +633,8 @@ void ScriptEngine::run() { emit doneRunning(); _doneRunningThisScript = true; + + qDebug() << "Agent script-engine stop completed"; } // NOTE: This is private because it must be called on the same thread that created the timers, which is why From 637600697b520c609986c5c352c6e9042e88a541 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:35:48 -0700 Subject: [PATCH 27/39] remove ScriptEngine stop completion debug --- libraries/script-engine/src/ScriptEngine.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 49ea585f83..ac2c212001 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -633,8 +633,6 @@ void ScriptEngine::run() { emit doneRunning(); _doneRunningThisScript = true; - - qDebug() << "Agent script-engine stop completed"; } // NOTE: This is private because it must be called on the same thread that created the timers, which is why From 78942b0a87b4adba512bb40be2329a91039fb75f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 15:49:28 -0700 Subject: [PATCH 28/39] let QSettings destructor call Setting::Manager sync --- libraries/shared/src/SettingManager.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/shared/src/SettingManager.cpp b/libraries/shared/src/SettingManager.cpp index 8bb6c89c82..0e05278db6 100644 --- a/libraries/shared/src/SettingManager.cpp +++ b/libraries/shared/src/SettingManager.cpp @@ -18,11 +18,12 @@ namespace Setting { Manager::~Manager() { // Cleanup timer stopTimer(); - disconnect(_saveTimer, 0, 0, 0); - + delete _saveTimer; + // Save all settings before exit saveAll(); - sync(); + + // sync will be called in the QSettings destructor } void Manager::registerHandle(Setting::Interface* handle) { From ba2ccc1e98614cd8e32d1c09fd7e94284f59300c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 16:23:44 -0700 Subject: [PATCH 29/39] change waitOnChildren syntax in AC monitor --- .../src/AssignmentClientMonitor.cpp | 16 ++++++++++++---- assignment-client/src/AssignmentClientMonitor.h | 3 ++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index a4015264ca..f5e98b51d8 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -66,16 +66,16 @@ AssignmentClientMonitor::~AssignmentClientMonitor() { stopChildProcesses(); } -void AssignmentClientMonitor::waitOnChildren(int msecs) { +void AssignmentClientMonitor::waitOnChildren(int waitMsecs) { QMutableListIterator i(_childProcesses); while (i.hasNext()) { QProcess* childProcess = i.next(); if (childProcess->state() == QProcess::NotRunning) { i.remove(); - } else if (msecs > 0) { + } else if (waitMsecs > 0) { qDebug() << "Waiting on child process" << childProcess->processId() << "to finish."; - bool finished = childProcess->waitForFinished(msecs); + bool finished = childProcess->waitForFinished(waitMsecs); if (finished) { i.remove(); } @@ -191,8 +191,16 @@ void AssignmentClientMonitor::checkSpares() { nodeList->writeUnverifiedDatagram(diePacket, childNode); } } + + // check if any of the previous processes have now gone down + QMutableListIterator i(_childProcesses); + while (i.hasNext()) { + QProcess* childProcess = i.next(); - waitOnChildren(0); + if (childProcess->state() == QProcess::NotRunning) { + i.remove(); + } + } } diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index e0129bd9b9..89cf6759cf 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -33,7 +33,6 @@ public: quint16 assignmentServerPort); ~AssignmentClientMonitor(); - void waitOnChildren(int msecs); void stopChildProcesses(); private slots: void readPendingDatagrams(); @@ -44,6 +43,8 @@ public slots: private: void spawnChildClient(); + void waitOnChildren(int waitMsecs); + QTimer _checkSparesTimer; // every few seconds see if it need fewer or more spare children const unsigned int _numAssignmentClientForks; From 83cd50c7ae62a1664d2c78156639c3bed84682ec Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 16:24:20 -0700 Subject: [PATCH 30/39] use waitOnChildren again from checkSpares --- assignment-client/src/AssignmentClientMonitor.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index f5e98b51d8..499110c1f5 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -193,14 +193,7 @@ void AssignmentClientMonitor::checkSpares() { } // check if any of the previous processes have now gone down - QMutableListIterator i(_childProcesses); - while (i.hasNext()) { - QProcess* childProcess = i.next(); - - if (childProcess->state() == QProcess::NotRunning) { - i.remove(); - } - } + waitOnChildren(0); } From ddb52348bc4c4eb954af02ae9769cc67aca4edff Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 16:24:39 -0700 Subject: [PATCH 31/39] don't force use of Settings to all QCoreApplication --- domain-server/src/DomainServer.cpp | 1 + interface/src/Application.cpp | 5 ++++- interface/src/main.cpp | 1 - libraries/shared/src/SettingInterface.cpp | 11 ++++------- libraries/shared/src/SettingInterface.h | 7 +++++-- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 6ee1d6c765..01998301b2 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -69,6 +69,7 @@ DomainServer::DomainServer(int argc, char* argv[]) : _iceServerSocket(ICE_SERVER_DEFAULT_HOSTNAME, ICE_SERVER_DEFAULT_PORT) { LogUtils::init(); + Setting::init(); setOrganizationName("High Fidelity"); setOrganizationDomain("highfidelity.io"); diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 8d660848f2..38bce82349 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -335,10 +336,12 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _notifiedPacketVersionMismatchThisDomain(false), _domainConnectionRefusals(QList()) { + + Setting::init(); + #ifdef Q_OS_WIN installNativeEventFilter(&MyNativeEventFilter::getInstance()); #endif - _logger = new FileLogger(this); // After setting organization name in order to get correct directory diff --git a/interface/src/main.cpp b/interface/src/main.cpp index ae1b93959f..13ca17355b 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -37,7 +37,6 @@ static BOOL CALLBACK enumWindowsCallback(HWND hWnd, LPARAM lParam) { } #endif - int main(int argc, const char* argv[]) { #ifdef Q_OS_WIN // Run only one instance of Interface at a time. diff --git a/libraries/shared/src/SettingInterface.cpp b/libraries/shared/src/SettingInterface.cpp index e263d83ef6..eead317390 100644 --- a/libraries/shared/src/SettingInterface.cpp +++ b/libraries/shared/src/SettingInterface.cpp @@ -36,7 +36,7 @@ namespace Setting { } // Sets up the settings private instance. Should only be run once at startup - void setupPrivateInstance() { + void init() { // read the ApplicationInfo.ini file for Name/Version/Domain information QSettings::setDefaultFormat(QSettings::IniFormat); QSettings applicationInfo(PathUtils::resourcesPath() + "info/ApplicationInfo.ini", QSettings::IniFormat); @@ -59,14 +59,11 @@ namespace Setting { QObject::connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater())); privateInstance->moveToThread(thread); thread->start(); - qCDebug(shared) << "Settings thread started."; - + qCDebug(shared) << "Settings thread started."; + // Register cleanupPrivateInstance to run inside QCoreApplication's destructor. qAddPostRoutine(cleanupPrivateInstance); - } - // Register setupPrivateInstance to run after QCoreApplication's constructor. - Q_COREAPP_STARTUP_FUNCTION(setupPrivateInstance) - + } Interface::~Interface() { if (privateInstance) { diff --git a/libraries/shared/src/SettingInterface.h b/libraries/shared/src/SettingInterface.h index 43e97d5113..5092fd09c8 100644 --- a/libraries/shared/src/SettingInterface.h +++ b/libraries/shared/src/SettingInterface.h @@ -16,11 +16,14 @@ #include namespace Setting { + void init(); + void cleanupSettings(); + class Interface { public: QString getKey() const { return _key; } - bool isSet() const { return _isSet; } - + bool isSet() const { return _isSet; } + virtual void setVariant(const QVariant& variant) = 0; virtual QVariant getVariant() = 0; From acac2f5a2231b475cb88b8043d9c7cd7ef46a5b5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 16:51:27 -0700 Subject: [PATCH 32/39] don't init if Settings not init --- libraries/shared/src/SettingInterface.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/libraries/shared/src/SettingInterface.cpp b/libraries/shared/src/SettingInterface.cpp index eead317390..26cbe3ca13 100644 --- a/libraries/shared/src/SettingInterface.cpp +++ b/libraries/shared/src/SettingInterface.cpp @@ -73,16 +73,20 @@ namespace Setting { void Interface::init() { if (!privateInstance) { - qWarning() << "Setting::Interface::init(): Manager not yet created, bailing"; - return; + // WARNING: As long as we are using QSettings this should always be triggered for each Setting::Handle + // in an assignment-client - the QSettings backing we use for this means persistence of these + // settings from an AC (when there can be multiple terminating at same time on one machine) + // is currently not supported + qWarning() << "Setting::Interface::init() for key" << _key << "- Manager not yet created." << + "Settings persistence disabled."; + } else { + // Register Handle + privateInstance->registerHandle(this); + _isInitialized = true; + + // Load value from disk + load(); } - - // Register Handle - privateInstance->registerHandle(this); - _isInitialized = true; - - // Load value from disk - load(); } void Interface::maybeInit() { From 2916596948bd827bb42cf0455e1569412f80d9eb Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 16:54:35 -0700 Subject: [PATCH 33/39] put back call to clearAccessCache --- assignment-client/src/Agent.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 3c99f0a209..b22c2d1b46 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -227,4 +227,6 @@ void Agent::aboutToFinish() { // our entity tree is going to go away so tell that to the EntityScriptingInterface DependencyManager::get()->setEntityTree(NULL); + + NetworkAccessManager::getInstance().clearAccessCache(); } From 7b91dbfe074ada65e431206dff8124052e6dc069 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 17:24:20 -0700 Subject: [PATCH 34/39] wait on all children simultaneously --- .../src/AssignmentClientMonitor.cpp | 62 +++++++++---------- .../src/AssignmentClientMonitor.h | 5 +- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 499110c1f5..4eae44ddeb 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -66,56 +66,53 @@ AssignmentClientMonitor::~AssignmentClientMonitor() { stopChildProcesses(); } -void AssignmentClientMonitor::waitOnChildren(int waitMsecs) { - QMutableListIterator i(_childProcesses); - while (i.hasNext()) { - QProcess* childProcess = i.next(); +void AssignmentClientMonitor::simultaneousWaitOnChildren(int waitMsecs) { + QElapsedTimer waitTimer; + waitTimer.start(); - if (childProcess->state() == QProcess::NotRunning) { - i.remove(); - } else if (waitMsecs > 0) { - qDebug() << "Waiting on child process" << childProcess->processId() << "to finish."; - bool finished = childProcess->waitForFinished(waitMsecs); - if (finished) { - i.remove(); - } - } - } + // loop as long as we still have processes around and we're inside the wait window + while(_childProcesses.size() > 0 && !waitTimer.hasExpired(waitMsecs)) { + // continue processing events so we can handle a process finishing up + QCoreApplication::processEvents(); + } +} + +void AssignmentClientMonitor::childProcessFinished() { + QProcess* childProcess = qobject_cast(sender()); + _childProcesses.removeOne(childProcess); } void AssignmentClientMonitor::stopChildProcesses() { auto nodeList = DependencyManager::get(); // ask child processes to terminate - QMutableListIterator i(_childProcesses); - while (i.hasNext()) { - QProcess* childProcess = i.next(); + foreach(QProcess* childProcess, _childProcesses) { qDebug() << "Attempting to terminate child process" << childProcess->processId(); childProcess->terminate(); } - - // try to give all the children time to shutdown - waitOnChildren(WAIT_FOR_CHILD_MSECS); - - // ask even more firmly - QMutableListIterator j(_childProcesses); - while (j.hasNext()) { - QProcess* childProcess = j.next(); - qDebug() << "Attempting to kill child process" << childProcess->processId(); - childProcess->kill(); - } - waitOnChildren(WAIT_FOR_CHILD_MSECS); + simultaneousWaitOnChildren(WAIT_FOR_CHILD_MSECS); + + if (_childProcesses.size() > 0) { + // ask even more firmly + foreach(QProcess* childProcess, _childProcesses) { + qDebug() << "Attempting to kill child process" << childProcess->processId(); + childProcess->kill(); + } + + simultaneousWaitOnChildren(WAIT_FOR_CHILD_MSECS); + } } void AssignmentClientMonitor::aboutToQuit() { stopChildProcesses(); + // clear the log handler so that Qt doesn't call the destructor on LogHandler qInstallMessageHandler(0); } void AssignmentClientMonitor::spawnChildClient() { - QProcess *assignmentClient = new QProcess(this); + QProcess* assignmentClient = new QProcess(this); _childProcesses.append(assignmentClient); @@ -152,6 +149,8 @@ void AssignmentClientMonitor::spawnChildClient() { assignmentClient->start(QCoreApplication::applicationFilePath(), _childArguments); + connect(assignmentClient, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(childProcessFinished())); + qDebug() << "Spawned a child client with PID" << assignmentClient->pid(); } @@ -191,9 +190,6 @@ void AssignmentClientMonitor::checkSpares() { nodeList->writeUnverifiedDatagram(diePacket, childNode); } } - - // check if any of the previous processes have now gone down - waitOnChildren(0); } diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index 89cf6759cf..2eb3004bc6 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -37,13 +37,14 @@ public: private slots: void readPendingDatagrams(); void checkSpares(); - + void childProcessFinished(); + public slots: void aboutToQuit(); private: void spawnChildClient(); - void waitOnChildren(int waitMsecs); + void simultaneousWaitOnChildren(int waitMsecs); QTimer _checkSparesTimer; // every few seconds see if it need fewer or more spare children From be03a65dc70d3c29e4078102bca0d7a0581ae06b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 17:30:22 -0700 Subject: [PATCH 35/39] remember the pid of previous child processes --- assignment-client/src/AssignmentClientMonitor.cpp | 12 +++++++++--- assignment-client/src/AssignmentClientMonitor.h | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 4eae44ddeb..dbeb76a884 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -79,7 +79,12 @@ void AssignmentClientMonitor::simultaneousWaitOnChildren(int waitMsecs) { void AssignmentClientMonitor::childProcessFinished() { QProcess* childProcess = qobject_cast(sender()); - _childProcesses.removeOne(childProcess); + qint64 processID = _childProcesses.key(childProcess); + + if (processID > 0) { + qDebug() << "Child process" << processID << "has finished. Removing from internal map."; + _childProcesses.remove(processID); + } } void AssignmentClientMonitor::stopChildProcesses() { @@ -114,8 +119,7 @@ void AssignmentClientMonitor::aboutToQuit() { void AssignmentClientMonitor::spawnChildClient() { QProcess* assignmentClient = new QProcess(this); - _childProcesses.append(assignmentClient); - + // unparse the parts of the command-line that the child cares about QStringList _childArguments; if (_assignmentPool != "") { @@ -149,9 +153,11 @@ void AssignmentClientMonitor::spawnChildClient() { assignmentClient->start(QCoreApplication::applicationFilePath(), _childArguments); + // make sure we hear that this process has finished when it does connect(assignmentClient, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(childProcessFinished())); qDebug() << "Spawned a child client with PID" << assignmentClient->pid(); + _childProcesses.insert(assignmentClient->processId(), assignmentClient); } void AssignmentClientMonitor::checkSpares() { diff --git a/assignment-client/src/AssignmentClientMonitor.h b/assignment-client/src/AssignmentClientMonitor.h index 2eb3004bc6..f05ed3e661 100644 --- a/assignment-client/src/AssignmentClientMonitor.h +++ b/assignment-client/src/AssignmentClientMonitor.h @@ -58,7 +58,7 @@ private: QString _assignmentServerHostname; quint16 _assignmentServerPort; - QList _childProcesses; + QMap _childProcesses; }; #endif // hifi_AssignmentClientMonitor_h From ebeba81233c1fd2ed1716995d3fab61c0440077c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 8 May 2015 17:37:48 -0700 Subject: [PATCH 36/39] make sure an ACM asks for PID in log --- assignment-client/src/AssignmentClientMonitor.cpp | 5 ++++- libraries/shared/src/LogHandler.cpp | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index dbeb76a884..f87679ffce 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -41,10 +41,13 @@ AssignmentClientMonitor::AssignmentClientMonitor(const unsigned int numAssignmen _assignmentServerPort(assignmentServerPort) { qDebug() << "_requestAssignmentType =" << _requestAssignmentType; - + // start the Logging class with the parent's target name LogHandler::getInstance().setTargetName(ASSIGNMENT_CLIENT_MONITOR_TARGET_NAME); + // make sure we output process IDs for a monitor otherwise it's insane to parse + LogHandler::getInstance().setShouldOutputPID(true); + // create a NodeList so we can receive stats from children DependencyManager::registerInheritance(); auto addressManager = DependencyManager::set(); diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index 64ded81a52..60cddb5cfe 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -24,7 +24,7 @@ LogHandler& LogHandler::getInstance() { } LogHandler::LogHandler() : - _shouldOutputPID(true) + _shouldOutputPID(false) { // setup our timer to flush the verbose logs every 5 seconds QTimer* logFlushTimer = new QTimer(this); From 2dc866755d60f4b5d5c76a85bfd7e283111b05f6 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 May 2015 09:56:10 -0700 Subject: [PATCH 37/39] fix OS comments, remove test call from Agent --- assignment-client/src/Agent.cpp | 2 -- assignment-client/src/octree/OctreeServer.cpp | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index b22c2d1b46..3c99f0a209 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -227,6 +227,4 @@ void Agent::aboutToFinish() { // our entity tree is going to go away so tell that to the EntityScriptingInterface DependencyManager::get()->setEntityTree(NULL); - - NetworkAccessManager::getInstance().clearAccessCache(); } diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index d2c6cfc4cd..557e21ca4a 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -1225,8 +1225,8 @@ void OctreeServer::aboutToFinish() { qDebug() << qPrintable(_safeServerName) << "inform Octree Inbound Packet Processor that we are shutting down..."; - // we're going down - set the NodeList linkedDataCallback to NULL so we do not create any more OctreeSendThreads - // this ensures that when we forceNodeShutdown below for each node we don't get any more newly connecting nodes + // we're going down - set the NodeList linkedDataCallback to NULL so we do not create any more OctreeQueryNode objects. + // This ensures that when we forceNodeShutdown below for each node we don't get any more newly connecting nodes auto nodeList = DependencyManager::get(); nodeList->linkedDataCreateCallback = NULL; From 7b903d29e72625b8f83141f4f8bf09ccb5236f5a Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 May 2015 12:58:00 -0700 Subject: [PATCH 38/39] fix missed conflict on merge with upstream/master --- assignment-client/src/AssignmentClientMonitor.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index d57174c32d..d19eb90df6 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -194,12 +194,8 @@ void AssignmentClientMonitor::checkSpares() { qDebug() << "asking child" << aSpareId << "to exit."; SharedNodePointer childNode = nodeList->nodeWithUUID(aSpareId); childNode->activateLocalSocket(); -<<<<<<< HEAD - QByteArray diePacket = byteArrayWithPopulatedHeader(PacketTypeStopNode); -======= QByteArray diePacket = nodeList->byteArrayWithPopulatedHeader(PacketTypeStopNode); ->>>>>>> 5160eb5b94951200390de0de91962cc2aa9efccb nodeList->writeUnverifiedDatagram(diePacket, childNode); } } From 68af209417e55ca7ca747f4c859f798d943b06a8 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 May 2015 16:20:03 -0700 Subject: [PATCH 39/39] call Setting::init from Interface setupEssentials --- interface/src/Application.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index c970a42909..62a7553d9c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -249,6 +249,8 @@ bool setupEssentials(int& argc, char** argv) { DependencyManager::registerInheritance(); DependencyManager::registerInheritance(); + + Setting::init(); // Set dependencies auto addressManager = DependencyManager::set(); @@ -336,9 +338,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _notifiedPacketVersionMismatchThisDomain(false), _domainConnectionRefusals(QList()) { - - Setting::init(); - #ifdef Q_OS_WIN installNativeEventFilter(&MyNativeEventFilter::getInstance()); #endif