concurrentqueue icon indicating copy to clipboard operation
concurrentqueue copied to clipboard

possible race condtion in accessing object in shared_ptr if share_ptr is the template type for concurrentqueue?

Open tesla1060 opened this issue 2 years ago • 2 comments

Hi, I would like to know if below passing shared_ptr created potential race condtion,

#include <string>
#include <thread>
#include <memory>
#include <condition_variable>
#include <mutex>
#include <deque>
#include <iostream>
#include <vector>
#include "third_party/concurrentqueue.h"
struct Dummy {
    std::string name_;
};

class Consumer {
public:
    Consumer() {
        is_running_ = true;
        data_thread_ = std::unique_ptr<std::thread>(new std::thread(&Consumer::listening, this));
    }
    void push(std::shared_ptr<Dummy> frame)
    {
        nice_queue.enqueue(frame);
    }
    virtual void listening() {
        while (is_running_) {
            std::shared_ptr<Dummy> frame;
            if (nice_queue.try_dequeue(frame)) {
                std::cout << frame->name_ << std::endl; //<--- is here a possible race condition as there is no memory fence
            }
        }
    }
    std::unique_ptr<std::thread> data_thread_;
    moodycamel::ConcurrentQueue<std::shared_ptr<Dummy>> nice_queue;
    std::atomic<bool> is_running_;
};

class Producer {
public:
    Producer(std::shared_ptr<Consumer> consumer)
    :consumer_(consumer){}
    void send(std::shared_ptr<Dummy> frame) {
        consumer_->push(frame);
    }
protected:
    std::shared_ptr<Consumer> consumer_;
};

int main() {
    std::shared_ptr<Consumer> consumer = std::make_shared<Consumer>();
    Producer prod(consumer);
    for(int i =0; i<100; i++){
        auto item = std::make_shared<Dummy>();
        item->name_ = std::to_string(i);
        prod.send(item);
    }
}

My question is as the code shows, the Producer send an item to Consumer to be processed in another thread, so two things happens strictly one after another, but due to there is no memory fence, is there a possibility that when Consumer proccess the item, it doesnt see the final version of the item, and thus have an undefined behaviour?

Another question is will the below code has race condition? as this time I am passing index of the vector through the queue.

class MultiThread {
public:
    MultiThread()
    :size_(100000), data_count_(0), data_list_(100000){
        thread_send_ = std::unique_ptr<std::thread>(new std::thread(&MultiThread::write_tick, this));
    }
    void produce() {
        while(1) {
            int node = data_count_ % size_;
            data_list_[node].name_=std::to_string(data_count_);
            tick_queue_.enqueue(node);
            ++data_count_;
        }

    }
    void write_tick()
    {
        while (1)
        {
            int node;
            if (tick_queue_.try_dequeue(node)) {
                std::cout << data_list_[node].name_ << std::endl; //<-----Race Condition herer?
            }
        }
    }
    std::unique_ptr<std::thread> thread_send_;
    moodycamel::ConcurrentQueue<int> tick_queue_;
    std::vector<Dummy> data_list_;
    int data_count_;
    const int size_;
};

tesla1060 avatar Aug 25 '23 12:08 tesla1060

  1. There's memory fences in the queue itself for precisely this reason -- enqueuing will "happen before" the dequeue operation for any given element, making it safe to access name_ here.
  2. As the vector is not changing in size while being accessed concurrently, this pattern would normally be safe (each element can be accessed independently by different threads without races), however produce can loop around and write to the same node again while it's being concurrently accessed by write_tick. This is unlikely in practice but possible and technically undefined behaviour.

cameron314 avatar Aug 26 '23 22:08 cameron314

thanks! understand now.

tesla1060 avatar Aug 31 '23 00:08 tesla1060