TSAN thread race error detected with Boost intrusive_ptr use

164 views Asked by At

Below code generates a TSAN error(race condition). Is this a valid error ? or a false positive ?
Object is destroyed only after ref count becomes zero(after all other thread memory operations are visible - with atomic_thread_fence)
If I use a std::shared_ptr instead of boost::intrusive_ptr, then TSAN error disappears.
Since both threads use the object as read-only, I presume this should be safe.
If this is a valid error how do I fix it ?
gcc version - 7.3.1 boost version - 1.72.0

compile command : "g++ -ggdb -I /usr/local/boost_1_72_0 -O3 -fsanitize=thread TSan_Intr_Ptr.cpp -lpthread "

#include <boost/smart_ptr/intrusive_ptr.hpp>
#include <boost/smart_ptr/detail/spinlock.hpp>
#include <boost/atomic.hpp>
#include <thread>
#include <iostream>
#include <vector>
#include <atomic>
#include <unistd.h>
using namespace std;

struct Shared
{
        mutable boost::atomic<int> refcount_;

        //From https://www.boost.org/doc/libs/1_72_0/doc/html/atomic/usage_examples.html
        friend void intrusive_ptr_add_ref(const Shared * x)
        {
                x->refcount_.fetch_add(1, boost::memory_order_relaxed);
        }

        friend void intrusive_ptr_release(const Shared* x)
        {
                if (x->refcount_.fetch_sub(1, boost::memory_order_release) == 1)
                {
                        boost::atomic_thread_fence(boost::memory_order_acquire);
                        delete x;
                }
        }
};

vector<boost::intrusive_ptr<Shared const>>  g_vec;
boost::detail::spinlock g_lock = BOOST_DETAIL_SPINLOCK_INIT;

void consumer()
{
        while(true)
        {
                g_lock.lock();
                g_vec.clear();
                g_lock.unlock();
                usleep(10);
        }

}

int main()
{
        thread thd(consumer);

        while(true)
        {
                boost::intrusive_ptr<Shared const> p(new Shared);
                g_lock.lock();
                g_vec.push_back(p);
                g_lock.unlock();
                usleep(1);
        }
        return 0;
}

TSAN Error

WARNING: ThreadSanitizer: data race (pid=14513)
  Write of size 8 at 0x7b0400000010 by main thread:
    #0 operator delete(void*) <null> (libtsan.so.0+0x00000006fae4)
    #1 intrusive_ptr_release(Shared const*) /Test/TSan_Intr_Ptr_Min.cpp:25 (a.out+0x000000401195)
    #2 boost::intrusive_ptr<Shared const>::~intrusive_ptr() /boost_1_72_0/boost/smart_ptr/intrusive_ptr.hpp:98 (a.out+0x000000401195)
    #3 main /x01/exch/Test/TSan_Intr_Ptr_Min.cpp:51 (a.out+0x000000401195)

Previous atomic write of size 4 at 0x7b0400000010 by thread T1:
    #0 __tsan_atomic32_fetch_sub <null> (libtsan.so.0+0x00000006576f)
    #1 boost::atomics::detail::gcc_atomic_operations<4ul, true>::fetch_sub(unsigned int volatile&, unsigned int, boost::memory_order) /boost_1_72_0/boost/atomic/detail/ops_gcc_atomic.hpp:116 (a.out+0x000000401481)
    #2 boost::atomics::detail::base_atomic<int, int>::fetch_sub(int, boost::memory_order) volatile /usr/local/boost_1_72_0/boost/atomic/detail/atomic_template.hpp:348 (a.out+0x000000401481)
    #3 intrusive_ptr_release(Shared const*) /Test/TSan_Intr_Ptr_Min.cpp:22 (a.out+0x000000401481)
...
2

There are 2 answers

1
aKumara On BEST ANSWER

it seems using memory_order_acq_rel resolves the issue. (May be https://www.boost.org/doc/libs/1_72_0/doc/html/atomic/usage_examples.html example is in-correct)


        friend void intrusive_ptr_add_ref(const Shared * x)
        {
                x->refcount_.fetch_add(1, boost::memory_order_acq_rel);
        }

        friend void intrusive_ptr_release(const Shared* x)
        {
                if (x->refcount_.fetch_sub(1, boost::memory_order_acq_rel) == 1)
                {
                        delete x;
                }
        }
0
Turtlefight On

This is a false positive.

The underlying issue is that TSAN does not support atomic thread fences.
This is still the case as of the time this answer was written.

So from TSAN's point of view its as if the line boost::atomic_thread_fence(boost::memory_order_acquire); doesn't exist at all.

The fix in this case is to just explicitly inform TSAN about the acquire fence:

void intrusive_ptr_release(const Shared* x) {
    if (x->refcount_.fetch_sub(1, boost::memory_order_release) == 1) {
#if __SANITIZE_THREAD__
        // TSAN doesn't handle atomic_thread_fence at all.
        // simulate it as an acquire on refcount_:
        __tsan_acquire(&x->refcount_);
#endif
        boost::atomic_thread_fence(boost::memory_order_acquire);
        delete x;
                        
    }
}

godbolt example


gcc 12 added a warning in this case (-Wtsan), that will warn you about this if you're using atomic_thread_fence and compiling with -fsanitize=thread:

In function 'void boost::atomics::detail::thread_fence(boost::memory_order)'
warning: 'atomic_thread_fence' is not supported with '-fsanitize=thread' [-Wtsan]
  380 |     __atomic_thread_fence(atomics::detail::convert_memory_order_to_gcc(order));
      |     ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

godbolt example

see GCC Bug 97868 - warn about using fences with TSAN


Similar questions: