From e2df9e29e20a5944d85817b14180fb137833749f Mon Sep 17 00:00:00 2001
From: "Anthony J. Thibault" <tony@highfidelity.io>
Date: Fri, 15 Dec 2017 16:46:27 -0800
Subject: [PATCH] Fix for crash in AnimSkeleton::getNumJoints()

When initAnimGraph is called it asynchronously loads the Animation graph in the background.
If the model url is changed, or the Model is deleted in between the initial load and it's completion,
It's possible to access a bad Rig::_animSkeleton pointer.
The fix is to hold onto the _animSkeleton pointer via a weak ref.
---
 libraries/animation/src/Rig.cpp | 13 +++++++++++--
 libraries/animation/src/Rig.h   |  2 +-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp
index 78aa1f4ba8..44745c5c2d 100644
--- a/libraries/animation/src/Rig.cpp
+++ b/libraries/animation/src/Rig.cpp
@@ -1641,9 +1641,17 @@ void Rig::initAnimGraph(const QUrl& url) {
         // load the anim graph
         _animLoader.reset(new AnimNodeLoader(url));
         _animLoading = true;
-        connect(_animLoader.get(), &AnimNodeLoader::success, [this](AnimNode::Pointer nodeIn) {
+        std::weak_ptr<AnimSkeleton> weakSkeletonPtr = _animSkeleton;
+        connect(_animLoader.get(), &AnimNodeLoader::success, [this, weakSkeletonPtr](AnimNode::Pointer nodeIn) {
             _animNode = nodeIn;
-            _animNode->setSkeleton(_animSkeleton);
+
+            // abort load if the previous skeleton was deleted.
+            auto sharedSkeletonPtr = weakSkeletonPtr.lock();
+            if (!sharedSkeletonPtr) {
+                return;
+            }
+
+            _animNode->setSkeleton(sharedSkeletonPtr);
 
             if (_userAnimState.clipNodeEnum != UserAnimState::None) {
                 // restore the user animation we had before reset.
@@ -1651,6 +1659,7 @@ void Rig::initAnimGraph(const QUrl& url) {
                 _userAnimState = { UserAnimState::None, "", 30.0f, false, 0.0f, 0.0f };
                 overrideAnimation(origState.url, origState.fps, origState.loop, origState.firstFrame, origState.lastFrame);
             }
+
             // restore the role animations we had before reset.
             for (auto& roleAnimState : _roleAnimStates) {
                 auto roleState = roleAnimState.second;
diff --git a/libraries/animation/src/Rig.h b/libraries/animation/src/Rig.h
index e738ad1c19..1ec4d9527f 100644
--- a/libraries/animation/src/Rig.h
+++ b/libraries/animation/src/Rig.h
@@ -31,7 +31,7 @@ class AnimInverseKinematics;
 // Rig instances are reentrant.
 // However only specific methods thread-safe.  Noted below.
 
-class Rig : public QObject, public std::enable_shared_from_this<Rig> {
+class Rig : public QObject {
     Q_OBJECT
 public:
     struct StateHandler {