From a7d75b121fe5f0fdd944144daf422a64f207c632 Mon Sep 17 00:00:00 2001
From: SamGondelman <samuel_gondelman@alumni.brown.edu>
Date: Fri, 8 Jun 2018 11:20:41 -0700
Subject: [PATCH] fix avatarHash crash

---
 interface/src/avatar/AvatarManager.cpp  | 8 +++++---
 libraries/avatars/src/AvatarHashMap.cpp | 1 +
 libraries/avatars/src/AvatarHashMap.h   | 4 +---
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp
index 2e9c9fdecd..4d133706e6 100644
--- a/interface/src/avatar/AvatarManager.cpp
+++ b/interface/src/avatar/AvatarManager.cpp
@@ -468,13 +468,14 @@ void AvatarManager::updateAvatarRenderStatus(bool shouldRenderAvatars) {
     _shouldRender = shouldRenderAvatars;
     const render::ScenePointer& scene = qApp->getMain3DScene();
     render::Transaction transaction;
+    auto avatarHashCopy = getHashCopy();
     if (_shouldRender) {
-        for (auto avatarData : _avatarHash) {
+        for (auto avatarData : avatarHashCopy) {
             auto avatar = std::static_pointer_cast<Avatar>(avatarData);
             avatar->addToScene(avatar, scene, transaction);
         }
     } else {
-        for (auto avatarData : _avatarHash) {
+        for (auto avatarData : avatarHashCopy) {
             auto avatar = std::static_pointer_cast<Avatar>(avatarData);
             avatar->removeFromScene(avatar, scene, transaction);
         }
@@ -514,7 +515,8 @@ RayToAvatarIntersectionResult AvatarManager::findRayIntersectionVector(const Pic
 
     glm::vec3 normDirection = glm::normalize(ray.direction);
 
-    for (auto avatarData : _avatarHash) {
+    auto avatarHashCopy = getHashCopy();
+    for (auto avatarData : avatarHashCopy) {
         auto avatar = std::static_pointer_cast<Avatar>(avatarData);
         if ((avatarsToInclude.size() > 0 && !avatarsToInclude.contains(avatar->getID())) ||
             (avatarsToDiscard.size() > 0 && avatarsToDiscard.contains(avatar->getID()))) {
diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp
index 829c98a418..974ae92432 100644
--- a/libraries/avatars/src/AvatarHashMap.cpp
+++ b/libraries/avatars/src/AvatarHashMap.cpp
@@ -82,6 +82,7 @@ AvatarSharedPointer AvatarHashMap::addAvatar(const QUuid& sessionUUID, const QWe
     avatar->setSessionUUID(sessionUUID);
     avatar->setOwningAvatarMixer(mixerWeakPointer);
 
+    // addAvatar is only called from newOrExistingAvatar, which already locks _hashLock
     _avatarHash.insert(sessionUUID, avatar);
     emit avatarAddedEvent(sessionUUID);
 
diff --git a/libraries/avatars/src/AvatarHashMap.h b/libraries/avatars/src/AvatarHashMap.h
index 6747025de0..ef6f7845eb 100644
--- a/libraries/avatars/src/AvatarHashMap.h
+++ b/libraries/avatars/src/AvatarHashMap.h
@@ -46,7 +46,7 @@ class AvatarHashMap : public QObject, public Dependency {
 public:
     AvatarHash getHashCopy() { QReadLocker lock(&_hashLock); return _avatarHash; }
     const AvatarHash getHashCopy() const { QReadLocker lock(&_hashLock); return _avatarHash; }
-    int size() { return _avatarHash.size(); }
+    int size() { QReadLocker lock(&_hashLock); return _avatarHash.size(); }
 
     // Currently, your own avatar will be included as the null avatar id.
     
@@ -152,8 +152,6 @@ protected:
     virtual void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason);
 
     AvatarHash _avatarHash;
-    // "Case-based safety": Most access to the _avatarHash is on the same thread. Write access is protected by a write-lock.
-    // If you read from a different thread, you must read-lock the _hashLock. (Scripted write access is not supported).
     mutable QReadWriteLock _hashLock;
 
 private: