0

I have a class as follows. I want to hold 2 member variable as a atomic. One will hold the current value and the other the maximum value. If the current number is greater than the maximum number, the maximum number will be updated. Is function thread safe? Is there any problem like infinite loop or etc.? I wanted to ask because I'm not quite sure. If you have a better solution, can you suggest it? Thank you.

`

#include <atomic>

class Myclass {
public:
    void inc() {
        std::size_t cur_count = (++_curCount);
        std::size_t max_count = _maxCount.load();
        if(cur_count > max_count) {
            while(!_maxCount.compare_exchange_strong(max_count, cur_count) && cur_count >max_count)
                ;
        }
    }

private:
    std::atomic<std::size_t> _maxCount{0};
    std::atomic<std::size_t> _curCount{0};
};

`

I just want to be sure the code is thread-safe.

Enes Aygün
  • 31
  • 4
  • 1
    You a CAS retry loop to keep attempting to swap in your fetch_add result, while it's greater than the the expected value you've seen, is a way to implement atomic_fetch_max, if that existed. You can synthesize any atomic RMW operation on one variable using a CAS retry loop. You don't really need the if, just put the `cur_count >max_count` first in the `while`. Or use a `do{ if()break }while(!cas())` to make it more human-readable. – Peter Cordes Nov 14 '22 at 05:19
  • 1
    Does `max_count` ever decrease? – Nicol Bolas Nov 14 '22 at 05:34
  • Trying to do lockless programming with atomics is a daunting challenge that typically results in weird race conditions that are impossible to debug - especially when there is more than one variable involved. Just use a mutex. – selbie Nov 14 '22 at 06:01
  • @PeterCordes Thank you. You are right. Short circuit behaviour may help to me. I am editing my code as follows. while(curCount > maxCount && !_maxPoolElementCount.compare_exchange_strong(maxCount, curCount)){} – Enes Aygün Nov 14 '22 at 06:47
  • If `max_count` is supposed to decrease while an `atomic_max` operation on another thread is in progress (retrying CAS), that could increase it back up to match the `++_curCount` result it got! If you need `_curCount` and `_maxCount` to work as an atomic pair, you need to keep them together in one `atomic< struct foo >` or something. (IIRC, `std::pair<>` has some member functions that make it not compatible with `std::atomic`.) See also [How can I implement ABA counter with c++11 CAS?](https://stackoverflow.com/q/38984153) re: efficient access to a struct of two size_t/pointer objects – Peter Cordes Nov 14 '22 at 06:51
  • So decreasing `_maxCount` is only irrelevant if you never do it in parallel with other threads doing increments, like you're sure that any incrementers have finished applying or trying to apply their increases to the max. Or if it's ok that they increase it again. – Peter Cordes Nov 14 '22 at 06:53
  • @PeterCordes I'm just decreasing _curCount. – Enes Aygün Nov 14 '22 at 06:59
  • @PeterCordes _maxCount never decreases. – Enes Aygün Nov 14 '22 at 07:02
  • @EnesAygün: Nicol Bolas was asking about `_maxCount`, so you should delete that comment where you said "yes"! That's why I replied to explain why that was a potential showstopper. I'd thought about that some while writing my first comment, but decided to assume that max was truly a max (like apparently is actually the case), and/or thought for a while it might not be as messy as it actually would be if max needed to decrease. – Peter Cordes Nov 14 '22 at 07:05
  • @PeterCordes sorry, I misunderstood. I deleted comment. So I understand that there is no problem in the code right now. Is that true? – Enes Aygün Nov 14 '22 at 08:16
  • I think it's correct. For efficiency, you want `compare_exchange_weak` since it's in a retry loop anyway. And like I said, you can restructure to a do{}while or whatever other way to only write the skip condition once. The first load (before the loop) can be `std::memory_order_relaxed`, and so can everything else depending on what if anything is using this to sync-with and create a happens-before between two threads. The final count, and the max, will be correct after the dust settles from multiple threads running it. – Peter Cordes Nov 14 '22 at 08:44

0 Answers0