Merge pull request #14192 from Atlante45/fix/traits-race

Prevent race on internal client traits members

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>

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>

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>

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

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)
+    void markTraitUpdated(AvatarTraits::TraitType updatedTrait);
-        { _traitStatuses[updatedTrait] = Updated; _hasChangedTraits = true; }
+    void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID);
-    void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID)
+    void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID);
-        { _traitStatuses.instanceInsert(traitType, updatedInstanceID, Updated); _hasChangedTraits = true; }
-    void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID)
-        { _traitStatuses.instanceInsert(traitType, deleteInstanceID, Deleted); _hasChangedTraits = true; }
 
     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 };