3

For example consider

class ProcessList {
private
   std::vector<std::shared_ptr<SomeObject>> list;
   Mutex mutex;
public:
   void Add(std::shared_ptr<SomeObject> o) { 
       Locker locker(&mutex); // Start of critical section. Locker release so will the mutex - In house stuff
       list.push_back(std::make_shared<SomeObject>(o).
   }

   void Remove(std::shared_ptr<SomeObject> o) { 
       Locker locker(&mutex); // Start of critical section. Locker release so will the mutex - In house stuff
       // Code to remove said object but indirectly modifying the reference count in copy below
   }

   void Process() {
       std::vector<std::shared_ptr<SomeObject>> copy;

       {
           Locker locker(&mutes);
           copy = std::vector<std::shared_ptr<SomeObject>>(
                list.begin(), list.end()
           )
       }
       for (auto it = copy.begin(), it != copy.end(); ++it) {
           it->Procss(); // This may take time/add/remove to the list
       }
    }
};

One thread runs Process. Multiple threads run add/remove.

Will the reference count be safe and always correct - or should a mutex be placed around that?

Ed Heal
  • 59,252
  • 17
  • 87
  • 127
  • Reference counting can be implemented thread-safely. Considering that you have C++11 which also has threads, I'd assume that refcounting is also implemented in a thread-safe way. BTW: Even the original `shared_ptr` from Boost was already thread-safe. – Ulrich Eckhardt Mar 14 '15 at 11:22
  • Does each reference count have a mutex? – Ed Heal Mar 14 '15 at 11:26
  • 1
    No, they use atomic operations. You could implement them using a mutex for every counter, but that would be inefficient. You could also implement them using a single global mutex, which would be simpler, but probably still less efficient than using atomics. – Ulrich Eckhardt Mar 14 '15 at 11:38
  • 1
    @UlrichEckhardt does the standard mandate that this is the case (that `shared_ptr` is thread safe)? If so could you give a reference? Thanks. – davmac Mar 14 '15 at 12:49
  • On https://duckduckgo.com/?q=C%2B%2B+shared_ptr+thread+safe this is answered by the first link. – Ulrich Eckhardt Mar 14 '15 at 12:56
  • possible duplicate of [std::shared\_ptr thread safety](http://stackoverflow.com/questions/14482830/stdshared-ptr-thread-safety) – davmac Mar 14 '15 at 13:03
  • [Another near-dupe](http://stackoverflow.com/q/28113769/179910). – Jerry Coffin Mar 14 '15 at 16:00

3 Answers3

1

Yes, the standard (at §20.8.2.2, at least as of N3997) that's intended to require that the reference counting be thread-safe.

For your simple cases like Add:

void Add(std::shared_ptr<SomeObject> o) { 
    Locker locker(&mutex);
    list.push_back(std::make_shared<SomeObject>(o).
}

...the guarantees in the standard are strong enough that you shouldn't need the mutex, so you can just have:

void Add(std::shared_ptr<SomeObject> o) { 
    list.push_back(std::make_shared<SomeObject>(o).
}

For some of your operations, it's not at all clear that thread-safe reference counting necessarily obviates your mutex though. For example, inside of your Process, you have:

   {
       Locker locker(&mutes);
       copy = std::vector<std::shared_ptr<SomeObject>>(
            list.begin(), list.end()
       )
   }

This carries out the entire copy as an atomic operation--nothing else can modify the list during the copy. This assures that your copy gives you a snapshot of the list precisely as it was when the copy was started. If you eliminate the mutex, the reference counting will still work, but your copy might reflect changes made while the copy is being made.

In other words, the thread safety of the shared_ptr only assures that each individual increment or decrement is atomic--it doesn't assure that manipulations of the entire list are atomic as the mutex does in this case.

Since your list is actually a vector, you should be able to simplify the copying code a bit to just copy = list.

Also note that your Locker seems to be a subset of what std::lock_guard provides. It appears you could use:

std::lock_guard<std::mutex> locker(&mutes);

...in its place quite easily.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • But is the vector thread safe as well? I think that it is not so there is an interaction between the two. Anyway our locker does a little extra in terms of logging. – Ed Heal Mar 14 '15 at 18:04
  • @EdHeal: There's nothing to prohibit a vector implementation from providing thread safety, but it's not required either. If multiple threads modify the vector, then yes, you need to use a mutex (or whatever) to serialize that access. – Jerry Coffin Mar 14 '15 at 18:24
  • It is a vector of shared pointers. - so the items are safe but the vector is not - is that correct – Ed Heal Mar 14 '15 at 21:03
  • @EdHeal: that's the idea, yes. – Jerry Coffin Mar 14 '15 at 21:07
0

It will be an overhead to have a mutex for reference counting.

Internally, mutexes use atomic operations, basically a mutex does internal thread safe reference counting. So you can just use atomics for your reference counting directly instead of using a mutex and essentially doing double the work.

dtech
  • 47,916
  • 17
  • 112
  • 190
-2

Unless your CPU architecture have an atomic increment/decrement and you used that for reference count, then, no, it's not safe; C++ makes no guarantee on the thread safety of x++/x-- operation on any of its standard types.

Use atomic<int> if your compiler supports them (C++11), otherwise you'll need to have the lock.

Further references:

Lie Ryan
  • 62,238
  • 13
  • 100
  • 144
  • 2
    The question is whether `std::shared_ptr` (which is what is being used in the code provided by OP) is threadsafe. There is no "x++/x--" operation here (unless it's in the shared_ptr implementation). – davmac Mar 14 '15 at 12:48