Synchronization via shared_ptr: ThreadSanitzier false positive?

Question

The following code synchronizes via shared_ptr:

#include <memory>
#include <thread>
#include <future>
#include <chrono>
#include <cassert>
#include <atomic>

using std::shared_ptr;
using std::async;
using std::launch;
using std::this_thread::sleep_for;
using namespace std::literals;

void f1(shared_ptr<int> p)
{
    sleep_for(50ms); // make sure the other has started
    assert(*p == 42);
    p.reset();
    sleep_for(50ms); // make sure the other has deleted *p
}

void f2(shared_ptr<int> p)
{
    while (p.use_count() != 1)
    {
        sleep_for(1ms);
    }
    p.reset();
    sleep_for(50ms);
}

int main()
{
    shared_ptr<int> p(new int(42));
    auto t1 = async(launch::async, f1, p);
    auto t2 = async(launch::async, f2, p);

    p.reset();

    t1.get();
    t2.get();

    return 0;
}

I compile this with

clang++-4.0 -std=c++1z -stdlib=libc++ -Wall -g -O3 -march=native -fsanitize=thread -fno-omit-frame-pointer -pthread sharedPtr.cc -o sharedPtr

When running this, ThreadSanitizer gives me following issues:

==================
WARNING: ThreadSanitizer: data race (pid=273)
  Write of size 8 at 0x7b0400000000 by thread T2:
    #0 operator delete(void*) ??:? (sharedPtr+0x4b4af1)
    #1 std::__1::default_delete<int>::operator()(int*) const /usr/include/c++/v1/memory:2516 (discriminator 1) (sharedPtr+0x4b74d8)
    #2 std::__1::__shared_ptr_pointer<int*, std::__1::default_delete<int>, std::__1::allocator<int> >::__on_zero_shared() /usr/include/c++/v1/memory:3759 (discriminator 1) (sharedPtr+0x4b74d8)
[...]

  Previous read of size 4 at 0x7b0400000000 by thread T1:
    #0 f1(std::__1::shared_ptr<int>) /home/dv/src/git/c++-concurrency/test/sharedPtr.cc:22 (sharedPtr+0x4b6fca)
    #1 _ZNSt3__18__invokeIPFvNS_10shared_ptrIiEEEJS2_EEEDTclclsr3std3__1E7forwardIT_Efp_Espclsr3std3__1E7forwardIT0_Efp0_EEEOS5_DpOS6_ /usr/include/c++/v1/__functional_base:415 (sharedPtr+0x4b78cf)
[...]

I assume that C++ guarantees enough synchronization via the shared_ptr reference count that reading through the shared_ptr never races with the deleter (for different shared_ptr objects). And I'd expect that this is quite common usage, so I'm surprised that ThreadSanitizer complains about this.

So here are my questions:

Is my use safe (and my assumptions about shared_ptr synchronization correct)? (I expect the answer to this being yes, so now follow my real questions:)
Does libc++ implement the synchronization correctly?
Does ThreadSanitizer really doesn't see the synchronization through the reference count?

Possible duplicate of [why is std::shared\_ptr using atomic cpu operations](http://stackoverflow.com/questions/8980328/why-is-stdshared-ptr-using-atomic-cpu-operations) — The Quantum Physicist, Mar 22 '17 at 11:47
Ok, my first question is probably answered several times on SO. I added it merely for completeness. My real question is the third. — Craig P, Mar 22 '17 at 12:02
I'm retracting my duplicate-close request. Please correct your question and write the first two points as assumptions, so that people understand on what part to focus. — The Quantum Physicist, Mar 22 '17 at 12:04

score 3 · Answer 1 · answered Mar 22 '17 at 12:17

3

Is my use safe (and my assumptions about shared_ptr synchronization correct)?

I don't see anything in the standard that requires a memory fence in the implementation of use_count(). cppreference indicates that most implementations use memory_order_relaxed for the read, so sequencing is not guaranteed.

quoting: "In multithreaded environment, the value returned by use_count is approximate (typical implementations use a memory_order_relaxed load)"

Strictly speaking therefore, I don't think the use of use_count() as a semaphore is safe, since it relies on an assumption about the implementation.

Does libc++ implement the synchronization correctly?

Yes

Does ThreadSanitizer really doesn't see the synchronization through the reference count?

I think ThreadSanitizer is correctly calling you out.

answered Mar 22 '17 at 12:17

Richard Hodges

68,278
7
90
142

I agree that use_count() doesn't provide synchronization, and I'm not using it that way. The synchronization is provided by p.reset() (i.e. the manipulation of the internal reference count). – Craig P Mar 22 '17 at 12:33
@CraigP `p.reset()` and `p.use_count()` are two ends of the same data flow pipeline. The use of `memory_order_relaxed` in the fetch in `use_count` means that the second thread may not observe the change since the fetch can be re-ordered. Even `sleep_for` does not strictly require a memory fence to be emitted. The code is a race. It happens to work on a 386 but need not complete on all architectures. – Richard Hodges Mar 22 '17 at 13:18
sleep_for is only used to guarantee the pattern that ThreadSanitizer complains about. I hope it doesn't introduce (aquire/release) synchronization, otherwise my test would test what I want to. – Craig P Mar 22 '17 at 13:35
[SO submitted my other comment when I hit , and didn't let me edit it due to some 5min rule...] @richard I still claim that I don't use use_count() for synchronization. It's only used for preducing the test pattern I want to test (i.e. f2() deletes the object). Synchronization is solely between the reset() calls: p.reset() in f1() synchronizes with p.reset() in f2(), so that the reference count in f2() reaches zero and the object is deleted. – Craig P Mar 22 '17 at 13:49
@CraigP right, but because of the possibility of re-ordering, there is no guarantee that `(p.use_count() == 1)` will ever be `true`. It just happens to work on your architecture. – Richard Hodges Mar 22 '17 at 14:18
1

@RichardHow so? When I enter `f2()`, `p.use_count()` will be 1 or greater. Given concurrent progress for all three threads, `p.reset()` in the other threads will be called at some time and assuming fair hardware, even a relaxed load inside `use_count()` will eventually read 1. – Craig P Mar 22 '17 at 14:45

Synchronization via shared_ptr: ThreadSanitzier false positive?

1 Answers1