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
master
Andrey Kleshchev 2024-01-23 02:23:28 +02:00 committed by Andrey Kleshchev
parent 4a34a11966
commit da48bd9439
3 changed files with 50 additions and 21 deletions

View File

@ -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
}

View File

@ -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

View File

@ -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;
}