C++ multi-threaded texture loader work item removal from main thread

Question

I have a texture loading thread, which receives requests to load textures from a main thread via a concurrent queue.

The texture loader request is a simple struct with a raw pointer to the object that will receive the texture:

struct TextureLoaderRequest
{
    std::string mFilename;
    ContentViewer *mContentViewer;
};

The actual texture object contained within the ContentViewer is protected by a mutex and some atomic booleans (also contained within ContentViewer):

std::atomic<bool>               mIsLoaded;
std::atomic<bool>               mIsVisible;
std::mutex                      mImageMutex;

Then the texture access routines are as follows:

void ContentViewer::setTexture(ci::gl::TextureRef texture)
{
    std::lock_guard<std::mutex> guard(mImageMutex);
    mImage = texture;
}

ci::gl::TextureRef ContentViewer::getTexture()
{
    std::lock_guard<std::mutex> guard(mImageMutex);
    if (mIsVisible)
    {
        if (mImage != nullptr)
        {
            mIsLoaded = true;
            return mImage;
        }
        mIsLoaded = false;
    }
    return nullptr;
}

The texture loader may receive many texture load requests from the main thread at a single time, and then works through the queue loading and assigning textures to the content viewer pointed to in the texture load request message.

The problem I am having is that when the main thread "deletes" the content viewers, the texture loading thread may have an outstanding request in its queue, and by the time it gets to processing it, the content viewer has been deleted and the program crashes.

I'm not sure how to go about removing that outstanding texture load request sitting in the texture thread work queue. I cannot allow the main thread to wait for the relevant texture to be loaded for the content viewer, but then, what would the best practice strategy for achieving this be?

Thanks - Laythe

Answer 1

Ok, so there are two options imho, either:

1. You wait for all the requests for a specific object to be finished before deleting it.
2. You check if the object is still there before you execute any operation that was scheduled.

I don't have sufficient insight on your application: how the queue is implemented and why and when requests are scheduled, so I cannot give feedback on that.

Answer 2

I found that I needed to construct a cancellation list out of a std::mutex protected vector. When the main thread wants to quit it simply adds an entry into the vector and continues on. The texture loader thread has the additional burden of checking the list for every received texture request, but operation is not on the critical path.

I'd still be interested in alternatives/suggestions.

A little outline of the thread is below:

void textureLoaderThreadFn()
{
    log("texture loader thread started");

    while (!mShouldQuit)
    {
        // Wait for texture loader request
        TextureLoaderRequest *textureLoaderRequest = nullptr;
        mTextureRequests->popBack(&textureLoaderRequest);

        // it is possible popBack didnt modify textureLoaderRequest (eg. when cancelled on exit)
        if (textureLoaderRequest != nullptr)
        {
            std::lock_guard<std::mutex> lk(mCancellationListMutex);

            if (std::find(mCancellationList.begin(), mCancellationList.end(), textureLoaderRequest->mFilename) != mCancellationList.end())
            {
                // Cancelled

                // we must reset the isLoading that was set by the main thread,
                // so that the request to load the texture can get put back if need be
                textureLoaderRequest->mContentViewer->mIsLoading = false;

                // remove from cancellation list
                mCancellationList.erase(std::remove(mCancellationList.begin(), mCancellationList.end(), textureLoaderRequest->mFilename), mCancellationList.end());
            }
            else
            {
                // Not cancelled
                <SERVICE TEXTURE REQUEST>
            }

            // dont need this anymore
            delete textureLoaderRequest;
        }
    }
    log("texture loader thread stopped");

    // Empty the queue    
    int count = 0;
    TextureLoaderRequest *textureLoaderRequest = nullptr;
    while (mTextureRequests->tryPopBack(&textureLoaderRequest))
    {
        if (textureLoaderRequest != nullptr)
            delete textureLoaderRequest;
        count++;
    }
    log("texture loader thread purged " + std::to_string(count) + " outstanding texture load requests");
}

Answer 3

I would suggest you add a status flag to the the content viewer to distinguish 3 statuses; scheduled for coloration, being colored and not scheduled for coloration. The main thread should delete a content viewer only when its status is scheduled for coloration or not scheduled for coloration.

The texture worker thread changes the status to being colored and once it is colored it put it to colored.

The change of the statuses and the check of the status if it can be deleted should always be scoped by the same mutex; you might put the flag as private in content viewer and use two public methods 1)void change_status(status) and 2)bool can_delete().

Both functions should start with acquiring same mutex. the 1) to be used in different transitions in main thread and in texture worker thread and the 2) is used by the main thread before deleting the content viewer to return true only if status is not being colored.

In the texture work thread, before quitting, you might delete the last colored content in the case it was not deleted by main thread (as it might had been on status being colored).