From 1bcbda7ad6454988e4fba25c2e1f389109ba701d Mon Sep 17 00:00:00 2001
From: Clement <clement.brisset@gmail.com>
Date: Thu, 11 Oct 2018 16:16:24 -0700
Subject: [PATCH] Prevent race on internal client traits members

---
 assignment-client/src/Agent.h                 |  1 -
 interface/src/avatar/MyAvatar.cpp             |  1 +
 interface/src/avatar/MyAvatar.h               |  1 -
 libraries/avatars/src/ClientTraitsHandler.cpp | 23 +++++++++++++++++++
 libraries/avatars/src/ClientTraitsHandler.h   | 19 +++++++--------
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h
index 2b5ff51b49..7d47c8e713 100644
--- a/assignment-client/src/Agent.h
+++ b/assignment-client/src/Agent.h
@@ -21,7 +21,6 @@
 #include <QtCore/QTimer>
 #include <QUuid>
 
-#include <ClientTraitsHandler.h>
 #include <EntityEditPacketSender.h>
 #include <EntityTree.h>
 #include <ScriptEngine.h>
diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp
index b347963cf1..5418410dd4 100755
--- a/interface/src/avatar/MyAvatar.cpp
+++ b/interface/src/avatar/MyAvatar.cpp
@@ -26,6 +26,7 @@
 #include <AccountManager.h>
 #include <AddressManager.h>
 #include <AudioClient.h>
+#include <ClientTraitsHandler.h>
 #include <display-plugins/DisplayPlugin.h>
 #include <FSTReader.h>
 #include <GeometryUtil.h>
diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index 16b765711a..c99fd3bce6 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -21,7 +21,6 @@
 #include <AvatarConstants.h>
 #include <avatars-renderer/Avatar.h>
 #include <avatars-renderer/ScriptAvatar.h>
-#include <ClientTraitsHandler.h>
 #include <controllers/Pose.h>
 #include <controllers/Actions.h>
 #include <EntityItem.h>
diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp
index a06b53da7c..f8247d9e52 100644
--- a/libraries/avatars/src/ClientTraitsHandler.cpp
+++ b/libraries/avatars/src/ClientTraitsHandler.cpp
@@ -31,7 +31,27 @@ ClientTraitsHandler::ClientTraitsHandler(AvatarData* owningAvatar) :
     nodeList->getPacketReceiver().registerListener(PacketType::SetAvatarTraits, this, "processTraitOverride");
 }
 
+void ClientTraitsHandler::markTraitUpdated(AvatarTraits::TraitType updatedTrait) {
+    Lock lock(_traitLock);
+    _traitStatuses[updatedTrait] = Updated;
+    _hasChangedTraits = true;
+}
+
+void ClientTraitsHandler::markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID) {
+    Lock lock(_traitLock);
+    _traitStatuses.instanceInsert(traitType, updatedInstanceID, Updated);
+    _hasChangedTraits = true;
+}
+
+void ClientTraitsHandler::markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID) {
+    Lock lock(_traitLock);
+    _traitStatuses.instanceInsert(traitType, deleteInstanceID, Deleted);
+    _hasChangedTraits = true;
+}
+
 void ClientTraitsHandler::resetForNewMixer() {
+    Lock lock(_traitLock);
+
     // re-set the current version to 0
     _currentTraitVersion = AvatarTraits::DEFAULT_TRAIT_VERSION;
 
@@ -46,6 +66,8 @@ void ClientTraitsHandler::resetForNewMixer() {
 }
 
 void ClientTraitsHandler::sendChangedTraitsToMixer() {
+    Lock lock(_traitLock);
+
     if (hasChangedTraits() || _shouldPerformInitialSend) {
         // we have at least one changed trait to send
 
@@ -113,6 +135,7 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() {
 
 void ClientTraitsHandler::processTraitOverride(QSharedPointer<ReceivedMessage> message, SharedNodePointer sendingNode) {
     if (sendingNode->getType() == NodeType::AvatarMixer) {
+        Lock lock(_traitLock);
         while (message->getBytesLeftToRead()) {
             AvatarTraits::TraitType traitType;
             message->readPrimitive(&traitType);
diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h
index 27ba58d46b..3900268101 100644
--- a/libraries/avatars/src/ClientTraitsHandler.h
+++ b/libraries/avatars/src/ClientTraitsHandler.h
@@ -26,14 +26,11 @@ public:
 
     void sendChangedTraitsToMixer();
 
-    bool hasChangedTraits() { return _hasChangedTraits; }
+    bool hasChangedTraits() const { return _hasChangedTraits; }
 
-    void markTraitUpdated(AvatarTraits::TraitType updatedTrait)
-        { _traitStatuses[updatedTrait] = Updated; _hasChangedTraits = true; }
-    void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID)
-        { _traitStatuses.instanceInsert(traitType, updatedInstanceID, Updated); _hasChangedTraits = true; }
-    void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID)
-        { _traitStatuses.instanceInsert(traitType, deleteInstanceID, Deleted); _hasChangedTraits = true; }
+    void markTraitUpdated(AvatarTraits::TraitType updatedTrait);
+    void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID);
+    void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID);
 
     void resetForNewMixer();
 
@@ -41,17 +38,21 @@ public slots:
     void processTraitOverride(QSharedPointer<ReceivedMessage> message, SharedNodePointer sendingNode);
 
 private:
+    using Mutex = std::recursive_mutex;
+    using Lock = std::lock_guard<Mutex>;
+
     enum ClientTraitStatus {
         Unchanged,
         Updated,
         Deleted
     };
 
-    AvatarData* _owningAvatar;
+    AvatarData* const _owningAvatar;
 
+    Mutex _traitLock;
     AvatarTraits::AssociatedTraitValues<ClientTraitStatus, Unchanged> _traitStatuses;
-    AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION };
 
+    AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION };
     AvatarTraits::TraitVersion _currentSkeletonVersion { AvatarTraits::NULL_TRAIT_VERSION };
     
     bool _shouldPerformInitialSend { false };