llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.33k stars 11.7k forks source link

libcxx: race condition on weak_ptr/shared_ptr destructor #111048

Open anarthal opened 2 days ago

anarthal commented 2 days ago

I've been running several tests involving sharing std::shared_ptr and std::weak_ptr objects under thread sanitizer. I've been getting a bunch of race condition reports. The code under test shares a single, constant value allocated using std::make_shared by copying a bunch of std::shared_ptr and std::weak_ptr objects to several threads.

I've been able to isolate a minimum reproducible example with clang-18 and libc++-18 packages under Ubuntu 24.04. Please find a full Dockerfile containing self-contained instructions on how to reproduce the issue at the bottom of this message.

Interestingly, the race condition is only reported when combining std::shared_ptr and std::weak_ptr objects. Commenting the branch creating the weak pointers makes the race condition disappear.

Please let me know if I can help anyhow.

Dockerfile for reproduction:

FROM ubuntu:24.04

RUN DEB_FRONTEND=noninteractive apt-get update && apt-get install -y \
    clang-18 libc++-18-dev libc++abi-18-dev

COPY <<EOF /repro/main.cpp

#include <memory>
#include <thread>
#include <vector>

void perform()
{
    // Setup
    auto ptr = std::make_shared<int>(0);

    std::vector<std::thread> threads;
    for (int i = 0; i < 16; ++i)
    {
        if (i % 2)
        {
            threads.emplace_back([ptr]() mutable {
                std::this_thread::yield();
                ptr.reset();
            });
        }
        else
        {
            threads.emplace_back([w = std::weak_ptr<int>(ptr)]() mutable {
                std::this_thread::yield();
                w.reset();
            });
        }
    }

    ptr.reset();

    for (auto& t : threads)
        t.join();
}

int main()
{
    for (int i = 0; i < 10000; ++i)
        perform();
}

EOF

RUN clang++-18 -fsanitize=thread -std=c++20 -g -O0 -stdlib=libc++ -o /repro/main /repro/main.cpp
RUN TSAN_OPTIONS=halt_on_error=1 /repro/main

TSAN report:

2.954 ==================
2.954 WARNING: ThreadSanitizer: data race (pid=7)
2.954   Write of size 8 at 0x720800002720 by thread T25791:
2.954     #0 operator delete(void*) <null> (main+0xe130d) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.954     #1 void std::__1::__libcpp_operator_delete[abi:ne180100]<void*>(void*) /usr/lib/llvm-18/bin/../include/c++/v1/new:280:3 (main+0xe75f5) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #2 void std::__1::__do_deallocate_handle_size[abi:ne180100]<>(void*, unsigned long) /usr/lib/llvm-18/bin/../include/c++/v1/new:302:10 (main+0xe7571) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #3 std::__1::__libcpp_deallocate[abi:ne180100](void*, unsigned long, unsigned long) /usr/lib/llvm-18/bin/../include/c++/v1/new:317:12 (main+0xe74ba) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #4 std::__1::allocator<std::__1::__shared_ptr_emplace<int, std::__1::allocator<int>>>::deallocate[abi:ne180100](std::__1::__shared_ptr_emplace<int, std::__1::allocator<int>>*, unsigned long) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/allocator.h:139:7 (main+0xe8aba) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #5 std::__1::allocator_traits<std::__1::allocator<std::__1::__shared_ptr_emplace<int, std::__1::allocator<int>>>>::deallocate[abi:ne180100](std::__1::allocator<std::__1::__shared_ptr_emplace<int, std::__1::allocator<int>>>&, std::__1::__shared_ptr_emplace<int, std::__1::allocator<int>>*, unsigned long) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/allocator_traits.h:289:9 (main+0xe8a45) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #6 std::__1::__shared_ptr_emplace<int, std::__1::allocator<int>>::__on_zero_shared_weak() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:294:5 (main+0xe877c) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #7 std::__1::weak_ptr<int>::~weak_ptr() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:1401:15 (main+0xe78d6) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #8 std::__1::weak_ptr<int>::reset() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:1450:3 (main+0xe77b3) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #9 perform()::$_1::operator()() /repro/main.cpp:25:19 (main+0xe38e2) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #10 decltype(std::declval<perform()::$_1>()()) std::__1::__invoke[abi:ne180100]<perform()::$_1>(perform()::$_1&&) /usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/invoke.h:344:25 (main+0xe3855) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #11 void std::__1::__thread_execute[abi:ne180100]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, perform()::$_1>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, perform()::$_1>&, std::__1::__tuple_indices<...>) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:193:3 (main+0xe380d) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #12 void* std::__1::__thread_proxy[abi:ne180100]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, perform()::$_1>>(void*) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:202:3 (main+0xe3452) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955 
2.955   Previous read of size 8 at 0x720800002720 by main thread:
2.955     #0 std::__1::__shared_count::__release_shared[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:157:7 (main+0xe8ce0) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #1 std::__1::__shared_weak_count::__release_shared[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:186:25 (main+0xe8c79) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.955     #2 std::__1::shared_ptr<int>::~shared_ptr[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:648:17 (main+0xe4136) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #3 std::__1::shared_ptr<int>::reset[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:698:50 (main+0xe3ed3) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #4 perform() /repro/main.cpp:30:9 (main+0xe1cab) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #5 main /repro/main.cpp:39:9 (main+0xe2060) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956 
2.956   Thread T25791 (tid=25799, running) created by main thread at:
2.956     #0 pthread_create <null> (main+0x616df) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #1 std::__1::__libcpp_thread_create[abi:ne180100](unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-18/bin/../include/c++/v1/__threading_support:317:10 (main+0xe4599) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #2 std::__1::thread::thread<perform()::$_1, void>(perform()::$_1&&) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:212:14 (main+0xe324e) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #3 std::__1::thread* std::__1::construct_at[abi:ne180100]<std::__1::thread, perform()::$_1, std::__1::thread*>(std::__1::thread*, perform()::$_1&&) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/construct_at.h:41:46 (main+0xe3171) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #4 std::__1::thread* std::__1::__construct_at[abi:ne180100]<std::__1::thread, perform()::$_1, std::__1::thread*>(std::__1::thread*, perform()::$_1&&) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/construct_at.h:49:10 (main+0xe3105) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #5 void std::__1::allocator_traits<std::__1::allocator<std::__1::thread>>::construct[abi:ne180100]<std::__1::thread, perform()::$_1, void, void>(std::__1::allocator<std::__1::thread>&, std::__1::thread*, perform()::$_1&&) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/allocator_traits.h:305:5 (main+0xe30a1) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.956     #6 void std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::__construct_one_at_end[abi:ne180100]<perform()::$_1>(perform()::$_1&&) /usr/lib/llvm-18/bin/../include/c++/v1/vector:902:5 (main+0xe2ea4) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.957     #7 std::__1::thread& std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::emplace_back<perform()::$_1>(perform()::$_1&&) /usr/lib/llvm-18/bin/../include/c++/v1/vector:1508:5 (main+0xe1f6c) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.957     #8 perform() /repro/main.cpp:23:21 (main+0xe1c62) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.957     #9 main /repro/main.cpp:39:9 (main+0xe2060) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2)
2.957 
2.957 SUMMARY: ThreadSanitizer: data race (/repro/main+0xe130d) (BuildId: 729198def45d306742914bd873c6632f97bd2eb2) in operator delete(void*)
2.957 ==================
Flamefire commented 2 days ago

FWIW: The issue seems to be solely in weak_ptr as this example fails in the same way. I also added a define check to verify the atomic is actually used, see also https://godbolt.org/z/7ara7MP5f:

#include <memory>
#include <thread>
#include <vector>

#if defined(_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT) && !defined(_LIBCPP_HAS_NO_THREADS)
// OK
#else
#error "FAIL"
#endif

void perform()
{
    // Setup
    std::weak_ptr<int> w_outer = std::shared_ptr<int>(new int);

    std::vector<std::thread> threads;
    for (int i = 0; i < 16; ++i)
    {
        threads.emplace_back([w = w_outer]() mutable {
            std::this_thread::yield();
            w.reset();
        });
    }

    w_outer.reset();

    for (auto& t : threads)
        t.join();
}

int main()
{
    for (int i = 0; i < 10000; ++i)
        perform();
}