C++ (CAS-like) synchronization on getter and setter of user defined types

Question

Suppose there is a user defined class MyClass that has a setter and a getter function.

class MyClass {
    int m_value;
    public:
        void set(int value)
        { m_value = value; }

        int get() const
        { return m_value;}
};

And there is a function increment() that can increase the value of the object by 1, which may be invoked in multiple threads.

void increment(MyClass& myClass)
{
    myClass.set(myClass.get() + 1);
}

What is the best way to make this function thread-safe? By just using a lock? Is there any way to achieve it by using some CAS-like operations?

It's unlikely you need atomics and they may even be slower, better to just protect the class' data with a mutex. — goji, Oct 08 '13 at 04:31

score 2 · Accepted Answer · edited May 23 '17 at 12:10

2

If you're using C++11, you could just use std::atomic<int> which provides atomic load, store and increment which seems to be everything you need.

I suspect you don't need atomics though as in most cases they will be slower than your basic mutex.

See this other question before you make your decision.

edited May 23 '17 at 12:10

Community

1
1

answered Oct 08 '13 at 04:35

goji

6,911
3
42
59

I don't thinke `std::atomic` will work because I am using a user defined type. Thanks for the advice and the link. – user571470 Oct 08 '13 at 04:50
So really your only solution is to protect it with a mutex then. Simplifies the decision ;) – goji Oct 08 '13 at 04:53
I also mean to use std::atomic internally in your MyClass. If that is what you meant by user defined type. – goji Oct 08 '13 at 04:54
Even though I use atomic internally, the `increment()` is not thread-safe because two threads can set the same value simultaneously. – user571470 Oct 08 '13 at 04:58
1

@user571470 they can't do that if the value is atomic. The increment operations get sequenced. What is unsafe is your `Increment` function. You should provide the means to increment the value without reading, then writing. – juanchopanza Oct 08 '13 at 05:15
Isn't that the whole point of using `std::atomic` or `std::mutex` inside the `increment()` member function? – goji Oct 08 '13 at 05:15
1

See http://coliru.stacked-crooked.com/a/e0aa11350d653bb2 Both of these are free of data races, but depending on how you use them, they may not be logically correct. Such is the difficulty in making a class thread safe. – goji Oct 08 '13 at 05:22
Yes, if it's logically correct, the function should be changed to void increment(MyClass& myClass) { LockGuard guard; myClass.set(myClass.get() + 1); } – user571470 Oct 08 '13 at 17:44

score -3 · Answer 2 · answered Oct 08 '13 at 04:47

-3

Use std::mutex and enjoy

    #include <mutex>

    class MyClass 
    {
        int m_value;
        std::mutex mtx;

        void set(int value) 
        {
            mtx.lock();
            m_value = value;
            mtx.unlock();
        }

        int get() const 
        {
            return m_value;
        }
    }

answered Oct 08 '13 at 04:47

Ronaldinho

125
1
9

This will not work. The `increment()` is still not thread-safe. – user571470 Oct 08 '13 at 04:54
increment() not need thread-safe because MyClass.set function is thread-safe. – Ronaldinho Oct 08 '13 at 04:56
but two threads can call set() with the same value, which is not the idea of incrementing the value twice. – user571470 Oct 08 '13 at 05:01

C++ (CAS-like) synchronization on getter and setter of user defined types

2 Answers2