Merge pull request #1553 from ZappoMan/generic_thread_fix

fixes leak of GenericThread and also fixes crash in octree servers on client disconnect

interface/src/Application.cpp

@@ -1427,13 +1427,13 @@ void Application::terminate() {
     // let the avatar mixer know we're out
     NodeList::getInstance()->sendKillNode(&NODE_TYPE_AVATAR_MIXER, 1);
 
-    printf("");
     _voxelProcessor.terminate();
     _voxelHideShowThread.terminate();
     _voxelEditSender.terminate();
     _particleEditSender.terminate();
     if (_persistThread) {
         _persistThread->terminate();
+        _persistThread->deleteLater();
         _persistThread = NULL;
     }
 }
@@ -4395,6 +4395,7 @@ void Application::updateLocalOctreeCache(bool firstTime) {
 
         if (_persistThread) {
             _persistThread->terminate();
+            _persistThread->deleteLater();
             _persistThread = NULL;
         }
 

libraries/octree-server/src/OctreeQueryNode.cpp

@@ -158,13 +158,13 @@ void OctreeQueryNode::writeToPacket(const unsigned char* buffer, int bytes) {
 }
 
 OctreeQueryNode::~OctreeQueryNode() {
-    delete[] _octreePacket;
-    delete[] _lastOctreePacket;
-
     if (_octreeSendThread) {
         _octreeSendThread->terminate();
-        delete _octreeSendThread;
+        _octreeSendThread->deleteLater();
     }
+
+    delete[] _octreePacket;
+    delete[] _lastOctreePacket;
 }
 
 bool OctreeQueryNode::updateCurrentViewFrustum() {

libraries/octree-server/src/OctreeServer.cpp

@@ -75,22 +75,22 @@ OctreeServer::~OctreeServer() {
 
     if (_jurisdictionSender) {
         _jurisdictionSender->terminate();
-        delete _jurisdictionSender;
+        _jurisdictionSender->deleteLater();
     }
 
     if (_octreeInboundPacketProcessor) {
         _octreeInboundPacketProcessor->terminate();
-        delete _octreeInboundPacketProcessor;
+        _octreeInboundPacketProcessor->deleteLater();
     }
 
     if (_persistThread) {
         _persistThread->terminate();
-        delete _persistThread;
+        _persistThread->deleteLater();
     }
 
     delete _jurisdiction;
     _jurisdiction = NULL;
-    
+
     qDebug() << "OctreeServer::run()... DONE";
 }
 
@@ -504,9 +504,9 @@ void OctreeServer::processDatagram(const QByteArray& dataByteArray, const HifiSo
         // need to make sure we have it in our nodeList.
         QUuid nodeUUID = QUuid::fromRfc4122(dataByteArray.mid(numBytesPacketHeader,
                                                               NUM_BYTES_RFC4122_UUID));
-        
+
         SharedNodePointer node = nodeList->nodeWithUUID(nodeUUID);
-        
+
         if (node) {
             nodeList->updateNodeWithData(node.data(), senderSockAddr, (unsigned char *) dataByteArray.data(),
                                          dataByteArray.size());
@@ -593,7 +593,7 @@ void OctreeServer::run() {
     setvbuf(stdout, NULL, _IOLBF, 0);
 
     // tell our NodeList about our desire to get notifications
-    connect(nodeList, SIGNAL(nodeKilled(SharedNodePointer)), SLOT(nodeKilled(SharedNodePointer)));
+    connect(nodeList, SIGNAL(nodeKilled(SharedNodePointer)), this, SLOT(nodeKilled(SharedNodePointer)));
     nodeList->linkedDataCreateCallback = &OctreeServer::attachQueryNodeToNode;
 
     srand((unsigned)time(0));
@@ -685,7 +685,7 @@ void OctreeServer::run() {
         strftime(utcBuffer, MAX_TIME_LENGTH, " [%m/%d/%Y %X UTC]", gmtm);
     }
     qDebug() << "Now running... started at: " << localBuffer << utcBuffer;
-    
+
     QTimer* domainServerTimer = new QTimer(this);
     connect(domainServerTimer, SIGNAL(timeout()), this, SLOT(checkInWithDomainServerOrExit()));
     domainServerTimer->start(DOMAIN_SERVER_CHECK_IN_USECS / 1000);

libraries/shared/src/GenericThread.cpp

@@ -8,8 +8,11 @@
 //  Generic Threaded or non-threaded processing class
 //
 
+#include <QDebug>
+
 #include "GenericThread.h"
 
+
 GenericThread::GenericThread() :
     _stopThread(false),
     _isThreaded(false) // assume non-threaded, must call initialize()
@@ -23,15 +26,11 @@ GenericThread::~GenericThread() {
 void GenericThread::initialize(bool isThreaded) {
     _isThreaded = isThreaded;
     if (_isThreaded) {
-        QThread* _thread = new QThread(this);
+        _thread = new QThread(this);
 
         // when the worker thread is started, call our engine's run..
         connect(_thread, SIGNAL(started()), this, SLOT(threadRoutine()));
 
-        // XXXBHG: this is a known memory leak/thread leak. I will fix this shortly.
-        //connect(this, SIGNAL(finished()), this, SLOT(deleteLater()));
-        //connect(_thread, SIGNAL(finished()), _thread, SLOT(deleteLater()));
-
         this->moveToThread(_thread);
 
         // Starts an event loop, and emits _thread->started()
@@ -42,7 +41,11 @@ void GenericThread::initialize(bool isThreaded) {
 void GenericThread::terminate() {
     if (_isThreaded) {
         _stopThread = true;
-        //_isThreaded = false;
+
+        if (_thread) {
+            _thread->wait();
+            _thread->deleteLater();
+        }
     }
 }
 
@@ -61,7 +64,10 @@ void GenericThread::threadRoutine() {
         }
     }
 
-    if (_isThreaded) {
+    // If we were on a thread, then quit our thread
-        emit finished();
+    if (_isThreaded && _thread) {
+        _thread->quit();
     }
+
+    emit finished();
 }