Merge pull request #6510 from birarda/es-deadlock

fix infinite loop on shutdown in OctreeInboundPacketProcessor

assignment-client/src/octree/OctreeInboundPacketProcessor.cpp

@@ -43,6 +43,7 @@ void OctreeInboundPacketProcessor::resetStats() {
     _totalPackets = 0;
     _lastNackTime = usecTimestampNow();
 
+    QWriteLocker locker(&_senderStatsLock);
     _singleSenderStats.clear();
 }
 
@@ -220,6 +221,8 @@ void OctreeInboundPacketProcessor::trackInboundPacket(const QUuid& nodeUUID, uns
     _totalElementsInPacket += editsInPacket;
     _totalPackets++;
 
+    QWriteLocker locker(&_senderStatsLock);
+
     // find the individual senders stats and track them there too...
     // see if this is the first we've heard of this node...
     if (_singleSenderStats.find(nodeUUID) == _singleSenderStats.end()) {
@@ -242,6 +245,8 @@ int OctreeInboundPacketProcessor::sendNackPackets() {
     int packetsSent = 0;
     int totalBytesSent = 0;
 
+    QWriteLocker locker(&_senderStatsLock);
+
     NodeToSenderStatsMapIterator i = _singleSenderStats.begin();
     while (i != _singleSenderStats.end()) {
 
@@ -262,10 +267,9 @@ int OctreeInboundPacketProcessor::sendNackPackets() {
         }
 
         const SharedNodePointer& destinationNode = DependencyManager::get<NodeList>()->nodeWithUUID(nodeUUID);
-        // If the node no longer exists, wait until the ReceivedPacketProcessor has cleaned up the node
-        // to remove it from our stats list.
-        // FIXME Is it safe to clean it up here before ReceivedPacketProcess has?
+        // if the node no longer exists, remove its stats
         if (!destinationNode) {
+            i = _singleSenderStats.erase(i);
             continue;
         }
 

assignment-client/src/octree/OctreeInboundPacketProcessor.h

@@ -72,7 +72,7 @@ public:
 
     void resetStats();
 
-    NodeToSenderStatsMap& getSingleSenderStats() { return _singleSenderStats; }
+    NodeToSenderStatsMap getSingleSenderStats() { QReadLocker locker(&_senderStatsLock); return _singleSenderStats; }
 
     virtual void terminating() { _shuttingDown = true; ReceivedPacketProcessor::terminating(); }
 
@@ -94,15 +94,16 @@ private:
     OctreeServer* _myServer;
     int _receivedPacketCount;
     
-    quint64 _totalTransitTime;
-    quint64 _totalProcessTime;
-    quint64 _totalLockWaitTime;
-    quint64 _totalElementsInPacket;
-    quint64 _totalPackets;
+    std::atomic<uint64_t> _totalTransitTime;
+    std::atomic<uint64_t> _totalProcessTime;
+    std::atomic<uint64_t> _totalLockWaitTime;
+    std::atomic<uint64_t> _totalElementsInPacket;
+    std::atomic<uint64_t> _totalPackets;
     
     NodeToSenderStatsMap _singleSenderStats;
+    QReadWriteLock _senderStatsLock;
 
-    quint64 _lastNackTime;
+    std::atomic<uint64_t> _lastNackTime;
     bool _shuttingDown;
 };
 #endif // hifi_OctreeInboundPacketProcessor_h

assignment-client/src/octree/OctreeServer.cpp

@@ -711,7 +711,7 @@ bool OctreeServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url
 
 
         int senderNumber = 0;
-        NodeToSenderStatsMap& allSenderStats = _octreeInboundPacketProcessor->getSingleSenderStats();
+        NodeToSenderStatsMap allSenderStats = _octreeInboundPacketProcessor->getSingleSenderStats();
         for (NodeToSenderStatsMapConstIterator i = allSenderStats.begin(); i != allSenderStats.end(); i++) {
             senderNumber++;
             QUuid senderID = i.key();