Search code examples
c++race-conditionc++20range-v3

Why thread sanitizer complains about this std::ranges::views::filter code?


When running this code thread sanitizer complains about data race. Why?

#include <iostream>
#include <ranges>
#include <thread>
#include <vector>

int main(){
    std::vector v{11,22,33,44,55,66};
    auto view = v | std::ranges::views::filter([](const auto x){
        return x>47;
    });
    std::jthread jt1([&]{
    int sum =0;
    for (const auto& val: view) {sum+=val;}
    });    
    std::jthread jt2([&]{
    int sum =0;
    for (const auto& val: view) {sum+=val;}
    });
}

note: I know answer to this, I learned it recently watching a talk, but I found it very surprising and no existing questions related to this, so I wanted to share it. If nobody is interested in answering I will answer it myself.


Solution

  • [&] in a thread-running lambda should immediately ring an alarm. It is generally not safe to pass random objects by non-const reference to threads. In particular, it is not thread-safe to invoke non-const member functions on standard library objects.

    begin is not a const member function of std::ranges::filter_view.

    constexpr iterator begin();
    Expects: pred_.has_value().
    Returns: {*this, ranges::find_if(base_, ref(*pred_))}.
    Remarks: In order to provide the amortized constant time complexity required by the range concept, this function caches the result within the filter_view for use on subsequent calls.

    In libstdc++ implementation, it's this exact caching operation that trips the thread sanitizer.

    An attempt to make view constant results in a compilation error.

    The same is true about reverse_view.

    Other range adaptor do have const overloads for begin.

    It is possible to work around this in practice by calling view.begin() before constructing the threads. I don't know if this makes undefined behaviour officially go away (I think it probably should), but it does silence the sanitizer.