2

I'm thread-analyzing using ThreadSanitizer, and I'm getting a warning that's very, very confusing to my understanding on how mutexes work. I'm using gcc 6.3 on Debian Stretch.

In a class, at one thread, I have:

auto MyPtr = std::make_shared<MyClass>(...);

At another place that's called by another thread, I have:

if(MyPtr.get()) {...}

ThreadSanitizer warned me about a race condition, which is great. So I fixed this by the following:

std::unique_lock<decltype(MyMutex)> lg(MyMutex); //MyMutex is std::mutex
auto MyPtr = std::make_shared<...>(...);
lg.unlock();

And the other spot:

std::unique_lock<decltype(MyMutex)> lg(MyMutex);
if(MyPtr.get()) {...}
// mutex unlocks at the end of the function, which is like after another if-condition.

Now the data race is gone, and ThreadSanitizer says that the Mutex is being unlocked "twice"... 

WARNING: ThreadSanitizer: unlock of an unlocked mutex (or by a wrong thread)

and it's pointing to the unlock() call + the end of the other function.

How could a mutex be unlocked twice? Could someone explain?

Now Since this gave me this headache, I decided to do this instead:

std::shared_ptr<MyClass> MyPtr; //in the class definition
std::atomic_store(&MyPtr, std::make_shared<MyClass>(...));

And now I get a data-race complaint:

WARNING: ThreadSanitizer: data race

So am I using ThreadSanitizer wrong? Could someone explain what's going on here?

The Quantum Physicist
  • 24,987
  • 19
  • 103
  • 189
  • 1
    Try `std::lock_guard`. You might want to post a [mcve], because without seeing the complete code, (at least) I am unable to figure out why ThreadSanitizer complains (it depends on a lot of stuff, which you haven't mentioned in your description, that's why code is better than text IMO) – Rakete1111 Jul 20 '17 at 15:22
  • How is your second thread getting access to `MyPtr`? There seems to me a possibility that `MyPtr` goes out to scope in the first thread (thus destroying the pointee) before the second gets to run - but there's not enough code to be sure. Thread 2 needs to ensure it has a copy of MyPtr for its lifetime - easiest way is to copy-by-capture in a lambda. You'd usually do this with a `weak_ptr`. – marko Jul 20 '17 at 16:23
  • @Rakete1111 I'm unable to create the minimal example... I tried and failed for like 30 minutes. It doesn't create the problem. – The Quantum Physicist Jul 20 '17 at 16:23
  • @marko It's a member of the same class that has both methods. – The Quantum Physicist Jul 20 '17 at 16:24
  • 1
    Also worth noting - `std::shared_ptr` offers some thread-safety guarantees - particularly around any operation that modifies the reference count. It is therefore always safe to copy or release a reference to a `shared_ptr`. – marko Jul 20 '17 at 16:26
  • @marko True, but it's not thread-safe to use `operator=` from another `shared_ptr`, AFAIK. The thread-safety is offered with an atomic reference counter only. – The Quantum Physicist Jul 20 '17 at 16:28
  • @TheQuantumPhysicist Ok. Let's frame it another way: is an object of your class guaranteed to exist as long as the thread that subsequently uses `MyPtr`? The use of a mutex to protect your data doesn't help here as it got deleted as well with the object. – marko Jul 20 '17 at 16:28
  • @marko Good question. Let me play around with shared_ptr instances to make sure that's the case. – The Quantum Physicist Jul 20 '17 at 16:31
  • @TheQuantumPhysicist if that were true, the other thread safety guarantees are essentially useless. The pointee of the `shared_ptr` on the rhs of the expression safely gains an extra reference - notionally owned by the lhs. The pointee on the LHS safety looses a reference (potentially getting deleted). The assumption here is that the shared_ptr isn't deleted beneath you - but that's an entirely different kind problem - and possibly the one that's pertinent to you here. Could you post a more complete code sample so we can see what you're trying to achieve? – marko Jul 20 '17 at 16:41
  • @marko That's actually very, very difficult. The program is huge. I'm still thinking of a way to do this. – The Quantum Physicist Jul 20 '17 at 17:09
  • @marko By making the other load atomic the problem was solved... – The Quantum Physicist Jul 20 '17 at 17:21

1 Answers1

1

I never figured out the mutex problem, but I was able to get rid of the data race with atomics by making the other load atomic:

if(std::atomic_load(&MyPtr).get()) {...}
The Quantum Physicist
  • 24,987
  • 19
  • 103
  • 189
  • Be careful relying on that result later, because you aren't keeping the the `shared_ptr` returned by `atomic_load` around, it can still get invalidated at any point. If you are using that pointer at all, keep the `shared_ptr` around – Russell Greene Jun 17 '22 at 16:55