From 9f754e40d0234a69bc988eae009bc186a4fcdd3b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 11 Mar 2015 16:59:06 -0700 Subject: [PATCH 1/9] move all silent node timers into LimitedNodeList --- domain-server/src/DomainServer.cpp | 4 ---- interface/src/Application.cpp | 9 --------- libraries/networking/src/LimitedNodeList.cpp | 5 +++++ libraries/networking/src/LimitedNodeList.h | 2 ++ libraries/networking/src/ThreadedAssignment.cpp | 4 ---- 5 files changed, 7 insertions(+), 17 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index d02ad73b47..7af9ffd85c 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -267,10 +267,6 @@ void DomainServer::setupNodeListAndAssignments(const QUuid& sessionUUID) { connect(nodeList.data(), &LimitedNodeList::nodeAdded, this, &DomainServer::nodeAdded); connect(nodeList.data(), &LimitedNodeList::nodeKilled, this, &DomainServer::nodeKilled); - QTimer* silentNodeTimer = new QTimer(this); - connect(silentNodeTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); - silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); - connect(&nodeList->getNodeSocket(), SIGNAL(readyRead()), SLOT(readAvailableDatagrams())); // add whatever static assignments that have been parsed to the queue diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index c53f387518..1e2b06ec27 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -146,7 +146,6 @@ const qint64 MAXIMUM_CACHE_SIZE = 10 * BYTES_PER_GIGABYTES; // 10GB static QTimer* locationUpdateTimer = NULL; static QTimer* balanceUpdateTimer = NULL; -static QTimer* silentNodeTimer = NULL; static QTimer* identityPacketTimer = NULL; static QTimer* billboardPacketTimer = NULL; static QTimer* checkFPStimer = NULL; @@ -426,12 +425,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : // connect to the packet sent signal of the _entityEditSender connect(&_entityEditSender, &EntityEditPacketSender::packetSent, this, &Application::packetSent); - // move the silentNodeTimer to the _nodeThread - silentNodeTimer = new QTimer(); - connect(silentNodeTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); - silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); - silentNodeTimer->moveToThread(_nodeThread); - // send the identity packet for our avatar each second to our avatar mixer identityPacketTimer = new QTimer(); connect(identityPacketTimer, &QTimer::timeout, _myAvatar, &MyAvatar::sendIdentityPacket); @@ -548,7 +541,6 @@ void Application::cleanupBeforeQuit() { // depending on what thread they run in locationUpdateTimer->stop(); balanceUpdateTimer->stop(); - QMetaObject::invokeMethod(silentNodeTimer, "stop", Qt::BlockingQueuedConnection); identityPacketTimer->stop(); billboardPacketTimer->stop(); checkFPStimer->stop(); @@ -558,7 +550,6 @@ void Application::cleanupBeforeQuit() { // and then delete those that got created by "new" delete locationUpdateTimer; delete balanceUpdateTimer; - delete silentNodeTimer; delete identityPacketTimer; delete billboardPacketTimer; delete checkFPStimer; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 279d958082..520dc650ed 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -80,6 +80,10 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short connect(localSocketUpdate, &QTimer::timeout, this, &LimitedNodeList::updateLocalSockAddr); localSocketUpdate->start(LOCAL_SOCKET_UPDATE_INTERVAL_MSECS); + QTimer* silentNodeTimer = new QTimer(this); + connect(silentNodeTimer, &QTimer::timeout, this, &LimitedNodeList::removeSilentNodes); + silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); + // check the local socket right now updateLocalSockAddr(); @@ -500,6 +504,7 @@ void LimitedNodeList::resetPacketStats() { } void LimitedNodeList::removeSilentNodes() { + QSet killedNodes; eachNodeHashIterator([&](NodeHash::iterator& it){ diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index afbdf23fba..a071eced31 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -223,6 +223,8 @@ protected: HifiSockAddr _localSockAddr; HifiSockAddr _publicSockAddr; HifiSockAddr _stunSockAddr; + + QTimer* _silentNodeTimer; // XXX can BandwidthRecorder be used for this? int _numCollectedPackets; diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index ea94a8e22c..79b4e7f437 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -67,10 +67,6 @@ void ThreadedAssignment::commonInit(const QString& targetName, NodeType_t nodeTy connect(domainServerTimer, SIGNAL(timeout()), this, SLOT(checkInWithDomainServerOrExit())); domainServerTimer->start(DOMAIN_SERVER_CHECK_IN_MSECS); - QTimer* silentNodeRemovalTimer = new QTimer(this); - connect(silentNodeRemovalTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); - silentNodeRemovalTimer->start(NODE_SILENCE_THRESHOLD_MSECS); - if (shouldSendStats) { // send a stats packet every 1 second QTimer* statsTimer = new QTimer(this); From 5d9a3811ca2b06d0baf0222875d4f7f3b0f9710f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 11 Mar 2015 17:14:20 -0700 Subject: [PATCH 2/9] more graceful cleanup for Application NodeList and DatagramProcessor --- interface/src/Application.cpp | 31 ++++++++++++++++----------- interface/src/Application.h | 6 ++---- interface/src/DatagramProcessor.cpp | 4 ++++ interface/src/DatagramProcessor.h | 1 + libraries/networking/src/NodeList.cpp | 5 +++++ libraries/networking/src/NodeList.h | 2 ++ 6 files changed, 32 insertions(+), 17 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 1e2b06ec27..c0c7bca214 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -256,7 +256,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _dependencyManagerIsSetup(setupEssentials(argc, argv)), _window(new MainWindow(desktop())), _toolWindow(NULL), - _nodeThread(new QThread(this)), _datagramProcessor(), _undoStack(), _undoStackScriptingInterface(&_undoStack), @@ -327,18 +326,20 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _runningScriptsWidget = new RunningScriptsWidget(_window); // start the nodeThread so its event loop is running - _nodeThread->setObjectName("Datagram Processor Thread"); - _nodeThread->start(); + QThread* nodeThread = new QThread(); + nodeThread->setObjectName("Datagram Processor Thread"); + nodeThread->start(); // make sure the node thread is given highest priority - _nodeThread->setPriority(QThread::TimeCriticalPriority); + nodeThread->setPriority(QThread::TimeCriticalPriority); + + _datagramProcessor = new DatagramProcessor(nodeList.data()); // put the NodeList and datagram processing on the node thread - nodeList->moveToThread(_nodeThread); - _datagramProcessor.moveToThread(_nodeThread); + nodeList->moveToThread(nodeThread); // connect the DataProcessor processDatagrams slot to the QUDPSocket readyRead() signal - connect(&nodeList->getNodeSocket(), SIGNAL(readyRead()), &_datagramProcessor, SLOT(processDatagrams())); + connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, _datagramProcessor, &DatagramProcessor::processDatagrams); // put the audio processing on a separate thread QThread* audioThread = new QThread(); @@ -533,7 +534,7 @@ void Application::aboutToQuit() { } void Application::cleanupBeforeQuit() { - _datagramProcessor.shutdown(); // tell the datagram processor we're shutting down, so it can short circuit + _datagramProcessor->shutdown(); // tell the datagram processor we're shutting down, so it can short circuit _entities.shutdown(); // tell the entities system we're shutting down, so it will stop running scripts ScriptEngine::stopAllScripts(this); // stop all currently running global scripts @@ -581,10 +582,6 @@ Application::~Application() { tree->lockForWrite(); _entities.getTree()->setSimulation(NULL); tree->unlock(); - - // ask the datagram processing thread to quit and wait until it is done - _nodeThread->quit(); - _nodeThread->wait(); _octreeProcessor.terminate(); _entityEditSender.terminate(); @@ -603,6 +600,14 @@ Application::~Application() { DependencyManager::destroy(); //DependencyManager::destroy(); DependencyManager::destroy(); + + auto nodeList = DependencyManager::get(); + QThread* nodeThread = nodeList->thread(); + nodeList->deleteLater(); + + // ask the node thread to quit and wait until it is done + nodeThread->quit(); + nodeThread->wait(); qInstallMessageHandler(NULL); // NOTE: Do this as late as possible so we continue to get our log messages } @@ -1459,7 +1464,7 @@ void Application::checkFPS() { _fps = (float)_frameCount / diffTime; _frameCount = 0; - _datagramProcessor.resetCounters(); + _datagramProcessor->resetCounters(); _timerStart.start(); // ask the node list to check in with the domain server diff --git a/interface/src/Application.h b/interface/src/Application.h index 1da5f430b6..2841a39707 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -445,10 +445,8 @@ private: MainWindow* _window; ToolWindow* _toolWindow; - - - QThread* _nodeThread; - DatagramProcessor _datagramProcessor; + + DatagramProcessor* _datagramProcessor; QUndoStack _undoStack; UndoStackScriptingInterface _undoStackScriptingInterface; diff --git a/interface/src/DatagramProcessor.cpp b/interface/src/DatagramProcessor.cpp index 1e63ce6655..b22eb96030 100644 --- a/interface/src/DatagramProcessor.cpp +++ b/interface/src/DatagramProcessor.cpp @@ -27,6 +27,10 @@ DatagramProcessor::DatagramProcessor(QObject* parent) : } +DatagramProcessor::~DatagramProcessor() { + qDebug() << "DP dtor called from" << QThread::currentThread() << "and the DP thread is" << thread(); +} + void DatagramProcessor::processDatagrams() { PerformanceWarning warn(Menu::getInstance()->isOptionChecked(MenuOption::PipelineWarnings), "DatagramProcessor::processDatagrams()"); diff --git a/interface/src/DatagramProcessor.h b/interface/src/DatagramProcessor.h index 7fc192ee2d..0a6fa3689a 100644 --- a/interface/src/DatagramProcessor.h +++ b/interface/src/DatagramProcessor.h @@ -18,6 +18,7 @@ class DatagramProcessor : public QObject { Q_OBJECT public: DatagramProcessor(QObject* parent = 0); + ~DatagramProcessor(); int getInPacketCount() const { return _inPacketCount; } int getOutPacketCount() const { return _outPacketCount; } diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index e63f230f6e..9ca229a245 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -62,6 +63,10 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned connect(&AccountManager::getInstance(), &AccountManager::logoutComplete , this, &NodeList::reset); } +NodeList::~NodeList() { + qDebug() << "NL dtor called from" << QThread::currentThread() << "and the NL thread is" << thread(); +} + qint64 NodeList::sendStats(const QJsonObject& statsObject, HifiSockAddr destination) { QByteArray statsPacket = byteArrayWithPopulatedHeader(PacketTypeNodeJsonStats); QDataStream statsPacketStream(&statsPacket, QIODevice::Append); diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 1c6de4bb6c..0624ae5810 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -78,6 +78,8 @@ private: NodeList(NodeList const&); // Don't implement, needed to avoid copies of singleton void operator=(NodeList const&); // Don't implement, needed to avoid copies of singleton + ~NodeList(); + void sendSTUNRequest(); bool processSTUNResponse(const QByteArray& packet); From 7a8e94f1e572fc927b4c3ecc6b5453384ddc1572 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 11 Mar 2015 17:18:49 -0700 Subject: [PATCH 3/9] remove extra debug information for node thread cleanup --- interface/src/DatagramProcessor.cpp | 4 ---- interface/src/DatagramProcessor.h | 1 - libraries/networking/src/NodeList.cpp | 4 ---- libraries/networking/src/NodeList.h | 2 -- 4 files changed, 11 deletions(-) diff --git a/interface/src/DatagramProcessor.cpp b/interface/src/DatagramProcessor.cpp index b22eb96030..1e63ce6655 100644 --- a/interface/src/DatagramProcessor.cpp +++ b/interface/src/DatagramProcessor.cpp @@ -27,10 +27,6 @@ DatagramProcessor::DatagramProcessor(QObject* parent) : } -DatagramProcessor::~DatagramProcessor() { - qDebug() << "DP dtor called from" << QThread::currentThread() << "and the DP thread is" << thread(); -} - void DatagramProcessor::processDatagrams() { PerformanceWarning warn(Menu::getInstance()->isOptionChecked(MenuOption::PipelineWarnings), "DatagramProcessor::processDatagrams()"); diff --git a/interface/src/DatagramProcessor.h b/interface/src/DatagramProcessor.h index 0a6fa3689a..7fc192ee2d 100644 --- a/interface/src/DatagramProcessor.h +++ b/interface/src/DatagramProcessor.h @@ -18,7 +18,6 @@ class DatagramProcessor : public QObject { Q_OBJECT public: DatagramProcessor(QObject* parent = 0); - ~DatagramProcessor(); int getInPacketCount() const { return _inPacketCount; } int getOutPacketCount() const { return _outPacketCount; } diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 9ca229a245..7e1854fb13 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -63,10 +63,6 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned connect(&AccountManager::getInstance(), &AccountManager::logoutComplete , this, &NodeList::reset); } -NodeList::~NodeList() { - qDebug() << "NL dtor called from" << QThread::currentThread() << "and the NL thread is" << thread(); -} - qint64 NodeList::sendStats(const QJsonObject& statsObject, HifiSockAddr destination) { QByteArray statsPacket = byteArrayWithPopulatedHeader(PacketTypeNodeJsonStats); QDataStream statsPacketStream(&statsPacket, QIODevice::Append); diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 0624ae5810..1c6de4bb6c 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -78,8 +78,6 @@ private: NodeList(NodeList const&); // Don't implement, needed to avoid copies of singleton void operator=(NodeList const&); // Don't implement, needed to avoid copies of singleton - ~NodeList(); - void sendSTUNRequest(); bool processSTUNResponse(const QByteArray& packet); From 851d020390f302ccf4ad22bc700f386b123c6deb Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 11 Mar 2015 17:21:49 -0700 Subject: [PATCH 4/9] remove QThread include required for debug --- libraries/networking/src/NodeList.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 7e1854fb13..e63f230f6e 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include From d7e99594aeaab9d984b5f13f27da7abbbd031e8e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 11 Mar 2015 18:04:32 -0700 Subject: [PATCH 5/9] make the application the parent of the node thread --- interface/src/Application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index c0c7bca214..4061ad97f6 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -326,7 +326,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _runningScriptsWidget = new RunningScriptsWidget(_window); // start the nodeThread so its event loop is running - QThread* nodeThread = new QThread(); + QThread* nodeThread = new QThread(this); nodeThread->setObjectName("Datagram Processor Thread"); nodeThread->start(); From decd240f60413b269df24e4a048058423cc73e34 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 12 Mar 2015 14:26:12 +0100 Subject: [PATCH 6/9] Update Login menu item on startup after creation --- interface/src/Application.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index a7a6c8c094..1e8db8619d 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1806,6 +1806,9 @@ void Application::initDisplay() { } void Application::init() { + // Make sure Login state is up to date + DependencyManager::get()->toggleLoginDialog(); + _environment.init(); DependencyManager::get()->init(this); From 0bd97ce40c8659cbc6a164ccea0bcbbae990bfb8 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 12 Mar 2015 18:53:29 +0100 Subject: [PATCH 7/9] Dependency::customDeleter uses a lambda --- libraries/shared/src/DependencyManager.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/libraries/shared/src/DependencyManager.h b/libraries/shared/src/DependencyManager.h index 3868bf14da..a6abe77c3f 100644 --- a/libraries/shared/src/DependencyManager.h +++ b/libraries/shared/src/DependencyManager.h @@ -17,18 +17,25 @@ #include #include +#include #include #define SINGLETON_DEPENDENCY \ friend class DependencyManager; class Dependency { +public: + typedef std::function DeleterFct; + protected: virtual ~Dependency() {} virtual void customDeleter() { - delete this; + _customDeleterFct(this); } - + + void setCustomDeleterFct(DeleterFct customDeleterFct) { _customDeleterFct = customDeleterFct; } + DeleterFct _customDeleterFct = [](Dependency* pointer) { delete pointer; }; + friend class DependencyManager; }; From eb5206f8e61cf1c261df8d9096e30abc220ab357 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 12 Mar 2015 19:05:06 +0100 Subject: [PATCH 8/9] Naming --- libraries/shared/src/DependencyManager.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/shared/src/DependencyManager.h b/libraries/shared/src/DependencyManager.h index a6abe77c3f..01b755fdd0 100644 --- a/libraries/shared/src/DependencyManager.h +++ b/libraries/shared/src/DependencyManager.h @@ -25,16 +25,16 @@ class Dependency { public: - typedef std::function DeleterFct; + typedef std::function DeleterFunction; protected: virtual ~Dependency() {} virtual void customDeleter() { - _customDeleterFct(this); + _customDeleter(this); } - void setCustomDeleterFct(DeleterFct customDeleterFct) { _customDeleterFct = customDeleterFct; } - DeleterFct _customDeleterFct = [](Dependency* pointer) { delete pointer; }; + void setCustomDeleter(DeleterFunction customDeleter) { _customDeleter = customDeleter; } + DeleterFunction _customDeleter = [](Dependency* pointer) { delete pointer; }; friend class DependencyManager; }; From 9269b2a0b2ae9d05abf721f32b3f3f91ad844b16 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Mar 2015 11:28:07 -0700 Subject: [PATCH 9/9] Revert "NOT MERGEABLE: graceful cleanup on Application dtor for NodeList" --- domain-server/src/DomainServer.cpp | 4 ++ interface/src/Application.cpp | 40 ++++++++++--------- interface/src/Application.h | 6 ++- libraries/networking/src/LimitedNodeList.cpp | 5 --- libraries/networking/src/LimitedNodeList.h | 2 - .../networking/src/ThreadedAssignment.cpp | 4 ++ 6 files changed, 34 insertions(+), 27 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 7af9ffd85c..d02ad73b47 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -267,6 +267,10 @@ void DomainServer::setupNodeListAndAssignments(const QUuid& sessionUUID) { connect(nodeList.data(), &LimitedNodeList::nodeAdded, this, &DomainServer::nodeAdded); connect(nodeList.data(), &LimitedNodeList::nodeKilled, this, &DomainServer::nodeKilled); + QTimer* silentNodeTimer = new QTimer(this); + connect(silentNodeTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); + silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); + connect(&nodeList->getNodeSocket(), SIGNAL(readyRead()), SLOT(readAvailableDatagrams())); // add whatever static assignments that have been parsed to the queue diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 5a84c7895a..141a58b317 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -146,6 +146,7 @@ const qint64 MAXIMUM_CACHE_SIZE = 10 * BYTES_PER_GIGABYTES; // 10GB static QTimer* locationUpdateTimer = NULL; static QTimer* balanceUpdateTimer = NULL; +static QTimer* silentNodeTimer = NULL; static QTimer* identityPacketTimer = NULL; static QTimer* billboardPacketTimer = NULL; static QTimer* checkFPStimer = NULL; @@ -257,6 +258,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _dependencyManagerIsSetup(setupEssentials(argc, argv)), _window(new MainWindow(desktop())), _toolWindow(NULL), + _nodeThread(new QThread(this)), _datagramProcessor(), _undoStack(), _undoStackScriptingInterface(&_undoStack), @@ -327,20 +329,18 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : _runningScriptsWidget = new RunningScriptsWidget(_window); // start the nodeThread so its event loop is running - QThread* nodeThread = new QThread(this); - nodeThread->setObjectName("Datagram Processor Thread"); - nodeThread->start(); + _nodeThread->setObjectName("Datagram Processor Thread"); + _nodeThread->start(); // make sure the node thread is given highest priority - nodeThread->setPriority(QThread::TimeCriticalPriority); - - _datagramProcessor = new DatagramProcessor(nodeList.data()); + _nodeThread->setPriority(QThread::TimeCriticalPriority); // put the NodeList and datagram processing on the node thread - nodeList->moveToThread(nodeThread); + nodeList->moveToThread(_nodeThread); + _datagramProcessor.moveToThread(_nodeThread); // connect the DataProcessor processDatagrams slot to the QUDPSocket readyRead() signal - connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, _datagramProcessor, &DatagramProcessor::processDatagrams); + connect(&nodeList->getNodeSocket(), SIGNAL(readyRead()), &_datagramProcessor, SLOT(processDatagrams())); // put the audio processing on a separate thread QThread* audioThread = new QThread(); @@ -427,6 +427,12 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : // connect to the packet sent signal of the _entityEditSender connect(&_entityEditSender, &EntityEditPacketSender::packetSent, this, &Application::packetSent); + // move the silentNodeTimer to the _nodeThread + silentNodeTimer = new QTimer(); + connect(silentNodeTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); + silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); + silentNodeTimer->moveToThread(_nodeThread); + // send the identity packet for our avatar each second to our avatar mixer identityPacketTimer = new QTimer(); connect(identityPacketTimer, &QTimer::timeout, _myAvatar, &MyAvatar::sendIdentityPacket); @@ -541,7 +547,7 @@ void Application::aboutToQuit() { } void Application::cleanupBeforeQuit() { - _datagramProcessor->shutdown(); // tell the datagram processor we're shutting down, so it can short circuit + _datagramProcessor.shutdown(); // tell the datagram processor we're shutting down, so it can short circuit _entities.shutdown(); // tell the entities system we're shutting down, so it will stop running scripts ScriptEngine::stopAllScripts(this); // stop all currently running global scripts @@ -549,6 +555,7 @@ void Application::cleanupBeforeQuit() { // depending on what thread they run in locationUpdateTimer->stop(); balanceUpdateTimer->stop(); + QMetaObject::invokeMethod(silentNodeTimer, "stop", Qt::BlockingQueuedConnection); identityPacketTimer->stop(); billboardPacketTimer->stop(); checkFPStimer->stop(); @@ -558,6 +565,7 @@ void Application::cleanupBeforeQuit() { // and then delete those that got created by "new" delete locationUpdateTimer; delete balanceUpdateTimer; + delete silentNodeTimer; delete identityPacketTimer; delete billboardPacketTimer; delete checkFPStimer; @@ -589,6 +597,10 @@ Application::~Application() { tree->lockForWrite(); _entities.getTree()->setSimulation(NULL); tree->unlock(); + + // ask the datagram processing thread to quit and wait until it is done + _nodeThread->quit(); + _nodeThread->wait(); _octreeProcessor.terminate(); _entityEditSender.terminate(); @@ -608,14 +620,6 @@ Application::~Application() { DependencyManager::destroy(); //DependencyManager::destroy(); DependencyManager::destroy(); - - auto nodeList = DependencyManager::get(); - QThread* nodeThread = nodeList->thread(); - nodeList->deleteLater(); - - // ask the node thread to quit and wait until it is done - nodeThread->quit(); - nodeThread->wait(); qInstallMessageHandler(NULL); // NOTE: Do this as late as possible so we continue to get our log messages } @@ -1494,7 +1498,7 @@ void Application::checkFPS() { _fps = (float)_frameCount / diffTime; _frameCount = 0; - _datagramProcessor->resetCounters(); + _datagramProcessor.resetCounters(); _timerStart.start(); // ask the node list to check in with the domain server diff --git a/interface/src/Application.h b/interface/src/Application.h index 248aaa0f6a..bcd31fcd51 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -445,8 +445,10 @@ private: MainWindow* _window; ToolWindow* _toolWindow; - - DatagramProcessor* _datagramProcessor; + + + QThread* _nodeThread; + DatagramProcessor _datagramProcessor; QUndoStack _undoStack; UndoStackScriptingInterface _undoStackScriptingInterface; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 520dc650ed..279d958082 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -80,10 +80,6 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short connect(localSocketUpdate, &QTimer::timeout, this, &LimitedNodeList::updateLocalSockAddr); localSocketUpdate->start(LOCAL_SOCKET_UPDATE_INTERVAL_MSECS); - QTimer* silentNodeTimer = new QTimer(this); - connect(silentNodeTimer, &QTimer::timeout, this, &LimitedNodeList::removeSilentNodes); - silentNodeTimer->start(NODE_SILENCE_THRESHOLD_MSECS); - // check the local socket right now updateLocalSockAddr(); @@ -504,7 +500,6 @@ void LimitedNodeList::resetPacketStats() { } void LimitedNodeList::removeSilentNodes() { - QSet killedNodes; eachNodeHashIterator([&](NodeHash::iterator& it){ diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index a071eced31..afbdf23fba 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -223,8 +223,6 @@ protected: HifiSockAddr _localSockAddr; HifiSockAddr _publicSockAddr; HifiSockAddr _stunSockAddr; - - QTimer* _silentNodeTimer; // XXX can BandwidthRecorder be used for this? int _numCollectedPackets; diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index 79b4e7f437..ea94a8e22c 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -67,6 +67,10 @@ void ThreadedAssignment::commonInit(const QString& targetName, NodeType_t nodeTy connect(domainServerTimer, SIGNAL(timeout()), this, SLOT(checkInWithDomainServerOrExit())); domainServerTimer->start(DOMAIN_SERVER_CHECK_IN_MSECS); + QTimer* silentNodeRemovalTimer = new QTimer(this); + connect(silentNodeRemovalTimer, SIGNAL(timeout()), nodeList.data(), SLOT(removeSilentNodes())); + silentNodeRemovalTimer->start(NODE_SILENCE_THRESHOLD_MSECS); + if (shouldSendStats) { // send a stats packet every 1 second QTimer* statsTimer = new QTimer(this);