From c542da97076ba0d27f4e191ae5d8249dbf8afe7e Mon Sep 17 00:00:00 2001
From: wangyix <wangyix@gmail.com>
Date: Thu, 12 Jun 2014 12:03:22 -0700
Subject: [PATCH] added locking on _singleSenderStats; untested!

---
 .../src/octree/OctreeInboundPacketProcessor.cpp     |  8 +++++++-
 .../src/octree/OctreeInboundPacketProcessor.h       |  3 +++
 assignment-client/src/octree/OctreeSendThread.cpp   | 13 ++++++-------
 assignment-client/src/octree/OctreeServer.h         |  2 +-
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp b/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp
index c1c564b58c..1912103bf0 100644
--- a/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp
+++ b/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp
@@ -25,7 +25,9 @@ OctreeInboundPacketProcessor::OctreeInboundPacketProcessor(OctreeServer* myServe
     _totalProcessTime(0),
     _totalLockWaitTime(0),
     _totalElementsInPacket(0),
-    _totalPackets(0)
+    _totalPackets(0),
+    _singleSenderStats(),
+    _singleSenderStatsLock()
 {
 }
 
@@ -36,7 +38,9 @@ void OctreeInboundPacketProcessor::resetStats() {
     _totalElementsInPacket = 0;
     _totalPackets = 0;
 
+    _singleSenderStatsLock.lockForWrite();
     _singleSenderStats.clear();
+    _singleSenderStatsLock.unlock();
 }
 
 
@@ -143,7 +147,9 @@ void OctreeInboundPacketProcessor::trackInboundPacket(const QUuid& nodeUUID, uns
     if (_singleSenderStats.find(nodeUUID) == _singleSenderStats.end()) {
         SingleSenderStats stats;
         stats.trackInboundPacket(sequence, transitTime, editsInPacket, processTime, lockWaitTime);
+        _singleSenderStatsLock.lockForWrite();
         _singleSenderStats[nodeUUID] = stats;
+        _singleSenderStatsLock.unlock();
     } else {
         SingleSenderStats& stats = _singleSenderStats[nodeUUID];
         stats.trackInboundPacket(sequence, transitTime, editsInPacket, processTime, lockWaitTime);
diff --git a/assignment-client/src/octree/OctreeInboundPacketProcessor.h b/assignment-client/src/octree/OctreeInboundPacketProcessor.h
index 3e3f5f2dcb..c9760dcc5c 100644
--- a/assignment-client/src/octree/OctreeInboundPacketProcessor.h
+++ b/assignment-client/src/octree/OctreeInboundPacketProcessor.h
@@ -71,6 +71,8 @@ public:
 
     void resetStats();
 
+    void lockSingleSenderStatsForRead() { _singleSenderStatsLock.lockForRead(); }
+    void unlockSingleSenderStats() { _singleSenderStatsLock.unlock(); }
     NodeToSenderStatsMap& getSingleSenderStats() { return _singleSenderStats; }
     const NodeToSenderStatsMap& getSingleSenderStats() const { return _singleSenderStats; }
 
@@ -91,5 +93,6 @@ private:
     quint64 _totalPackets;
     
     NodeToSenderStatsMap _singleSenderStats;
+    QReadWriteLock _singleSenderStatsLock;
 };
 #endif // hifi_OctreeInboundPacketProcessor_h
diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp
index 6b1119fca1..51696e35ea 100644
--- a/assignment-client/src/octree/OctreeSendThread.cpp
+++ b/assignment-client/src/octree/OctreeSendThread.cpp
@@ -120,7 +120,7 @@ int OctreeSendThread::sendNack(OctreeQueryNode* nodeData) {
         return 0;
     }
     
-    const OctreeInboundPacketProcessor* myServerPacketProcessor = _myServer->getInboundPacketProcessor();
+    OctreeInboundPacketProcessor* myServerPacketProcessor = _myServer->getInboundPacketProcessor();
 
     // if there are packets from _node that are waiting to be processed,
     // don't send a NACK since the missing packets may be among those waiting packets.
@@ -128,18 +128,15 @@ int OctreeSendThread::sendNack(OctreeQueryNode* nodeData) {
         return 0;
     }
 
-    // lock unlock required ????????? prolly: _singleSenderStats may have our node's entry deleted during this or something
-    // maybe just make a copy instead!!
+    myServerPacketProcessor->lockSingleSenderStatsForRead();
 
-    // lock
-
-    const QSet<unsigned short int> missingSequenceNumbersFromNode = myServerPacketProcessor
+    const QSet<unsigned short int>& missingSequenceNumbersFromNode = myServerPacketProcessor
         ->getSingleSenderStats().at(_node->getUUID()).getMissingSequenceNumbers();
 
     // check if there are any sequence numbers that need to be nacked
     int numSequenceNumbersAvailable = missingSequenceNumbersFromNode.size();
     if (numSequenceNumbersAvailable == 0) {
-        //unlock
+        myServerPacketProcessor->unlockSingleSenderStats();
         return 0;
     }
 
@@ -170,6 +167,8 @@ int OctreeSendThread::sendNack(OctreeQueryNode* nodeData) {
         dataAt += sizeof(unsigned short int);
     }
 
+    myServerPacketProcessor->unlockSingleSenderStats();
+
     // send it
     OctreeServer::didCallWriteDatagram(this);
     NodeList::getInstance()->writeDatagram((char*)packet, dataAt - packet, _node);
diff --git a/assignment-client/src/octree/OctreeServer.h b/assignment-client/src/octree/OctreeServer.h
index 39536c0981..22b2da0682 100644
--- a/assignment-client/src/octree/OctreeServer.h
+++ b/assignment-client/src/octree/OctreeServer.h
@@ -52,7 +52,7 @@ public:
     int getPacketsTotalPerInterval() const { return _packetsTotalPerInterval; }
     int getPacketsTotalPerSecond() const { return getPacketsTotalPerInterval() * INTERVALS_PER_SECOND; }
 
-    const OctreeInboundPacketProcessor* getInboundPacketProcessor() const { return _octreeInboundPacketProcessor; }
+    OctreeInboundPacketProcessor* getInboundPacketProcessor() { return _octreeInboundPacketProcessor; }
     
     static int getCurrentClientCount() { return _clientCount; }
     static void clientConnected() { _clientCount++; }