SL-18721 Shutdown fixes #2

Set DONE if decode thread is down instead of waiting for an update.
Decodes can't be canceled, so fix potential situation where we get two
responses

3 files changed, 50 insertions, 21 deletions

diff --git a/indra/llimage/llimageworker.cpp b/indra/llimage/llimageworker.cpp
index c1ee052997..fd59daad3d 100644
--- a/indra/llimage/llimageworker.cpp
+++ b/indra/llimage/llimageworker.cpp
@@ -35,8 +35,10 @@ class ImageRequest
 {
 public:
 	ImageRequest(const LLPointer<LLImageFormatted>& image,
-				 S32 discard, BOOL needs_aux,
-				 const LLPointer<LLImageDecodeThread::Responder>& responder);
+                 S32 discard,
+                 BOOL needs_aux,
+                 const LLPointer<LLImageDecodeThread::Responder>& responder,
+                 U32 request_id);
 	virtual ~ImageRequest();
 
 	/*virtual*/ bool processRequest();
@@ -48,6 +50,7 @@ private:
 	// input
 	LLPointer<LLImageFormatted> mFormattedImage;
 	S32 mDiscardLevel;
+    U32 mRequestId;
 	BOOL mNeedsAux;
 	// output
 	LLPointer<LLImageRaw> mDecodedImageRaw;
@@ -62,6 +65,7 @@ private:
 
 // MAIN THREAD
 LLImageDecodeThread::LLImageDecodeThread(bool /*threaded*/)
+    : mDecodeCount(0)
 {
     mThreadPool.reset(new LL::ThreadPool("ImageDecode", 8));
     mThreadPool->start();
@@ -92,9 +96,10 @@ LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(
 {
     LL_PROFILE_ZONE_SCOPED_CATEGORY_TEXTURE;
 
+    U32 decode_id = ++mDecodeCount;
     // Instantiate the ImageRequest right in the lambda, why not?
     bool posted = mThreadPool->getQueue().post(
-        [req = ImageRequest(image, discard, needs_aux, responder)]
+        [req = ImageRequest(image, discard, needs_aux, responder, decode_id)]
         () mutable
         {
             auto done = req.processRequest();
@@ -103,13 +108,10 @@ LLImageDecodeThread::handle_t LLImageDecodeThread::decodeImage(
     if (! posted)
     {
         LL_DEBUGS() << "Tried to start decoding on shutdown" << LL_ENDL;
-        // should this return 0?
+        return 0;
     }
 
-    // It's important to our consumer (LLTextureFetchWorker) that we return a
-    // nonzero handle. It is NOT important that the nonzero handle be unique:
-    // nothing is ever done with it except to compare it to zero, or zero it.
-    return 17;
+    return decode_id;
 }
 
 void LLImageDecodeThread::shutdown()
@@ -123,15 +125,18 @@ LLImageDecodeThread::Responder::~Responder()
 
 //----------------------------------------------------------------------------
 
-ImageRequest::ImageRequest(const LLPointer<LLImageFormatted>& image, 
-							S32 discard, BOOL needs_aux,
-							const LLPointer<LLImageDecodeThread::Responder>& responder)
+ImageRequest::ImageRequest(const LLPointer<LLImageFormatted>& image,
+                           S32 discard,
+                           BOOL needs_aux,
+                           const LLPointer<LLImageDecodeThread::Responder>& responder,
+                           U32 request_id)
 	: mFormattedImage(image),
 	  mDiscardLevel(discard),
 	  mNeedsAux(needs_aux),
 	  mDecodedRaw(FALSE),
 	  mDecodedAux(FALSE),
-	  mResponder(responder)
+	  mResponder(responder),
+	  mRequestId(request_id)
 {
 }
 
@@ -199,7 +204,7 @@ void ImageRequest::finishRequest(bool completed)
 	if (mResponder.notNull())
 	{
 		bool success = completed && mDecodedRaw && (!mNeedsAux || mDecodedAux);
-		mResponder->completed(success, mDecodedImageRaw, mDecodedImageAux);
+		mResponder->completed(success, mDecodedImageRaw, mDecodedImageAux, mRequestId);
 	}
 	// Will automatically be deleted
 }
diff --git a/indra/llimage/llimageworker.h b/indra/llimage/llimageworker.h
index ca4c0d93d0..b4ab9432e6 100644
--- a/indra/llimage/llimageworker.h
+++ b/indra/llimage/llimageworker.h
@@ -39,7 +39,7 @@ public:
 	protected:
 		virtual ~Responder();
 	public:
-		virtual void completed(bool success, LLImageRaw* raw, LLImageRaw* aux) = 0;
+		virtual void completed(bool success, LLImageRaw* raw, LLImageRaw* aux, U32 request_id) = 0;
 	};
 
 public:
@@ -53,6 +53,7 @@ public:
 						 const LLPointer<Responder>& responder);
 	size_t getPending();
 	size_t update(F32 max_time_ms);
+    S32 getTotalDecodeCount() { return mDecodeCount; }
 	void shutdown();
 
 private:
@@ -60,6 +61,7 @@ private:
 	// LLQueuedThread - instead this is the API by which we submit work to the
 	// "ImageDecode" ThreadPool.
 	std::unique_ptr<LL::ThreadPool> mThreadPool;
+    LLAtomicU32 mDecodeCount;
 };
 
 #endif
diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp
index 38c9b3717d..40bbe2b934 100644
--- a/indra/newview/lltexturefetch.cpp
+++ b/indra/newview/lltexturefetch.cpp
@@ -348,13 +348,13 @@ private:
 		}
 
 		// Threads:  Tid
-		virtual void completed(bool success, LLImageRaw* raw, LLImageRaw* aux)
+		virtual void completed(bool success, LLImageRaw* raw, LLImageRaw* aux, U32 request_id)
 		{
             LL_PROFILE_ZONE_SCOPED;
 			LLTextureFetchWorker* worker = mFetcher->getWorker(mID);
 			if (worker)
 			{
- 				worker->callbackDecoded(success, raw, aux);
+ 				worker->callbackDecoded(success, raw, aux, request_id);
 			}
 		}
 	private:
@@ -398,7 +398,7 @@ public:
 	void callbackCacheWrite(bool success);
 
 	// Threads:  Tid
-	void callbackDecoded(bool success, LLImageRaw* raw, LLImageRaw* aux);
+	void callbackDecoded(bool success, LLImageRaw* raw, LLImageRaw* aux, S32 decode_id);
 	
 	// Threads:  T*
 	void setGetStatus(LLCore::HttpStatus status, const std::string& reason)
@@ -1800,8 +1800,22 @@ bool LLTextureFetchWorker::doWork(S32 param)
 		setState(DECODE_IMAGE_UPDATE);
 		LL_DEBUGS(LOG_TXT) << mID << ": Decoding. Bytes: " << mFormattedImage->getDataSize() << " Discard: " << discard
 						   << " All Data: " << mHaveAllData << LL_ENDL;
-		mDecodeHandle = LLAppViewer::getImageDecodeThread()->decodeImage(mFormattedImage, discard, mNeedsAux,
-																  new DecodeResponder(mFetcher, mID, this));
+
+        // In case worked manages to request decode, be shut down,
+        // then init and request decode again with first decode
+        // still in progress, assign a sufficiently unique id
+        mDecodeHandle = LLAppViewer::getImageDecodeThread()->decodeImage(mFormattedImage,
+                                                                       discard,
+                                                                       mNeedsAux,
+                                                                       new DecodeResponder(mFetcher, mID, this));
+        if (mDecodeHandle == 0)
+        {
+            // Abort, failed to put into queue.
+            // Happens if viewer is shutting down
+            setState(DONE);
+            LL_DEBUGS(LOG_TXT) << mID << " DECODE_IMAGE abort: failed to post for decoding" << LL_ENDL;
+            return true;
+        }
 		// fall though
 	}
 	
@@ -2305,16 +2319,24 @@ void LLTextureFetchWorker::callbackCacheWrite(bool success)
 //////////////////////////////////////////////////////////////////////////////
 
 // Threads:  Tid
-void LLTextureFetchWorker::callbackDecoded(bool success, LLImageRaw* raw, LLImageRaw* aux)
+void LLTextureFetchWorker::callbackDecoded(bool success, LLImageRaw* raw, LLImageRaw* aux, S32 decode_id)
 {
 	LLMutexLock lock(&mWorkMutex);										// +Mw
 	if (mDecodeHandle == 0)
 	{
 		return; // aborted, ignore
 	}
+    if (mDecodeHandle != decode_id)
+    {
+        // Queue doesn't support canceling old requests.
+        // This shouldn't normally happen, but in case it's possible that a worked
+        // will request decode, be aborted, reinited then start a new decode
+        LL_DEBUGS(LOG_TXT) << mID << " received obsolete decode's callback" << LL_ENDL;
+        return; // ignore
+    }
 	if (mState != DECODE_IMAGE_UPDATE)
 	{
-// 		LL_WARNS(LOG_TXT) << "Decode callback for " << mID << " with state = " << mState << LL_ENDL;
+		LL_DEBUGS(LOG_TXT) << "Decode callback for " << mID << " with state = " << mState << LL_ENDL;
 		mDecodeHandle = 0;
 		return;
 	}