c++ - ThreadSanitizer reports "data race on operator delete(void*)" when using embedded reference counter -

please have @ following code:

#include <pthread.h> #include <boost/atomic.hpp>  class referencecounted {   public:     referencecounted() : ref_count_(1) {}      void reserve() {       ref_count_.fetch_add(1, boost::memory_order_relaxed);     }      void release() {       if (ref_count_.fetch_sub(1, boost::memory_order_release) == 1) {         boost::atomic_thread_fence(boost::memory_order_acquire);         delete this;       }     }    private:     boost::atomic<int> ref_count_; };  void* thread1(void* x) {   static_cast<referencecounted*>(x)->release();   return null; }  void* thread2(void* x) {   static_cast<referencecounted*>(x)->release();   return null; }  int main() {   referencecounted* obj = new referencecounted();   obj->reserve(); // thread1   obj->reserve(); // thread2   obj->release(); // main()   pthread_t t[2];   pthread_create(&t[0], null, thread1, obj);   pthread_create(&t[1], null, thread2, obj);   pthread_join(t[0], null);   pthread_join(t[1], null); } 

this similar reference counting example boost.atomic.

the main differences embedded ref_count_ initialized 1 in constructor (once constructor completed have single reference referencecounted object) , code doesn't use boost::intrusive_ptr.

please don't blame me using delete this in code - pattern have in large code base @ work , there's nothing can right now.

now code compiled clang 3.5 trunk (details below) , threadsanitizer (tsan v2) results in following output threadsanitizer:

warning: threadsanitizer: data race (pid=9871)   write of size 1 @ 0x7d040000f7f0 thread t2:     #0 operator delete(void*) <null>:0 (a.out+0x00000004738b)     #1 referencecounted::release() /home/a.romanek/tmp/tsan/main.cpp:15 (a.out+0x0000000a2c06)     #2 thread2(void*) /home/a.romanek/tmp/tsan/main.cpp:29 (a.out+0x0000000a2833)    previous atomic write of size 4 @ 0x7d040000f7f0 thread t1:     #0 __tsan_atomic32_fetch_sub <null>:0 (a.out+0x0000000896b6)     #1 boost::atomics::detail::base_atomic<int, int, 4u, true>::fetch_sub(int, boost::memory_order) volatile /home/a.romanek/tmp/boost/boost_1_55_0/boost/atomic/detail/gcc-atomic.hpp:499 (a.out+0x0000000a3329)     #2 referencecounted::release() /home/a.romanek/tmp/tsan/main.cpp:13 (a.out+0x0000000a2a71)     #3 thread1(void*) /home/a.romanek/tmp/tsan/main.cpp:24 (a.out+0x0000000a27d3)    location heap block of size 4 @ 0x7d040000f7f0 allocated main thread:     #0 operator new(unsigned long) <null>:0 (a.out+0x000000046e1d)     #1 main /home/a.romanek/tmp/tsan/main.cpp:34 (a.out+0x0000000a286f)    thread t2 (tid=9874, running) created main thread at:     #0 pthread_create <null>:0 (a.out+0x00000004a2d1)     #1 main /home/a.romanek/tmp/tsan/main.cpp:40 (a.out+0x0000000a294e)    thread t1 (tid=9873, finished) created main thread at:     #0 pthread_create <null>:0 (a.out+0x00000004a2d1)     #1 main /home/a.romanek/tmp/tsan/main.cpp:39 (a.out+0x0000000a2912)  summary: threadsanitizer: data race ??:0 operator delete(void*) ================== threadsanitizer: reported 1 warnings 

the strange thing thread t1 write of size 1 same memory location thread t2 when doing atomic decrement on reference counter.

how can former write explained? clean-up performed destructor of referencecounted class?

it false positive? or code wrong?

my setup is:

$ uname -a linux aromanek-laptop 3.13.0-29-generic #53-ubuntu smp wed jun 4 21:00:20 utc 2014 x86_64 x86_64 x86_64 gnu/linux  $ clang --version ubuntu clang version 3.5-1ubuntu1 (trunk) (based on llvm 3.5) target: x86_64-pc-linux-gnu thread model: posix 

the code compiled this:

clang++ main.cpp -i/home/a.romanek/tmp/boost/boost_1_55_0 -pthread -fsanitize=thread -o0 -g -ggdb3 -fpie -pie -fpic 

note on machine implementation of boost::atomic<t> resolves __atomic_load_n family of functions, threadsanitizer claims understand.

update 1: same happens when using clang 3.4 final release.

update 2: same problem occurs -std=c++11 , <atomic> both libstdc++ , libc++.

this looks false positive.

the thread_fence in release() method enforces outstanding writes fetch_sub-calls happen-before fence returns. therefore, delete on next line cannot race previous writes decreasing refcount.

quoting book c++ concurrency in action:

a release operation synchronizes-with fence order of std::memory_order_acquire [...] if release operation stores value that's read atomic operation prior fence on same thread fence.

since decreasing refcount read-modify-write operation, should apply here.

to elaborate, order of operations need ensure follows:

1. decreasing refcount value > 1
2. decreasing refcount 1
3. deleting object

2. , 3. synchronized implicitly, happen on same thread. 1. , 2. synchronized since both atomic read-modify-write operations on same value. if these 2 race whole refcounting broken in first place. left synchronizing 1. , 3..

this fence does. write 1. release operation is, discussed, synchronized 2., read on same value. 3., acquire fence on same thread 2., synchronizes write 1. guaranteed spec. happens without requiring addition acquire write on object (as suggested @kerreksb in comments), work, might potentially less efficient due additional write.

bottom line: don't play around memory orderings. experts them wrong , impact on performance negligible. unless have proven in profiling run kill performance , absolutely positively have to optimize this, pretend don't exist , stick default memory_order_seq_cst.