From 11a9332fa7bd830d8e44f15a91e008f3a6f9c19d Mon Sep 17 00:00:00 2001
From: Andrey Kleshchev <andreykproductengine@lindenlab.com>
Date: Mon, 25 May 2020 23:59:33 +0300
Subject: [PATCH] SL-12889 Failed to cache image crashes
Scaled down avatar or group image was getting into cache.
---
indra/newview/lltexturecache.cpp | 7 +++++--
indra/newview/lltexturefetch.cpp | 5 +++++
indra/newview/lltexturefetch.h | 1 +
indra/newview/llviewertexture.cpp | 9 +++++++--
4 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/indra/newview/lltexturecache.cpp b/indra/newview/lltexturecache.cpp
index 2e52414d719..6211d0ce3b4 100644
--- a/indra/newview/lltexturecache.cpp
+++ b/indra/newview/lltexturecache.cpp
@@ -616,6 +616,9 @@ bool LLTextureCacheRemoteWorker::doWrite()
if(idx >= 0)
{
// write to the fast cache.
+ // mRawImage is not entirely safe here since it is a pointer to one owned by cache worker,
+ // it could have been retrieved via getRequestFinished() and then modified.
+ // If writeToFastCache crashes, something is wrong around fetch worker.
if(!mCache->writeToFastCache(mID, idx, mRawImage, mRawDiscardLevel))
{
LL_WARNS() << "writeToFastCache failed" << LL_ENDL;
@@ -2155,8 +2158,8 @@ bool LLTextureCache::writeToFastCache(LLUUID image_id, S32 id, LLPointer<LLImage
h >>= i;
if(w * h *c > 0) //valid
{
- //make a duplicate to keep the original raw image untouched.
-
+ // Make a duplicate to keep the original raw image untouched.
+ // Might be good idea to do a copy during writeToCache() call instead of here
try
{
#if LL_WINDOWS
diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp
index fe058024f69..f64db7beb54 100644
--- a/indra/newview/lltexturefetch.cpp
+++ b/indra/newview/lltexturefetch.cpp
@@ -1977,6 +1977,11 @@ bool LLTextureFetchWorker::doWork(S32 param)
setState(WAIT_ON_WRITE);
++mCacheWriteCount;
CacheWriteResponder* responder = new CacheWriteResponder(mFetcher, mID);
+ // This call might be under work mutex, but mRawImage is not nessesary safe here.
+ // If something retrieves it via getRequestFinished() and modifies, image won't
+ // be protected by work mutex and won't be safe to use here nor in cache worker.
+ // So make sure users of getRequestFinished() does not attempt to modify image while
+ // fetcher is working
mCacheWriteHandle = mFetcher->mTextureCache->writeToCache(mID, cache_priority,
mFormattedImage->getData(), datasize,
mFileSize, mRawImage, mDecodedDiscard, responder);
diff --git a/indra/newview/lltexturefetch.h b/indra/newview/lltexturefetch.h
index cdf88685970..2aa194e1418 100644
--- a/indra/newview/lltexturefetch.h
+++ b/indra/newview/lltexturefetch.h
@@ -92,6 +92,7 @@ class LLTextureFetch : public LLWorkerThread
void deleteAllRequests();
// Threads: T*
+ // keep in mind that if fetcher isn't done, it still might need original raw image
bool getRequestFinished(const LLUUID& id, S32& discard_level,
LLPointer<LLImageRaw>& raw, LLPointer<LLImageRaw>& aux,
LLCore::HttpStatus& last_http_get_status);
diff --git a/indra/newview/llviewertexture.cpp b/indra/newview/llviewertexture.cpp
index a2cec9a6133..bd83a61e5b9 100644
--- a/indra/newview/llviewertexture.cpp
+++ b/indra/newview/llviewertexture.cpp
@@ -1238,6 +1238,8 @@ void LLViewerFetchedTexture::loadFromFastCache()
{
if (mBoostLevel == LLGLTexture::BOOST_ICON)
{
+ // Shouldn't do anything usefull since texures in fast cache are 16x16,
+ // it is here in case fast cache changes.
S32 expected_width = mKnownDrawWidth > 0 ? mKnownDrawWidth : DEFAULT_ICON_DIMENTIONS;
S32 expected_height = mKnownDrawHeight > 0 ? mKnownDrawHeight : DEFAULT_ICON_DIMENTIONS;
if (mRawImage && (mRawImage->getWidth() > expected_width || mRawImage->getHeight() > expected_height))
@@ -1485,7 +1487,8 @@ BOOL LLViewerFetchedTexture::createTexture(S32 usename/*= 0*/)
mOrigWidth = mRawImage->getWidth();
mOrigHeight = mRawImage->getHeight();
-
+ // This is only safe because it's a local image and fetcher doesn't use raw data
+ // from local images, but this might become unsafe in case of changes to fetcher
if (mBoostLevel == BOOST_PREVIEW)
{
mRawImage->biasedScaleToPowerOfTwo(1024);
@@ -1989,6 +1992,7 @@ bool LLViewerFetchedTexture::updateFetch()
if (mRawImage.notNull()) sRawCount--;
if (mAuxRawImage.notNull()) sAuxCount--;
+ // keep in mind that fetcher still might need raw image, don't modify original
bool finished = LLAppViewer::getTextureFetch()->getRequestFinished(getID(), fetch_discard, mRawImage, mAuxRawImage,
mLastHttpGetStatus);
if (mRawImage.notNull()) sRawCount++;
@@ -2048,7 +2052,8 @@ bool LLViewerFetchedTexture::updateFetch()
if (mRawImage && (mRawImage->getWidth() > expected_width || mRawImage->getHeight() > expected_height))
{
// scale oversized icon, no need to give more work to gl
- mRawImage->scale(expected_width, expected_height);
+ // since we got mRawImage from thread worker and image may be in use (ex: writing cache), make a copy
+ mRawImage = mRawImage->scaled(expected_width, expected_height);
}
}
--
GitLab