jpo38
jpo38

Reputation: 21514

Is there a sort fo weak mutex concept?

Let's consider this piece of code (does not necssarily make sense, it's just a MCVE):

class Foo
{
public:
    // this function is thread safe
    void doSomething();
};

static std::mutex mutex;
static std::shared_ptr<Foo> instance;

void workerThread()
{
    while ( true )
    {
        mutex.lock();
        if ( instance )
        {
            instance->doSomething();
        }
        mutex.unlock();
        msleep( 50 );
    }
}

void messupThread()
{
    while ( true )
    {
        mutex.lock();
        instance.reset( new Foo() );
        mutex.unlock();
        msleep( 1000 );
    }
}

A workerThread do operations on the Foo instance. A messupThread modifies the instance on which operations should be done.

This code is thread safe.

Problem comes if you start 10 workerThread, when one workerThread uses the instance, it will lock the other nine one (while doSomething is thread safe, they should be able to work concurrently).

Is there a kind of weak mutex/lock mechanism that would make any workerThread prevent messupThread from changing the instance but would not prevent a concurrent workerThread to use it.

I could do this, but I'm wondering if there is an easier way to implement that using standard objects/mechanisms:

class Foo
{
public:
    // this function is thread safe
    void doSomething();
};

static int useRefCount = 0;
static std::mutex work_mutex; // to protect useRefCount concurrent access
static std::mutex mutex;      // to protect instance concurrent access
static std::shared_ptr<Foo> instance;

void workerThread()
{
    while ( true )
    {
        work_mutex.lock();
        if ( useRefCount == 0 )
            mutex.lock(); // first one to start working, prevent messupThread() to change instance
        // else, another workerThread already locked the mutex
        useRefCount++;   
        work_mutex.unlock();           

        if ( instance )
        {
            instance->doSomething();
        }

        work_mutex.lock();
        useRefCount--;
        if ( useRefCount == 0 )
            mutex.unlock(); // no more workerThread working
        //else, keep mutex locked as another workerThread is still working
        work_mutex.unlock();

        msleep( 50 );
    }
}

void messupThread()
{
    while ( true )
    {
        mutex.lock();
        instance.reset( new Foo() );
        mutex.unlock();
        msleep( 1000 );
    }
}

Upvotes: 1

Views: 417

Answers (2)

Maarten Bamelis
Maarten Bamelis

Reputation: 2423

If you have access to std::shared_mutex, it might solve your problem nicely. It is basically a read/write lock but with different nomenclature: shared access vs. unique access.

Let the worker threads take a shared lock, and let the messup thread take a unique lock.

Whether shared access or unique access is preferred, depends on the implementation. So you might see your messup thread starved because of the high number of worker threads taking a shared lock very frequently.

// Worker thread:
std::shared_lock<std::shared_mutex> lock(m_mutex);

// Messup thread:
std::unique_lock<std::shared_mutex> lock(m_mutex);

Note that std::shared_mutex is part of the C++17 standard.

Upvotes: 5

Some programmer dude
Some programmer dude

Reputation: 409356

If all threads should be sharing the same single smart-pointer instance, it doesn't make much sense to use std::shared_ptr to begin with, but rather std::unique_ptr.

May I instead suggest that each thread have its own std::shared_ptr, copied from the global instance shared pointer? Then, protected by a lock, you copy from the global shared pointer to the local shared pointer, and when the lock is unlocked you call the doSomething function through the local shared pointer.

Perhaps something like

void workerThread()
{
    std::shared_ptr<Foo> local_instance;

    while (true)
    {
        {
            std::scoped_lock lock(mutex);
            local_instance = instance;
        }

        if (local_instance)
        {
            local_instance->doSomething();
        }

        msleep(50);
    }
}

Upvotes: 1

Related Questions