CODEWITHSUNDEEP
multithreadingc++11thread-safetyatomic
Vincent Van Groot
Vincent Van Groot

Reputation: 33

Accessing an atomic member of a class held by a shared_ptr

I'm trying to create a small class that will allow me to facilitate a communication between two threads.

Those threads most probably will outlive the context in which the above mentioned class was created as they are queued onto a thread pool.

What I have tried so far (on coliru as well): 

class A    
{
public:
    A(int maxVal) : maxValue(maxVal) {}    
    bool IsOverMax() const { return cur >= maxValue; }    
    void Increase() { cur++; }

private:
    const int  maxValue;    
    atomic_int cur{ 0 };
};

possible usage: 

void checking(const shared_ptr<A> counter)
{
    while(!counter->IsOverMax())
    {
        cout<<"Working\n";         // do work
        std::this_thread::sleep_for(10ms);
    }
}

void counting(shared_ptr<A> counter)
{
    while (!counter->IsOverMax())
    {
        cout<<"Counting\n";
        counter->Increase(); // does this fall under `...uses a non-const member function of shared_ptr then a data race will occur`?  http://en.cppreference.com/w/cpp/memory/shared_ptr/atomic
        std::this_thread::sleep_for(9ms);
    }
}

int main() 
{ 

    unique_ptr<thread> t1Ptr;
    unique_ptr<thread> t2Ptr;

    {
        auto aPtr = make_shared<A>(100); // This might be out of scope before t1 and t2 end
        t1Ptr.reset(new thread(checking, aPtr)); // To simbolize that t1,t2 will outlive the scope in which aPtr was originaly created
        t2Ptr.reset(new thread(counting, aPtr));
    }

    t2Ptr->join();
    t1Ptr->join();
    //cout<< aPtr->IsOverMax();
}

The reason I'm concerned is that the documentation says that:

If multiple threads of execution access the same std::shared_ptr object without synchronization and any of those accesses uses a non-const member function of shared_ptr then a data race will occur unless all such access is performed through these functions, which are overloads of the corresponding atomic access functions (std::atomic_load, std::atomic_store, etc.)

Upvotes: 3

Views: 761

Answers (2)

LWimsey
LWimsey

Reputation: 6707

So Increase is a non const function, are the copies of aPtr are the same std::shared_ptr for this context or not ?

At std::thread creation, aPtr is passed by value. Therefore, it is guaranteed that:

  • You don't introduce a data race since each thread gets its own instance of shared_ptr (although they manage the same object A).
    The documentation you are referring to describes a scenario whereby multiple threads operate on the same shared_ptr instance. In that case, only const member functions can be called (see below), or synchronization is required.
  • shared_ptr reference-count is incremented before aPtr goes out of scope in main

So yes, this is a correct way to use shared_ptr.

Is this code thread-safe?

Your code does not introduce a data race, neither with access to shared_ptr instances, nor with access to the managed object A. This means that there are no conflicting, non-atomic, read and write operations to the same memory location performed by multiple threads.

However, keep in mind that, in checking(), the call to IsOverMax() is separated from the actual work that follows (Increase() could be called by the second thread after IsOverMax() but before 'do work'). Therefore, you could 'do work' while cur has gone over its maximum. Whether or not that is a problem depends on your specification, but it is called a race condition which is not necessarily a programming error (unlike a data race which causes undefined behavior).

Would this be OK for a non atomic object (say using an std::mutex to lock around reads and writes to a regular int)?

cur can be a regular int (non-atomic) if you protect it with a std::mutex. The mutex must be locked for both write and read access in order to prevent a data race.

One remark on calling const member functions on objects shared by multiple threads.
The use of const alone does not guarantee that no data race is introduced.
In this case, the guarantee applies to shared_ptr const member functions, because the documentation says so.
I cannot find in the C++ standard whether that guarantee applies to all const member functions in the Standard Library

Upvotes: 2

Nevin
Nevin

Reputation: 4863

That documentation is talking about the member functions of shared_ptr, not the member functions of your class. Copies of shared_ptr objects are different objects.

I believe the code is thread safe, because the only changing variable written and read on different threads is cur, and that variable is atomic.

If cur was not atomic and access to it in both Increase() and IsOverMax() was protected by locking a std::mutex, that code would also be thread safe.

Upvotes: 0

Related Questions