From 789235b8c7e31e4efe512b886a0886b0a4e91537 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 14 Jul 2015 15:28:38 -0700 Subject: [PATCH] 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;