Is it concurrency-safe to call concurrency::concurrent_vector::push_back while iterating over that concurrent_vector in other thread?

Question

push_back, begin, end are described as concurrent safe in https://docs.microsoft.com/en-us/cpp/parallel/concrt/reference/concurrent-vector-class?view=vs-2019#push_back

However the below code is asserting. Probably because element is added but not initialized yet.

struct MyData
   {
   explicit MyData()
      {
      memset(arr, 0xA5, sizeof arr);
      }
   std::uint8_t arr[1024];
   };

struct MyVec
   {
   concurrency::concurrent_vector<MyData> v;
   };

auto vector_pushback(MyVec &vec) -> void
   {
   vec.v.push_back(MyData{});
   }

auto vector_loop(MyVec &vec) -> void
   {
   MyData myData;
   for (auto it = vec.v.begin(); it != vec.v.end(); ++it)
      {
      auto res = memcmp(&(it->arr), &(myData.arr), sizeof myData.arr);
      assert(res == 0);
      }
   }

int main()
{
   auto vec = MyVec{};
   auto th_vec = std::vector<std::thread>{};
   for (int i = 0; i < 1000; ++i)
      {
      th_vec.emplace_back(vector_pushback, std::ref(vec));
      th_vec.emplace_back(vector_loop, std::ref(vec));
      }

   for(auto &th : th_vec)
      th.join();

    return 0;
}

Answer 1

According to the docs, it should be safe to append to a concurrency::concurrent_vector while iterating over it because the elements are not actually stored contiguously in memory like std::vector:

A concurrent_vector object does not relocate its elements when you append to it or resize it. This enables existing pointers and iterators to remain valid during concurrent operations.

However, looking at the actual implementation of push_back in VS2017, I see the following, which I don't think is thread-safe:

iterator push_back( _Ty &&_Item )
{
    size_type _K;
    void *_Ptr = _Internal_push_back(sizeof(_Ty), _K);
    new (_Ptr) _Ty( std::move(_Item));
    return iterator(*this, _K, _Ptr);
}

I have to speculate on _Internal_push_back here, but I'd wager it allocates raw memory for storing the item (and points the last element towards this new node) so that the next line can use emplacement new. I'd imagine that _Internal_push_backis internally thread-safe, however I don't see any synchronization happening before the emplacement new. Meaning the following is possible:

• memory is obtained and the node is "present" (yet emplacement new hasn't happend)
• the looping thread encounters this node and performs memcmp to discover that they're not equal
• emplacement new happens.

There's definitely a race condition here. I can spontaneously reproduce the problem, moreso the more threads I use.

I recommend that you open a ticket with Microsoft support on this one.