From 789235b8c7e31e4efe512b886a0886b0a4e91537 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:28:38 -0700 Subject: [PATCH 01/10] make NodeList deleter be deleteLater --- assignment-client/src/AssignmentClient.cpp | 25 ++++++++-- assignment-client/src/AssignmentClient.h | 2 +- assignment-client/src/audio/AudioMixer.cpp | 5 -- .../src/audio/AudioMixerDatagramProcessor.cpp | 47 ------------------- .../src/audio/AudioMixerDatagramProcessor.h | 32 ------------- assignment-client/src/octree/OctreeServer.cpp | 34 +------------- assignment-client/src/octree/OctreeServer.h | 2 - interface/src/Application.cpp | 9 ++-- libraries/networking/src/NodeList.cpp | 5 ++ libraries/networking/src/NodeList.h | 2 - .../networking/src/PacketReceiverListener.cpp | 0 .../networking/src/PacketReceiverListener.h | 22 --------- .../networking/src/ThreadedAssignment.cpp | 39 +-------------- libraries/networking/src/ThreadedAssignment.h | 2 - 14 files changed, 33 insertions(+), 193 deletions(-) delete mode 100644 assignment-client/src/audio/AudioMixerDatagramProcessor.cpp delete mode 100644 assignment-client/src/audio/AudioMixerDatagramProcessor.h delete mode 100644 libraries/networking/src/PacketReceiverListener.cpp delete mode 100644 libraries/networking/src/PacketReceiverListener.h diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 6da3786ef7..c81a521552 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -62,6 +62,17 @@ AssignmentClient::AssignmentClient(Assignment::Type requestAssignmentType, QStri DependencyManager::registerInheritance(); auto actionFactory = DependencyManager::set(); + // setup a thread for the NodeList and its PacketReceiver + QThread* nodeThread = new QThread(this); + nodeThread->setObjectName("NodeList Thread"); + nodeThread->start(); + + // make sure the node thread is given highest priority + nodeThread->setPriority(QThread::TimeCriticalPriority); + + // put the NodeList on the node thread + nodeList->moveToThread(nodeThread); + // make up a uuid for this child so the parent can tell us apart. This id will be changed // when the domain server hands over an assignment. QUuid nodeUUID = QUuid::createUuid(); @@ -124,7 +135,6 @@ AssignmentClient::AssignmentClient(Assignment::Type requestAssignmentType, QStri packetReceiver.registerListener(PacketType::CreateAssignment, this, "handleCreateAssignmentPacket"); packetReceiver.registerListener(PacketType::StopNode, this, "handleStopNodePacket"); } - void AssignmentClient::stopAssignmentClient() { qDebug() << "Forced stop of assignment-client."; @@ -150,6 +160,16 @@ void AssignmentClient::stopAssignmentClient() { } } +AssignmentClient::~AssignmentClient() { + QThread* nodeThread = DependencyManager::get()->thread(); + + // remove the NodeList from the DependencyManager + DependencyManager::destroy(); + + // ask the node thread to quit and wait until it is done + nodeThread->quit(); + nodeThread->wait(); +} void AssignmentClient::aboutToQuit() { stopAssignmentClient(); @@ -251,9 +271,6 @@ void AssignmentClient::handleCreateAssignmentPacket(QSharedPointer pac _currentAssignment->moveToThread(workerThread); - // move the NodeList to the thread used for the _current assignment - nodeList->moveToThread(workerThread); - // Starts an event loop, and emits workerThread->started() workerThread->start(); } else { diff --git a/assignment-client/src/AssignmentClient.h b/assignment-client/src/AssignmentClient.h index 348255751c..e74cd50065 100644 --- a/assignment-client/src/AssignmentClient.h +++ b/assignment-client/src/AssignmentClient.h @@ -24,10 +24,10 @@ class QSharedMemory; class AssignmentClient : public QObject, public PacketListener { Q_OBJECT public: - AssignmentClient(Assignment::Type requestAssignmentType, QString assignmentPool, QUuid walletUUID, QString assignmentServerHostname, quint16 assignmentServerPort, quint16 assignmentMonitorPort); + ~AssignmentClient(); private slots: void sendAssignmentRequest(); void assignmentCompleted(); diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index a44759ae8d..d43601707d 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -52,7 +52,6 @@ #include "AudioRingBuffer.h" #include "AudioMixerClientData.h" -#include "AudioMixerDatagramProcessor.h" #include "AvatarAudioStream.h" #include "InjectedAudioStream.h" @@ -657,10 +656,6 @@ void AudioMixer::run() { // we do not want this event loop to be the handler for UDP datagrams, so disconnect disconnect(&nodeList->getNodeSocket(), 0, this, 0); - // setup a QThread with us as parent that will house the AudioMixerDatagramProcessor - _datagramProcessingThread = new QThread(this); - _datagramProcessingThread->setObjectName("Datagram Processor Thread"); - nodeList->addNodeTypeToInterestSet(NodeType::Agent); nodeList->linkedDataCreateCallback = [](Node* node) { diff --git a/assignment-client/src/audio/AudioMixerDatagramProcessor.cpp b/assignment-client/src/audio/AudioMixerDatagramProcessor.cpp deleted file mode 100644 index 73a4e01844..0000000000 --- a/assignment-client/src/audio/AudioMixerDatagramProcessor.cpp +++ /dev/null @@ -1,47 +0,0 @@ -// -// AudioMixerDatagramProcessor.cpp -// assignment-client/src -// -// Created by Stephen Birarda on 2014-08-14. -// 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 - -#include -#include - -#include "AudioMixerDatagramProcessor.h" - -AudioMixerDatagramProcessor::AudioMixerDatagramProcessor(QUdpSocket& nodeSocket, QThread* previousNodeSocketThread) : - _nodeSocket(nodeSocket), - _previousNodeSocketThread(previousNodeSocketThread) -{ - -} - -AudioMixerDatagramProcessor::~AudioMixerDatagramProcessor() { - // return the node socket to its previous thread - _nodeSocket.moveToThread(_previousNodeSocketThread); -} - -void AudioMixerDatagramProcessor::readPendingDatagrams() { - - HifiSockAddr senderSockAddr; - static QByteArray incomingPacket; - - // read everything that is available - while (_nodeSocket.hasPendingDatagrams()) { - incomingPacket.resize(_nodeSocket.pendingDatagramSize()); - - // just get this packet off the stack - _nodeSocket.readDatagram(incomingPacket.data(), incomingPacket.size(), - senderSockAddr.getAddressPointer(), senderSockAddr.getPortPointer()); - - // emit the signal to tell AudioMixer it needs to process a packet - emit packetRequiresProcessing(incomingPacket, senderSockAddr); - } -} diff --git a/assignment-client/src/audio/AudioMixerDatagramProcessor.h b/assignment-client/src/audio/AudioMixerDatagramProcessor.h deleted file mode 100644 index 94233a1373..0000000000 --- a/assignment-client/src/audio/AudioMixerDatagramProcessor.h +++ /dev/null @@ -1,32 +0,0 @@ -// -// AudioMixerDatagramProcessor.h -// assignment-client/src -// -// Created by Stephen Birarda on 2014-08-14. -// 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_AudioMixerDatagramProcessor_h -#define hifi_AudioMixerDatagramProcessor_h - -#include -#include - -class AudioMixerDatagramProcessor : public QObject { - Q_OBJECT -public: - AudioMixerDatagramProcessor(QUdpSocket& nodeSocket, QThread* previousNodeSocketThread); - ~AudioMixerDatagramProcessor(); -public slots: - void readPendingDatagrams(); -signals: - void packetRequiresProcessing(const QByteArray& receivedPacket, const HifiSockAddr& senderSockAddr); -private: - QUdpSocket& _nodeSocket; - QThread* _previousNodeSocketThread; -}; - -#endif // hifi_AudioMixerDatagramProcessor_h \ No newline at end of file diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index efcde790f9..dfb832eed4 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -838,36 +838,6 @@ void OctreeServer::handleJurisdictionRequestPacket(QSharedPointer pack _jurisdictionSender->queueReceivedPacket(packet, senderNode); } -void OctreeServer::setupDatagramProcessingThread() { - auto nodeList = DependencyManager::get(); - - // we do not want this event loop to be the handler for UDP datagrams, so disconnect - disconnect(&nodeList->getNodeSocket(), 0, this, 0); - - // setup a QThread with us as parent that will house the OctreeServerDatagramProcessor - _datagramProcessingThread = new QThread(this); - _datagramProcessingThread->setObjectName("Octree Datagram Processor"); - - // create an OctreeServerDatagramProcessor and move it to that thread - OctreeServerDatagramProcessor* datagramProcessor = new OctreeServerDatagramProcessor(nodeList->getNodeSocket(), thread()); - datagramProcessor->moveToThread(_datagramProcessingThread); - - // remove the NodeList as the parent of the node socket - nodeList->getNodeSocket().setParent(NULL); - nodeList->getNodeSocket().moveToThread(_datagramProcessingThread); - - // let the datagram processor handle readyRead from node socket - connect(&nodeList->getNodeSocket(), &QUdpSocket::readyRead, - datagramProcessor, &OctreeServerDatagramProcessor::readPendingDatagrams); - - // delete the datagram processor and the associated thread when the QThread quits - connect(_datagramProcessingThread, &QThread::finished, datagramProcessor, &QObject::deleteLater); - connect(datagramProcessor, &QObject::destroyed, _datagramProcessingThread, &QThread::deleteLater); - - // start the datagram processing thread - _datagramProcessingThread->start(); -} - bool OctreeServer::readOptionBool(const QString& optionName, const QJsonObject& settingsSectionObject, bool& result) { result = false; // assume it doesn't exist bool optionAvailable = false; @@ -1079,15 +1049,13 @@ void OctreeServer::run() { // use common init to setup common timers and logging commonInit(getMyLoggingServerTargetName(), getMyNodeType()); - setupDatagramProcessingThread(); - // read the configuration from either the payload or the domain server configuration readConfiguration(); beforeRun(); // after payload has been processed connect(nodeList.data(), SIGNAL(nodeAdded(SharedNodePointer)), SLOT(nodeAdded(SharedNodePointer))); - connect(nodeList.data(), SIGNAL(nodeKilled(SharedNodePointer)),SLOT(nodeKilled(SharedNodePointer))); + connect(nodeList.data(), SIGNAL(nodeKilled(SharedNodePointer)), SLOT(nodeKilled(SharedNodePointer))); // we need to ask the DS about agents so we can ping/reply with them diff --git a/assignment-client/src/octree/OctreeServer.h b/assignment-client/src/octree/OctreeServer.h index bcc9f8cc43..419cdabc58 100644 --- a/assignment-client/src/octree/OctreeServer.h +++ b/assignment-client/src/octree/OctreeServer.h @@ -145,8 +145,6 @@ protected: QString getConfiguration(); QString getStatusLink(); - void setupDatagramProcessingThread(); - int _argc; const char** _argv; char** _parsedArgV; diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index d02f5daea6..57f2b7c523 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -380,17 +380,12 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : // start the nodeThread so its event loop is running QThread* nodeThread = new QThread(this); - nodeThread->setObjectName("Datagram Processor Thread"); + nodeThread->setObjectName("NodeList Thread"); nodeThread->start(); // make sure the node thread is given highest priority nodeThread->setPriority(QThread::TimeCriticalPriority); - // have the NodeList use deleteLater from DM customDeleter - nodeList->setCustomDeleter([](Dependency* dependency) { - static_cast(dependency)->deleteLater(); - }); - // setup a timer for domain-server check ins QTimer* domainCheckInTimer = new QTimer(nodeList.data()); connect(domainCheckInTimer, &QTimer::timeout, nodeList.data(), &NodeList::sendDomainServerCheckIn); @@ -748,6 +743,8 @@ Application::~Application() { DependencyManager::destroy(); QThread* nodeThread = DependencyManager::get()->thread(); + + // remove the NodeList from the DependencyManager DependencyManager::destroy(); // ask the node thread to quit and wait until it is done diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 3773a8a540..45fdb2590b 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -45,6 +45,11 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned qRegisterMetaType(); firstCall = false; } + + setCustomDeleter([](Dependency* dependency){ + static_cast(dependency)->deleteLater(); + }); + auto addressManager = DependencyManager::get(); // handle domain change signals from AddressManager diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index b9f782f717..72e8d577e0 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -112,8 +112,6 @@ private: DomainHandler _domainHandler; int _numNoReplyDomainCheckIns; HifiSockAddr _assignmentServerSocket; - - friend class Application; }; #endif // hifi_NodeList_h diff --git a/libraries/networking/src/PacketReceiverListener.cpp b/libraries/networking/src/PacketReceiverListener.cpp deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/libraries/networking/src/PacketReceiverListener.h b/libraries/networking/src/PacketReceiverListener.h deleted file mode 100644 index ad0f1c6f7f..0000000000 --- a/libraries/networking/src/PacketReceiverListener.h +++ /dev/null @@ -1,22 +0,0 @@ -// -// PacketListener.h -// libraries/networking/src -// -// Created by Stephen Birarda on 07/14/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 -// - -#ifndef hifi_PacketListener_h -#define hifi_PacketListener_h - -#pragma once - -class PacketListener { -public: - virtual ~PacketListener(); -}; - -#endif // hifi_PacketListener_h diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index b1c4bdbc7f..fe63348e02 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -20,8 +20,8 @@ ThreadedAssignment::ThreadedAssignment(NLPacket& packet) : Assignment(packet), - _isFinished(false), - _datagramProcessingThread(NULL) + _isFinished(false) + { } @@ -42,31 +42,9 @@ void ThreadedAssignment::setFinished(bool isFinished) { _statsTimer->stop(); } - // 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 - _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(); } } @@ -120,16 +98,3 @@ void ThreadedAssignment::checkInWithDomainServerOrExit() { DependencyManager::get()->sendDomainServerCheckIn(); } } - -bool ThreadedAssignment::readAvailableDatagram(QByteArray& destinationByteArray, HifiSockAddr& senderSockAddr) { - auto nodeList = DependencyManager::get(); - - if (nodeList->getNodeSocket().hasPendingDatagrams()) { - destinationByteArray.resize(nodeList->getNodeSocket().pendingDatagramSize()); - nodeList->getNodeSocket().readDatagram(destinationByteArray.data(), destinationByteArray.size(), - senderSockAddr.getAddressPointer(), senderSockAddr.getPortPointer()); - return true; - } else { - return false; - } -} diff --git a/libraries/networking/src/ThreadedAssignment.h b/libraries/networking/src/ThreadedAssignment.h index 922a34b3e4..cfe2363c98 100644 --- a/libraries/networking/src/ThreadedAssignment.h +++ b/libraries/networking/src/ThreadedAssignment.h @@ -38,10 +38,8 @@ signals: void finished(); protected: - bool readAvailableDatagram(QByteArray& destinationByteArray, HifiSockAddr& senderSockAddr); void commonInit(const QString& targetName, NodeType_t nodeType, bool shouldSendStats = true); bool _isFinished; - QThread* _datagramProcessingThread; QTimer* _domainServerTimer = nullptr; QTimer* _statsTimer = nullptr; From 19c1ce2d6f3381de1cfdc5a2fb4de06f2f4002ee Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:29:41 -0700 Subject: [PATCH 02/10] remove the QUDPSocket readyRead hack --- libraries/networking/src/ThreadedAssignment.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index fe63348e02..7631a76a76 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -57,9 +57,6 @@ void ThreadedAssignment::commonInit(const QString& targetName, NodeType_t nodeTy auto nodeList = DependencyManager::get(); nodeList->setOwnerType(nodeType); - // this is a temp fix for Qt 5.3 - rebinding the node socket gives us readyRead for the socket on this thread - nodeList->rebindNodeSocket(); - _domainServerTimer = new QTimer(); connect(_domainServerTimer, SIGNAL(timeout()), this, SLOT(checkInWithDomainServerOrExit())); _domainServerTimer->start(DOMAIN_SERVER_CHECK_IN_MSECS); From e749b9727f1e0cdd7719f65009f33f973a00cfeb Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:33:05 -0700 Subject: [PATCH 03/10] have threaded assignment de-register for packets in finished --- libraries/networking/src/ThreadedAssignment.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index 7631a76a76..44d1aaf280 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -34,6 +34,9 @@ void ThreadedAssignment::setFinished(bool isFinished) { qDebug() << "ThreadedAssignment::setFinished(true) called - finishing up."; + // we should de-register immediately for any of our packets + DependencyManager::get()->getPacketReceiver().unregisterListener(this); + if (_domainServerTimer) { _domainServerTimer->stop(); } From 55eb24e69ef9a0268aa8c4a4ae9e93b9ab95961e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:41:31 -0700 Subject: [PATCH 04/10] call right method from packetReceiver --- libraries/networking/src/PacketReceiver.cpp | 32 ++++++++++++--------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 20cc5c5b15..31460de3b9 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -218,24 +218,32 @@ void PacketReceiver::processDatagrams() { auto listener = it.value(); if (listener.first) { + + bool success = false; if (matchingNode) { emit dataReceived(matchingNode->getType(), packet->getSizeWithHeader()); + QMetaMethod metaMethod = listener.second; + + static const QByteArray SHARED_NODE_NORMALIZED = QMetaObject::normalizedType("QSharedPointer"); + + if (metaMethod.parameterTypes().contains(SHARED_NODE_NORMALIZED)) { + success = metaMethod.invoke(listener.first, + Q_ARG(QSharedPointer, QSharedPointer(packet.release())), + Q_ARG(SharedNodePointer, matchingNode)); + + } else { + success = metaMethod.invoke(listener.first, + Q_ARG(QSharedPointer, QSharedPointer(packet.release()))); + } + } else { emit dataReceived(NodeType::Unassigned, packet->getSizeWithHeader()); - } - - bool success = false; - if (matchingNode) { success = listener.second.invoke(listener.first, Q_ARG(QSharedPointer, QSharedPointer(packet.release()))); - } else { - success = listener.second.invoke(listener.first, - Q_ARG(QSharedPointer, QSharedPointer(packet.release())), - Q_ARG(SharedNodePointer, matchingNode)); } - + if (!success) { qDebug() << "Error delivering packet " << nameForPacketType(packet->getType()) << " to listener: " << listener.first->objectName() << "::" << listener.second.name(); @@ -247,13 +255,11 @@ void PacketReceiver::processDatagrams() { << "has been destroyed - removing mapping."; _packetListenerMap.erase(it); } - - _packetListenerLock.unlock(); - } else { - _packetListenerLock.unlock(); qDebug() << "No listener found for packet type " << nameForPacketType(packet->getType()); } + + _packetListenerLock.unlock(); } } } From ca47165d72473818444dce42212e8573030886cf Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:42:28 -0700 Subject: [PATCH 05/10] compare bytesAvailable to 0 --- libraries/octree/src/OctreeRenderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/octree/src/OctreeRenderer.cpp b/libraries/octree/src/OctreeRenderer.cpp index 54ce95b803..9f4cd0d1a1 100644 --- a/libraries/octree/src/OctreeRenderer.cpp +++ b/libraries/octree/src/OctreeRenderer.cpp @@ -108,7 +108,7 @@ void OctreeRenderer::processDatagram(NLPacket& packet, SharedNodePointer sourceN bool error = false; - while (packet.bytesAvailable() && !error) { + while (packet.bytesAvailable() > 0 && !error) { if (packetIsCompressed) { if (packet.bytesAvailable() > (qint64) sizeof(OCTREE_PACKET_INTERNAL_SECTION_SIZE)) { packet.readPrimitive(§ionLength); From b7b2cb73efa87e5d96274673e7c7a1206ff28f06 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:43:10 -0700 Subject: [PATCH 06/10] remove check for shutting down --- libraries/networking/src/PacketReceiver.cpp | 4 ---- libraries/networking/src/PacketReceiver.h | 2 -- 2 files changed, 6 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 31460de3b9..1f163cdaf3 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -174,10 +174,6 @@ void PacketReceiver::processDatagrams() { //PerformanceWarning warn(Menu::getInstance()->isOptionChecked(MenuOption::PipelineWarnings), //"PacketReceiver::processDatagrams()"); - if (_isShuttingDown) { - return; // bail early... we're shutting down. - } - auto nodeList = DependencyManager::get(); while (nodeList->getNodeSocket().hasPendingDatagrams()) { diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index c84b5ef432..bf5889af1e 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -37,8 +37,6 @@ public: void resetCounters() { _inPacketCount = 0; _inByteCount = 0; } - void shutdown() { _isShuttingDown = true; } - void registerListenerForTypes(const QSet& types, PacketListener* listener, const char* slot); void registerListener(PacketType::Value type, PacketListener* listener, const char* slot); void unregisterListener(PacketListener* listener); From fb7cb7ff5368b8c0077055835c372204c8070207 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:49:54 -0700 Subject: [PATCH 07/10] add the option for PacketReceiver to drop packets --- assignment-client/src/AssignmentClient.cpp | 3 +++ interface/src/Application.cpp | 9 ++++----- libraries/networking/src/PacketReceiver.cpp | 5 +++++ libraries/networking/src/PacketReceiver.h | 4 +++- libraries/networking/src/ThreadedAssignment.cpp | 7 ++++++- 5 files changed, 21 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index c81a521552..bc2b18cb53 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -330,6 +330,9 @@ void AssignmentClient::assignmentCompleted() { auto nodeList = DependencyManager::get(); + // tell the packet receiver to stop dropping packets + nodeList->getPacketReceiver().setShouldDropPackets(false); + // reset our NodeList by switching back to unassigned and clearing the list nodeList->setOwnerType(NodeType::Unassigned); nodeList->reset(); diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 57f2b7c523..824de916d5 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -663,12 +663,11 @@ void Application::aboutToQuit() { void Application::cleanupBeforeQuit() { - // stop handling packets we've asked to handle - DependencyManager::get()->getPacketReceiver().unregisterListener(this); - _entities.clear(); // this will allow entity scripts to properly shutdown - - //_datagramProcessor->shutdown(); // tell the datagram processor we're shutting down, so it can short circuit + + // tell the packet receiver we're shutting down, so it can drop packets + DependencyManager::get()->getPacketReceiver().setShouldDropPackets(true); + _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 diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 1f163cdaf3..4c3f268d06 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -180,6 +180,11 @@ void PacketReceiver::processDatagrams() { // setup a buffer to read the packet into int packetSizeWithHeader = nodeList->getNodeSocket().pendingDatagramSize(); std::unique_ptr buffer = std::unique_ptr(new char[packetSizeWithHeader]); + + // if we're supposed to drop this packet then break out here + if (_shouldDropPackets) { + break; + } // setup a HifiSockAddr to read into HifiSockAddr senderSockAddr; diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index bf5889af1e..448ca07876 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -34,6 +34,8 @@ public: int getInPacketCount() const { return _inPacketCount; } int getInByteCount() const { return _inByteCount; } + + void setShouldDropPackets(bool shouldDropPackets) { _shouldDropPackets = shouldDropPackets; } void resetCounters() { _inPacketCount = 0; _inByteCount = 0; } @@ -61,7 +63,7 @@ private: QHash _packetListenerMap; int _inPacketCount = 0; int _inByteCount = 0; - bool _isShuttingDown = false; + bool _shouldDropPackets = false; }; #endif // hifi_PacketReceiver_h diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index 44d1aaf280..1c425806c9 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -34,8 +34,13 @@ void ThreadedAssignment::setFinished(bool isFinished) { qDebug() << "ThreadedAssignment::setFinished(true) called - finishing up."; + auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); + // we should de-register immediately for any of our packets - DependencyManager::get()->getPacketReceiver().unregisterListener(this); + packetReceiver.unregisterListener(this); + + // we should also tell the packet receiver to drop packets while we're cleaning up + packetReceiver.setShouldDropPackets(true); if (_domainServerTimer) { _domainServerTimer->stop(); From bbe5a3d682174464b4dc8ad33462b1a4657e3db7 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:51:32 -0700 Subject: [PATCH 08/10] remove commented out call to resetCounters --- interface/src/Application.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 824de916d5..f9ac9e3a2c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1797,7 +1797,6 @@ void Application::checkFPS() { _fps = (float)_frameCount / diffTime; _frameCount = 0; - //_datagramProcessor->resetCounters(); _timerStart.start(); } From b4d01c464408855e7a50ec9bd4ff459d2863ba79 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:52:16 -0700 Subject: [PATCH 09/10] fix variable name for coding standard --- libraries/networking/src/PacketReceiver.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index 448ca07876..f0066115e0 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -47,8 +47,8 @@ public slots: void processDatagrams(); signals: - void dataSent(quint8 channel_type, int bytes); - void dataReceived(quint8 channel_type, int bytes); + void dataSent(quint8 channelType, int bytes); + void dataReceived(quint8 channelType, int bytes); void packetVersionMismatch(PacketType::Value type); private: From 5edb809cd1509192e3b3ca3dbfd595bb45a9b5f1 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:54:23 -0700 Subject: [PATCH 10/10] remove header packet methods from LimitedNodeList --- libraries/networking/src/LimitedNodeList.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 5e23b2e93c..d397d5dea8 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -122,13 +122,6 @@ public: PacketReceiver& getPacketReceiver() { return _packetReceiver; } - // QByteArray byteArrayWithPopulatedHeader(PacketType::Value packetType) - // { return byteArrayWithUUIDPopulatedHeader(packetType, _sessionUUID); } - // int populatePacketHeader(QByteArray& packet, PacketType::Value packetType) - // { return populatePacketHeaderWithUUID(packet, packetType, _sessionUUID); } - // int populatePacketHeader(char* packet, PacketType::Value packetType) - // { return populatePacketHeaderWithUUID(packet, packetType, _sessionUUID); } - qint64 sendUnreliablePacket(const NLPacket& packet, const SharedNodePointer& destinationNode) { assert(false); return 0; } qint64 sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr) { assert(false); return 0; }