#3133 add handler to prevent crash when preview gets closed before getting the callback

2 files changed, 31 insertions, 16 deletions

diff --git a/indra/newview/llmodelpreview.cpp b/indra/newview/llmodelpreview.cpp
index d61333c2cc..29ab4e38c0 100644
--- a/indra/newview/llmodelpreview.cpp
+++ b/indra/newview/llmodelpreview.cpp
@@ -555,7 +555,7 @@ void LLModelPreview::rebuildUploadData()
                         {
                             // in case user provided a missing file later
                             texture->setIsMissingAsset(false);
-                            texture->setLoadedCallback(LLModelPreview::textureLoadedCallback, 0, true, false, this, &mCallbackTextureList, false);
+                            texture->setLoadedCallback(LLModelPreview::textureLoadedCallback, 0, true, false, new LLHandle<LLModelPreview>(getHandle()), &mCallbackTextureList, false);
                             texture->forceToSaveRawImage(0, F32_MAX);
                             texture->updateFetch();
                             if (mModelLoader)
@@ -784,6 +784,10 @@ void LLModelPreview::loadModel(std::string filename, S32 lod, bool force_disable
     std::map<std::string, std::string> joint_alias_map;
     getJointAliases(joint_alias_map);
 
+    LLHandle<LLModelPreview> preview_handle = getHandle();
+    auto load_textures_cb =
+            [preview_handle](LLImportMaterial& material, void* opaque) { return LLModelPreview::loadTextures(material, preview_handle); };
+
     // three possible file extensions, .dae .gltf .glb
     // check for .dae and if not then assume one of the .gl??
     std::string filename_lc(filename);
@@ -795,7 +799,7 @@ void LLModelPreview::loadModel(std::string filename, S32 lod, bool force_disable
             lod,
             &LLModelPreview::loadedCallback,
             &LLModelPreview::lookupJointByName,
-            &LLModelPreview::loadTextures,
+            load_textures_cb,
             &LLModelPreview::stateChangedCallback,
             this,
             mJointTransformMap,
@@ -812,7 +816,7 @@ void LLModelPreview::loadModel(std::string filename, S32 lod, bool force_disable
             lod,
             &LLModelPreview::loadedCallback,
             &LLModelPreview::lookupJointByName,
-            &LLModelPreview::loadTextures,
+            load_textures_cb,
             &LLModelPreview::stateChangedCallback,
             this,
             mJointTransformMap,
@@ -3130,9 +3134,9 @@ LLJoint* LLModelPreview::lookupJointByName(const std::string& str, void* opaque)
     return NULL;
 }
 
-U32 LLModelPreview::loadTextures(LLImportMaterial& material, void* opaque)
+U32 LLModelPreview::loadTextures(LLImportMaterial& material, LLHandle<LLModelPreview> handle)
 {
-    if (material.mDiffuseMapFilename.size())
+    if (material.mDiffuseMapFilename.size() && !handle.isDead())
     {
         material.mOpaqueData = new LLPointer< LLViewerFetchedTexture >;
         LLPointer< LLViewerFetchedTexture >& tex = (*reinterpret_cast< LLPointer< LLViewerFetchedTexture > * >(material.mOpaqueData));
@@ -3143,10 +3147,8 @@ U32 LLModelPreview::loadTextures(LLImportMaterial& material, void* opaque)
             // file was loaded previosly, reload image to get potential changes
             tex->clearFetchedResults();
         }
-        // Todo: might cause a crash if preview gets closed before we get the callback.
-        // Use a callback list or guard callback in some way
-        LLModelPreview* preview = (LLModelPreview*)opaque;
-        tex->setLoadedCallback(LLModelPreview::textureLoadedCallback, 0, true, false, opaque, &preview->mCallbackTextureList, false);
+        LLModelPreview* preview = (LLModelPreview*)handle.get();
+        tex->setLoadedCallback(LLModelPreview::textureLoadedCallback, 0, true, false, new LLHandle<LLModelPreview>(handle), &preview->mCallbackTextureList, false);
         tex->forceToSaveRawImage(0, F32_MAX);
         material.setDiffuseMap(tex->getID()); // record tex ID
         return 1;
@@ -4003,16 +4005,29 @@ void LLModelPreview::textureLoadedCallback(
     bool final,
     void* userdata)
 {
-    LLModelPreview* preview = (LLModelPreview*)userdata;
-    preview->refresh();
+    if (!userdata)
+        return;
+
+    LLHandle<LLModelPreview>* handle = (LLHandle<LLModelPreview>*)userdata;
 
-    if (final && preview->mModelLoader)
+    if (!handle->isDead())
     {
-        if (preview->mModelLoader->mNumOfFetchingTextures > 0)
+        LLModelPreview* preview = static_cast<LLModelPreview*>(handle->get());
+        preview->refresh();
+
+        if (final && preview->mModelLoader)
         {
-            preview->mModelLoader->mNumOfFetchingTextures--;
+            if (preview->mModelLoader->mNumOfFetchingTextures > 0)
+            {
+                preview->mModelLoader->mNumOfFetchingTextures--;
+            }
         }
     }
+
+    if (final || !success)
+    {
+        delete handle;
+    }
 }
 
 // static
diff --git a/indra/newview/llmodelpreview.h b/indra/newview/llmodelpreview.h
index e236d7ced7..0873263587 100644
--- a/indra/newview/llmodelpreview.h
+++ b/indra/newview/llmodelpreview.h
@@ -111,7 +111,7 @@ static const std::string lod_label_name[NUM_LOD + 1] =
     "I went off the end of the lod_label_name array.  Me so smart."
 };
 
-class LLModelPreview : public LLViewerDynamicTexture, public LLMutex
+class LLModelPreview : public LLViewerDynamicTexture, public LLMutex, public LLHandleProvider<LLModelPreview>
 {
     LOG_CLASS(LLModelPreview);
 
@@ -211,7 +211,7 @@ protected:
     static void         stateChangedCallback(U32 state, void* opaque);
 
     static LLJoint* lookupJointByName(const std::string&, void* opaque);
-    static U32          loadTextures(LLImportMaterial& material, void* opaque);
+    static U32          loadTextures(LLImportMaterial& material, LLHandle<LLModelPreview> handle);
 
     void lookupLODModelFiles(S32 lod);