Merge pull request #4437 from birarda/master

graceful cleanup of NodeList in Application

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

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;
@@ -258,7 +257,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),
@@ -329,18 +327,25 @@ 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(this);
+    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());
+    
+    // have the NodeList use deleteLater from DM customDeleter
+    nodeList->setCustomDeleter([](Dependency* dependency) {
+        static_cast<NodeList*>(dependency)->deleteLater();
+    });
     
     // 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();
@@ -427,12 +432,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);
@@ -547,7 +546,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
     
@@ -555,7 +554,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();
@@ -565,7 +563,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;
@@ -598,10 +595,6 @@ Application::~Application() {
     _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();
 
@@ -621,6 +614,13 @@ Application::~Application() {
     //DependencyManager::destroy<ScriptCache>();
     DependencyManager::destroy<SoundCache>();
     
+    QThread* nodeThread = DependencyManager::get<NodeList>()->thread();
+    DependencyManager::destroy<NodeList>();
+    
+    // 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
 }
 
@@ -1498,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
@@ -2739,7 +2739,6 @@ const GLfloat WORLD_DIFFUSE_COLOR[] = { 0.6f, 0.525f, 0.525f };
 const GLfloat WORLD_SPECULAR_COLOR[] = { 0.94f, 0.94f, 0.737f, 1.0f };
 
 const glm::vec3 GLOBAL_LIGHT_COLOR = {  0.6f, 0.525f, 0.525f };
-const float GLOBAL_LIGHT_INTENSITY = 1.0f;
 
 void Application::setupWorldLight() {
 

interface/src/Application.h

@@ -446,9 +446,7 @@ private:
 
     ToolWindow* _toolWindow;
     
-
-    QThread* _nodeThread;
-    DatagramProcessor _datagramProcessor;
+    DatagramProcessor* _datagramProcessor;
 
     QUndoStack _undoStack;
     UndoStackScriptingInterface _undoStackScriptingInterface;

interface/src/avatar/MyAvatar.cpp

@@ -49,8 +49,6 @@ using namespace std;
 const glm::vec3 DEFAULT_UP_DIRECTION(0.0f, 1.0f, 0.0f);
 const float YAW_SPEED = 500.0f;   // degrees/sec
 const float PITCH_SPEED = 100.0f; // degrees/sec
-const float COLLISION_RADIUS_SCALAR = 1.2f; // pertains to avatar-to-avatar collisions
-const float COLLISION_RADIUS_SCALE = 0.125f;
 const float DEFAULT_REAL_WORLD_FIELD_OF_VIEW_DEGREES = 30.0f;
 
 const float MAX_WALKING_SPEED = 2.5f; // human walking speed

interface/src/avatar/SkeletonModel.cpp

@@ -791,8 +791,6 @@ void SkeletonModel::renderBoundingCollisionShapes(float alpha) {
     glPopMatrix();
 }
 
-const int BALL_SUBDIVISIONS = 10;
-
 bool SkeletonModel::hasSkeleton() {
     return isActive() ? _geometry->getFBXGeometry().rootJointIndex != -1 : false;
 }

libraries/entities/src/LightEntityItem.cpp

@@ -32,7 +32,6 @@ LightEntityItem::LightEntityItem(const EntityItemID& entityItemID, const EntityI
     _type = EntityTypes::Light;
     
     // default property values
-    const quint8 MAX_COLOR = 255;
     _color[RED_INDEX] = _color[GREEN_INDEX] = _color[BLUE_INDEX] = 0;
     _intensity = 1.0f;
     _exponent = 0.0f;

libraries/model/src/model/Stage.cpp

@@ -267,7 +267,7 @@ void SunSkyStage::updateGraphicsObject() const {
     static int firstTime = 0;
     if (firstTime == 0) {
         firstTime++;
-        bool result = gpu::Shader::makeProgram(*(_skyPipeline->getProgram()));
+        gpu::Shader::makeProgram(*(_skyPipeline->getProgram()));
     
     }
 

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<SharedNodePointer> killedNodes;
     
     eachNodeHashIterator([&](NodeHash::iterator& it){

libraries/networking/src/LimitedNodeList.h

@@ -224,6 +224,8 @@ protected:
     HifiSockAddr _publicSockAddr;
     HifiSockAddr _stunSockAddr;
     
+    QTimer* _silentNodeTimer;
+
     // XXX can BandwidthRecorder be used for this?
     int _numCollectedPackets;
     int _numCollectedBytes;

libraries/networking/src/NodeList.h

@@ -37,6 +37,7 @@ const quint64 DOMAIN_SERVER_CHECK_IN_MSECS = 1 * 1000;
 
 const int MAX_SILENT_DOMAIN_SERVER_CHECK_INS = 5;
 
+class Application;
 class Assignment;
 
 class NodeList : public LimitedNodeList {
@@ -95,6 +96,8 @@ private:
     HifiSockAddr _assignmentServerSocket;
     bool _hasCompletedInitialSTUNFailure;
     unsigned int _stunRequestsSinceSuccess;
+    
+    friend class Application;
 };
 
 #endif // hifi_NodeList_h

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);

libraries/physics/src/MeshInfo.cpp

@@ -17,9 +17,11 @@ using namespace meshinfo;
 //origin is the default reference point for generating the tetrahedron from each triangle of the mesh.
 
 MeshInfo::MeshInfo(vector<Vertex> *vertices, vector<int> *triangles) :\
-_vertices(vertices),
-_triangles(triangles),
-_centerOfMass(Vertex(0.0, 0.0, 0.0)){
+    _vertices(vertices),
+    _centerOfMass(Vertex(0.0, 0.0, 0.0)),
+    _triangles(triangles)
+{
+    
 }
 
 MeshInfo::~MeshInfo(){