data is not synchronized when try to implement a producer, consumer, ring buffer model

-2

Correct Code Link: https://wandbox.org/permlink/JYr2XoaSxsS1QT14

I've been stuck for days because of this.. I tried to make a producer->ring buffer->consumer model. At first I used a mutex to make it, it works but it's not asynchronous. I want the consumer keeps reading without any stops, just like a video player, which I think I can't accomplish with a mutex.

Here's what I've done:

I make a FIFO structure based on fixed size dynamic array. (ring buffer)

I have a pair of pointers for both push_right and pop_left operations. So there won't have data race problems, if I understand correctly.

I make the producer to write several items ahead, then consumer starts to read and need to ensure that:
- consumer reads speed <= producer writes speed （consumer read pointer < producer write pointer ）
- A appropriate fixed array size, so that the producer won't override items that consumer haven't read.

My problem is that the output result is not as I expected, it's not synchronized.. And I have no idea how to debug this.

You can see the data write order (P ostream:) is not the same as the read order (O ostream:).

One possible output:

data size: 20

70 927 156 109 834 26 883 576 226 500 904 777 935 80 346 559 846 879 548 791 

********************

Consumer start working

791

548

879

846

26

346

109

156

927

70

500

226

576

883

26

834

109

156

927

70

Consumer done

********************

p_count: 20

c_count: 20

P ostream: 791 548 879 846 559 346 80 935 777 904 500 226 576 883 26 834 109 156 927 70 

C ostream: 791 548 879 846 26 346 109 156 927 70 500 226 576 883 26 834 109 156 927 70

Code:

(It contains main.cpp, circular_array and a data txt file, so I
think visit the link would be less pain)

https://wandbox.org/permlink/ddQjNFdxABrjminQ

main.cpp

#include <iostream>

#include <thread>

#include <chrono>

#include <vector>

#include <fstream>

#include <sstream>

#include "circular_array.h"

using namespace ythlearn;

using namespace std;



int p_count = 0;

int c_count = 0;

int fileSize = 0;

ostringstream p_os, c_os;



void producer(CircularArray<int>* Ca, vector<int> &data){

    for(int i = 0; i < 5; i++){

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }

    while(!data.empty()){

        this_thread::sleep_for(chrono::seconds(1));    

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }



}



void consumer(CircularArray<int>* Ca){

    cout << "********************" << endl;

    cout << "Consumer start working" << endl;

    this_thread::sleep_for(chrono::seconds(5));

    while(c_count < fileSize){

        this_thread::sleep_for(chrono::seconds(1));    

        int re = Ca->pop_left();

        cout << re << endl;

        c_os << re << " ";

        c_count++;



    }

    cout << "Consumer done" << endl;

    cout << "********************" << endl;



}





void getInput(vector<int>& data){

    ifstream ifs("test.txt");

    int j;

    while(ifs >> j){

        data.push_back(j);

    }

}



int main(){

    cout << unitbuf;

    vector<int> data;

    getInput(data);



    CircularArray<int> Ca;



    ::fileSize = data.size();

    cout << "data size: " << ::fileSize << endl;

    for(const auto& s: data){

        cout << s << " ";

    }

    cout << endl;





    thread th_producer(producer, &Ca, std::ref(data));

    thread th_consumer(consumer, &Ca);



    th_consumer.join();

    th_producer.join();

    cout << "p_count: " << p_count << endl

         << "c_count: " << c_count << endl;

    cout << "P ostream: " << p_os.str() << endl;

    cout << "C ostream: " << c_os.str() << endl;

    return 0;

}

circular_array.h

#pragma once

#include <stdexcept>

#include <iostream>

namespace ythlearn{

    template<typename T>

    class CircularArray{

        public:

            CircularArray(int N = 10){

                head = tail = new T[N];

                past_end_ptr1 = past_end_ptr2 = head + N;

                start_ptr1 = start_ptr2 = head;

                _capacity = N;

                _size = 0;

            }



            void push_right(T elem){

                *tail = elem;

                if(tail + 1 == past_end_ptr1){

                    tail = start_ptr1;

                }else{

                    tail++;

                }

            }



            T pop_left(){

                T re = *head;

                if(head + 1 == past_end_ptr2){

                    head = start_ptr2;

                }else{

                    head++;

                }

                return re;

            }



            CircularArray& operator=(const CircularArray&) = delete;

            CircularArray(const CircularArray&) = delete;

            ~CircularArray(){

                delete start_ptr1;

            }



        private:

            T* head;

            T* tail;

            T* start_ptr1, *start_ptr2;

            T* past_end_ptr1, *past_end_ptr2;

            int _capacity;

            int _size;

    };

}

test.txt

edited Nov 26 '18 at 9:23

asked Nov 20 '18 at 3:57

Rick

1,548929

Post the code and output; don’t post screen shots. Have some consideration for the people you are asking for help from.

– mevets
Nov 20 '18 at 18:34

1

It seems like Boost.Lockfree library may have an out-of-the-box solution. Check this out: boost::lockfree::spsc_queue

– Julien Villemure-Fréchette
Nov 26 '18 at 16:51

add a comment |

-2

Correct Code Link: https://wandbox.org/permlink/JYr2XoaSxsS1QT14

Here's what I've done:

I make a FIFO structure based on fixed size dynamic array. (ring buffer)

I have a pair of pointers for both push_right and pop_left operations. So there won't have data race problems, if I understand correctly.

I make the producer to write several items ahead, then consumer starts to read and need to ensure that:
- consumer reads speed <= producer writes speed （consumer read pointer < producer write pointer ）
- A appropriate fixed array size, so that the producer won't override items that consumer haven't read.

My problem is that the output result is not as I expected, it's not synchronized.. And I have no idea how to debug this.

You can see the data write order (P ostream:) is not the same as the read order (O ostream:).

One possible output:

data size: 20

70 927 156 109 834 26 883 576 226 500 904 777 935 80 346 559 846 879 548 791 

********************

Consumer start working

791

548

879

846

26

346

109

156

927

70

500

226

576

883

26

834

109

156

927

70

Consumer done

********************

p_count: 20

c_count: 20

P ostream: 791 548 879 846 559 346 80 935 777 904 500 226 576 883 26 834 109 156 927 70 

C ostream: 791 548 879 846 26 346 109 156 927 70 500 226 576 883 26 834 109 156 927 70

Code:

(It contains main.cpp, circular_array and a data txt file, so I
think visit the link would be less pain)

https://wandbox.org/permlink/ddQjNFdxABrjminQ

main.cpp

#include <iostream>

#include <thread>

#include <chrono>

#include <vector>

#include <fstream>

#include <sstream>

#include "circular_array.h"

using namespace ythlearn;

using namespace std;



int p_count = 0;

int c_count = 0;

int fileSize = 0;

ostringstream p_os, c_os;



void producer(CircularArray<int>* Ca, vector<int> &data){

    for(int i = 0; i < 5; i++){

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }

    while(!data.empty()){

        this_thread::sleep_for(chrono::seconds(1));    

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }



}



void consumer(CircularArray<int>* Ca){

    cout << "********************" << endl;

    cout << "Consumer start working" << endl;

    this_thread::sleep_for(chrono::seconds(5));

    while(c_count < fileSize){

        this_thread::sleep_for(chrono::seconds(1));    

        int re = Ca->pop_left();

        cout << re << endl;

        c_os << re << " ";

        c_count++;



    }

    cout << "Consumer done" << endl;

    cout << "********************" << endl;



}





void getInput(vector<int>& data){

    ifstream ifs("test.txt");

    int j;

    while(ifs >> j){

        data.push_back(j);

    }

}



int main(){

    cout << unitbuf;

    vector<int> data;

    getInput(data);



    CircularArray<int> Ca;



    ::fileSize = data.size();

    cout << "data size: " << ::fileSize << endl;

    for(const auto& s: data){

        cout << s << " ";

    }

    cout << endl;





    thread th_producer(producer, &Ca, std::ref(data));

    thread th_consumer(consumer, &Ca);



    th_consumer.join();

    th_producer.join();

    cout << "p_count: " << p_count << endl

         << "c_count: " << c_count << endl;

    cout << "P ostream: " << p_os.str() << endl;

    cout << "C ostream: " << c_os.str() << endl;

    return 0;

}

circular_array.h

#pragma once

#include <stdexcept>

#include <iostream>

namespace ythlearn{

    template<typename T>

    class CircularArray{

        public:

            CircularArray(int N = 10){

                head = tail = new T[N];

                past_end_ptr1 = past_end_ptr2 = head + N;

                start_ptr1 = start_ptr2 = head;

                _capacity = N;

                _size = 0;

            }



            void push_right(T elem){

                *tail = elem;

                if(tail + 1 == past_end_ptr1){

                    tail = start_ptr1;

                }else{

                    tail++;

                }

            }



            T pop_left(){

                T re = *head;

                if(head + 1 == past_end_ptr2){

                    head = start_ptr2;

                }else{

                    head++;

                }

                return re;

            }



            CircularArray& operator=(const CircularArray&) = delete;

            CircularArray(const CircularArray&) = delete;

            ~CircularArray(){

                delete start_ptr1;

            }



        private:

            T* head;

            T* tail;

            T* start_ptr1, *start_ptr2;

            T* past_end_ptr1, *past_end_ptr2;

            int _capacity;

            int _size;

    };

}

test.txt

edited Nov 26 '18 at 9:23

asked Nov 20 '18 at 3:57

Rick

1,548929

Post the code and output; don’t post screen shots. Have some consideration for the people you are asking for help from.

– mevets
Nov 20 '18 at 18:34

1

It seems like Boost.Lockfree library may have an out-of-the-box solution. Check this out: boost::lockfree::spsc_queue

– Julien Villemure-Fréchette
Nov 26 '18 at 16:51

add a comment |

-2

Correct Code Link: https://wandbox.org/permlink/JYr2XoaSxsS1QT14

Here's what I've done:

I make a FIFO structure based on fixed size dynamic array. (ring buffer)

I have a pair of pointers for both push_right and pop_left operations. So there won't have data race problems, if I understand correctly.

I make the producer to write several items ahead, then consumer starts to read and need to ensure that:
- consumer reads speed <= producer writes speed （consumer read pointer < producer write pointer ）
- A appropriate fixed array size, so that the producer won't override items that consumer haven't read.

My problem is that the output result is not as I expected, it's not synchronized.. And I have no idea how to debug this.

You can see the data write order (P ostream:) is not the same as the read order (O ostream:).

One possible output:

data size: 20

70 927 156 109 834 26 883 576 226 500 904 777 935 80 346 559 846 879 548 791 

********************

Consumer start working

791

548

879

846

26

346

109

156

927

70

500

226

576

883

26

834

109

156

927

70

Consumer done

********************

p_count: 20

c_count: 20

P ostream: 791 548 879 846 559 346 80 935 777 904 500 226 576 883 26 834 109 156 927 70 

C ostream: 791 548 879 846 26 346 109 156 927 70 500 226 576 883 26 834 109 156 927 70

Code:

(It contains main.cpp, circular_array and a data txt file, so I
think visit the link would be less pain)

https://wandbox.org/permlink/ddQjNFdxABrjminQ

main.cpp

#include <iostream>

#include <thread>

#include <chrono>

#include <vector>

#include <fstream>

#include <sstream>

#include "circular_array.h"

using namespace ythlearn;

using namespace std;



int p_count = 0;

int c_count = 0;

int fileSize = 0;

ostringstream p_os, c_os;



void producer(CircularArray<int>* Ca, vector<int> &data){

    for(int i = 0; i < 5; i++){

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }

    while(!data.empty()){

        this_thread::sleep_for(chrono::seconds(1));    

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }



}



void consumer(CircularArray<int>* Ca){

    cout << "********************" << endl;

    cout << "Consumer start working" << endl;

    this_thread::sleep_for(chrono::seconds(5));

    while(c_count < fileSize){

        this_thread::sleep_for(chrono::seconds(1));    

        int re = Ca->pop_left();

        cout << re << endl;

        c_os << re << " ";

        c_count++;



    }

    cout << "Consumer done" << endl;

    cout << "********************" << endl;



}





void getInput(vector<int>& data){

    ifstream ifs("test.txt");

    int j;

    while(ifs >> j){

        data.push_back(j);

    }

}



int main(){

    cout << unitbuf;

    vector<int> data;

    getInput(data);



    CircularArray<int> Ca;



    ::fileSize = data.size();

    cout << "data size: " << ::fileSize << endl;

    for(const auto& s: data){

        cout << s << " ";

    }

    cout << endl;





    thread th_producer(producer, &Ca, std::ref(data));

    thread th_consumer(consumer, &Ca);



    th_consumer.join();

    th_producer.join();

    cout << "p_count: " << p_count << endl

         << "c_count: " << c_count << endl;

    cout << "P ostream: " << p_os.str() << endl;

    cout << "C ostream: " << c_os.str() << endl;

    return 0;

}

circular_array.h

#pragma once

#include <stdexcept>

#include <iostream>

namespace ythlearn{

    template<typename T>

    class CircularArray{

        public:

            CircularArray(int N = 10){

                head = tail = new T[N];

                past_end_ptr1 = past_end_ptr2 = head + N;

                start_ptr1 = start_ptr2 = head;

                _capacity = N;

                _size = 0;

            }



            void push_right(T elem){

                *tail = elem;

                if(tail + 1 == past_end_ptr1){

                    tail = start_ptr1;

                }else{

                    tail++;

                }

            }



            T pop_left(){

                T re = *head;

                if(head + 1 == past_end_ptr2){

                    head = start_ptr2;

                }else{

                    head++;

                }

                return re;

            }



            CircularArray& operator=(const CircularArray&) = delete;

            CircularArray(const CircularArray&) = delete;

            ~CircularArray(){

                delete start_ptr1;

            }



        private:

            T* head;

            T* tail;

            T* start_ptr1, *start_ptr2;

            T* past_end_ptr1, *past_end_ptr2;

            int _capacity;

            int _size;

    };

}

test.txt

edited Nov 26 '18 at 9:23

asked Nov 20 '18 at 3:57

Rick

1,548929

Correct Code Link: https://wandbox.org/permlink/JYr2XoaSxsS1QT14

Here's what I've done:

I make a FIFO structure based on fixed size dynamic array. (ring buffer)

I have a pair of pointers for both push_right and pop_left operations. So there won't have data race problems, if I understand correctly.

I make the producer to write several items ahead, then consumer starts to read and need to ensure that:
- consumer reads speed <= producer writes speed （consumer read pointer < producer write pointer ）
- A appropriate fixed array size, so that the producer won't override items that consumer haven't read.

My problem is that the output result is not as I expected, it's not synchronized.. And I have no idea how to debug this.

You can see the data write order (P ostream:) is not the same as the read order (O ostream:).

One possible output:

data size: 20

70 927 156 109 834 26 883 576 226 500 904 777 935 80 346 559 846 879 548 791 

********************

Consumer start working

791

548

879

846

26

346

109

156

927

70

500

226

576

883

26

834

109

156

927

70

Consumer done

********************

p_count: 20

c_count: 20

P ostream: 791 548 879 846 559 346 80 935 777 904 500 226 576 883 26 834 109 156 927 70 

C ostream: 791 548 879 846 26 346 109 156 927 70 500 226 576 883 26 834 109 156 927 70

Code:

(It contains main.cpp, circular_array and a data txt file, so I
think visit the link would be less pain)

https://wandbox.org/permlink/ddQjNFdxABrjminQ

main.cpp

#include <iostream>

#include <thread>

#include <chrono>

#include <vector>

#include <fstream>

#include <sstream>

#include "circular_array.h"

using namespace ythlearn;

using namespace std;



int p_count = 0;

int c_count = 0;

int fileSize = 0;

ostringstream p_os, c_os;



void producer(CircularArray<int>* Ca, vector<int> &data){

    for(int i = 0; i < 5; i++){

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }

    while(!data.empty()){

        this_thread::sleep_for(chrono::seconds(1));    

        p_os << data.back() << " ";

        Ca->push_right(data.back());

        data.pop_back();

        p_count++;

    }



}



void consumer(CircularArray<int>* Ca){

    cout << "********************" << endl;

    cout << "Consumer start working" << endl;

    this_thread::sleep_for(chrono::seconds(5));

    while(c_count < fileSize){

        this_thread::sleep_for(chrono::seconds(1));    

        int re = Ca->pop_left();

        cout << re << endl;

        c_os << re << " ";

        c_count++;



    }

    cout << "Consumer done" << endl;

    cout << "********************" << endl;



}





void getInput(vector<int>& data){

    ifstream ifs("test.txt");

    int j;

    while(ifs >> j){

        data.push_back(j);

    }

}



int main(){

    cout << unitbuf;

    vector<int> data;

    getInput(data);



    CircularArray<int> Ca;



    ::fileSize = data.size();

    cout << "data size: " << ::fileSize << endl;

    for(const auto& s: data){

        cout << s << " ";

    }

    cout << endl;





    thread th_producer(producer, &Ca, std::ref(data));

    thread th_consumer(consumer, &Ca);



    th_consumer.join();

    th_producer.join();

    cout << "p_count: " << p_count << endl

         << "c_count: " << c_count << endl;

    cout << "P ostream: " << p_os.str() << endl;

    cout << "C ostream: " << c_os.str() << endl;

    return 0;

}

circular_array.h

#pragma once

#include <stdexcept>

#include <iostream>

namespace ythlearn{

    template<typename T>

    class CircularArray{

        public:

            CircularArray(int N = 10){

                head = tail = new T[N];

                past_end_ptr1 = past_end_ptr2 = head + N;

                start_ptr1 = start_ptr2 = head;

                _capacity = N;

                _size = 0;

            }



            void push_right(T elem){

                *tail = elem;

                if(tail + 1 == past_end_ptr1){

                    tail = start_ptr1;

                }else{

                    tail++;

                }

            }



            T pop_left(){

                T re = *head;

                if(head + 1 == past_end_ptr2){

                    head = start_ptr2;

                }else{

                    head++;

                }

                return re;

            }



            CircularArray& operator=(const CircularArray&) = delete;

            CircularArray(const CircularArray&) = delete;

            ~CircularArray(){

                delete start_ptr1;

            }



        private:

            T* head;

            T* tail;

            T* start_ptr1, *start_ptr2;

            T* past_end_ptr1, *past_end_ptr2;

            int _capacity;

            int _size;

    };

}

test.txt

c++ multithreading data-structures

edited Nov 26 '18 at 9:23

asked Nov 20 '18 at 3:57

Rick

1,548929

edited Nov 26 '18 at 9:23

asked Nov 20 '18 at 3:57

Rick

1,548929

edited Nov 26 '18 at 9:23

asked Nov 20 '18 at 3:57

Rick

1,548929

asked Nov 20 '18 at 3:57

Rick

1,548929

asked Nov 20 '18 at 3:57

Rick

1,548929

Post the code and output; don’t post screen shots. Have some consideration for the people you are asking for help from.

– mevets
Nov 20 '18 at 18:34

1

It seems like Boost.Lockfree library may have an out-of-the-box solution. Check this out: boost::lockfree::spsc_queue

– Julien Villemure-Fréchette
Nov 26 '18 at 16:51

add a comment |

Post the code and output; don’t post screen shots. Have some consideration for the people you are asking for help from.

– mevets
Nov 20 '18 at 18:34

1

It seems like Boost.Lockfree library may have an out-of-the-box solution. Check this out: boost::lockfree::spsc_queue

– Julien Villemure-Fréchette
Nov 26 '18 at 16:51

Post the code and output; don’t post screen shots. Have some consideration for the people you are asking for help from.

– mevets
Nov 20 '18 at 18:34

It seems like Boost.Lockfree library may have an out-of-the-box solution. Check this out: boost::lockfree::spsc_queue

– Julien Villemure-Fréchette
Nov 26 '18 at 16:51

add a comment |

1 Answer
1

active

oldest

votes

+150

Short version: The problem comes down to no implementation of the third thing you said you had done ("consumer read pointer < producer write pointer" and "the producer won't override items that consumer haven't read"). In particular, not checking for overwrite is being problematic. So good plan, not so good execution.

More details: You never check for the circular array being full. Since your test case lives on the border between the array being full and the array being overfull, you end up with a race condition. Sometimes the producer will overwrite the beginning of the array before the consumer reads it.

Here's a timeline (measured in seconds):

0: Producer writes 5 values to the array.
1: Producer writes a 6th value to the array.
2: Producer writes a 7th value to the array.
3: Producer writes an 8th value to the array.
4: Producer writes a 9th value to the array.
5: Producer writes a 10th value to the array (it's now at capacity), and Consumer starts its loop (but does not get far since the first step in each iteration is sleeping for a second).

5+n: Producer writes a value to the array, and Consumer reads a value from the array. If Producer goes first, the array's size temporarily goes up to 11, exceeding its capacity.

Look at the places where Consumer reads something different than what Producer wrote. Compare what Consumer read to what Producer wrote 10 steps later.

Now for the unasked-for general critique.

There are two aspects of your implementation of a circular array that look odd/wrong. First, there is the storage duplication, where you maintain two identical start pointers and two identical past-end pointers. Second, it looks like _size is always 0, which seems not helpful.

One aspect that is not necessarily wrong but that might be improvable is how you specify N. Have you considered making N a template parameter, similar to what was done for std::array? That could reduce your memory footprint (no need to store _capacity) and remove the need for dynamic memory management.

Addendum:

It has occurred to me that you have data members that should not be changed after construction, but that are not flagged const. You might want to address that, especially since flagging them const will make it clear that they cannot be involved in a race condition. So you could declare the circular array's data members more like:

private:

    int const _capacity;

    T * const start;

    T * const past_end;

    T* head;

    T* tail;

Then the constructor could be more like:

CircularArray(int N = 10) :

    _capacity(N),

    start(new T[N]),

    past_end(start + N)

{

    head = tail = start;

}

(Actually, you could use the initializer list for all the members; I just thought smaller changes would be more digestible.) Another style benefit of this is that the new and delete would both be applied to start, which looks better to someone checking the code.

Another simplification might be to use the expression start + N wherever you had been using past_end. The performance difference should be negligible, and you reduce the memory footprint.

Alternatively, my earlier suggestion of making N a template parameter would render the const question moot. Using N as a template parameter (still with a default value of 10), the circular array's data members could simply be:

private:

    T start[N]; // As a template parameter, N is a compile-time constant

    T* head; // Or an index into the array

    T* tail; // Or an index into the array

Switching to indices also gives more reason to drop the past-the-end member. The past-the-end index becomes N, which is a compile time constant -- no need to waste storage space on it.

edited Nov 26 '18 at 22:59

answered Nov 26 '18 at 4:52

JaMiT

1,267113

I was being so careless. I don't know why I made the consumer sleeps for 5 seconds. I delete it and everything works fine. Thank you so much. I will award the bounty when it's available.

– Rick
Nov 26 '18 at 8:49

I would also like to reply to some of your critique. 1. Yes, I don't tempt to check the circular array is full or not and overriding (data have already been read) is completely allowed. I just need to avoid data race while keep producer and consumer fully asynchronous. 2. I have 2 identical pointers because for both push_right and pop_left operations, I need to check tail + 1 == past_end_ptr or head + 1 == past_end_ptr, if so, then reset it. So in order to avoid data race, I think I need 2 identical of these pointers. 3. _size is indeed not useful here, I merely wrote it casually.

– Rick
Nov 26 '18 at 9:04

There is no race condition when two threads read from the same memory location. (Race conditions occur only when there is a write of some sort.) As long as you are not writing to the pointers, there is no need to maintain duplicate copies of them.

– JaMiT
Nov 26 '18 at 22:30

OK, I got it. Thanks for telling that ;D

– Rick
Nov 27 '18 at 1:52

add a comment |

Your Answer

StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});

}
});

draft saved

draft discarded

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53386000%2fdata-is-not-synchronized-when-try-to-implement-a-producer-consumer-ring-buffer%23new-answer', 'question_page');
}
);

Post as a guest

Name

Required, but never shown

1 Answer
1

active

oldest

votes

1 Answer
1

active

oldest

votes

+150

Here's a timeline (measured in seconds):

5+n: Producer writes a value to the array, and Consumer reads a value from the array. If Producer goes first, the array's size temporarily goes up to 11, exceeding its capacity.

Look at the places where Consumer reads something different than what Producer wrote. Compare what Consumer read to what Producer wrote 10 steps later.

Now for the unasked-for general critique.

private:

    int const _capacity;

    T * const start;

    T * const past_end;

    T* head;

    T* tail;

Then the constructor could be more like:

CircularArray(int N = 10) :

    _capacity(N),

    start(new T[N]),

    past_end(start + N)

{

    head = tail = start;

}

Another simplification might be to use the expression start + N wherever you had been using past_end. The performance difference should be negligible, and you reduce the memory footprint.

private:

    T start[N]; // As a template parameter, N is a compile-time constant

    T* head; // Or an index into the array

    T* tail; // Or an index into the array

Switching to indices also gives more reason to drop the past-the-end member. The past-the-end index becomes N, which is a compile time constant -- no need to waste storage space on it.

edited Nov 26 '18 at 22:59

answered Nov 26 '18 at 4:52

JaMiT

1,267113

I was being so careless. I don't know why I made the consumer sleeps for 5 seconds. I delete it and everything works fine. Thank you so much. I will award the bounty when it's available.

– Rick
Nov 26 '18 at 8:49

I would also like to reply to some of your critique. 1. Yes, I don't tempt to check the circular array is full or not and overriding (data have already been read) is completely allowed. I just need to avoid data race while keep producer and consumer fully asynchronous. 2. I have 2 identical pointers because for both push_right and pop_left operations, I need to check tail + 1 == past_end_ptr or head + 1 == past_end_ptr, if so, then reset it. So in order to avoid data race, I think I need 2 identical of these pointers. 3. _size is indeed not useful here, I merely wrote it casually.

– Rick
Nov 26 '18 at 9:04

There is no race condition when two threads read from the same memory location. (Race conditions occur only when there is a write of some sort.) As long as you are not writing to the pointers, there is no need to maintain duplicate copies of them.

– JaMiT
Nov 26 '18 at 22:30

OK, I got it. Thanks for telling that ;D

– Rick
Nov 27 '18 at 1:52

add a comment |

+150

Here's a timeline (measured in seconds):

5+n: Producer writes a value to the array, and Consumer reads a value from the array. If Producer goes first, the array's size temporarily goes up to 11, exceeding its capacity.

Look at the places where Consumer reads something different than what Producer wrote. Compare what Consumer read to what Producer wrote 10 steps later.

Now for the unasked-for general critique.

private:

    int const _capacity;

    T * const start;

    T * const past_end;

    T* head;

    T* tail;

Then the constructor could be more like:

CircularArray(int N = 10) :

    _capacity(N),

    start(new T[N]),

    past_end(start + N)

{

    head = tail = start;

}

Another simplification might be to use the expression start + N wherever you had been using past_end. The performance difference should be negligible, and you reduce the memory footprint.

private:

    T start[N]; // As a template parameter, N is a compile-time constant

    T* head; // Or an index into the array

    T* tail; // Or an index into the array

Switching to indices also gives more reason to drop the past-the-end member. The past-the-end index becomes N, which is a compile time constant -- no need to waste storage space on it.

edited Nov 26 '18 at 22:59

answered Nov 26 '18 at 4:52

JaMiT

1,267113

I was being so careless. I don't know why I made the consumer sleeps for 5 seconds. I delete it and everything works fine. Thank you so much. I will award the bounty when it's available.

– Rick
Nov 26 '18 at 8:49

I would also like to reply to some of your critique. 1. Yes, I don't tempt to check the circular array is full or not and overriding (data have already been read) is completely allowed. I just need to avoid data race while keep producer and consumer fully asynchronous. 2. I have 2 identical pointers because for both push_right and pop_left operations, I need to check tail + 1 == past_end_ptr or head + 1 == past_end_ptr, if so, then reset it. So in order to avoid data race, I think I need 2 identical of these pointers. 3. _size is indeed not useful here, I merely wrote it casually.

– Rick
Nov 26 '18 at 9:04

There is no race condition when two threads read from the same memory location. (Race conditions occur only when there is a write of some sort.) As long as you are not writing to the pointers, there is no need to maintain duplicate copies of them.

– JaMiT
Nov 26 '18 at 22:30

OK, I got it. Thanks for telling that ;D

– Rick
Nov 27 '18 at 1:52

add a comment |

+150

Here's a timeline (measured in seconds):

5+n: Producer writes a value to the array, and Consumer reads a value from the array. If Producer goes first, the array's size temporarily goes up to 11, exceeding its capacity.

Look at the places where Consumer reads something different than what Producer wrote. Compare what Consumer read to what Producer wrote 10 steps later.

Now for the unasked-for general critique.

private:

    int const _capacity;

    T * const start;

    T * const past_end;

    T* head;

    T* tail;

Then the constructor could be more like:

CircularArray(int N = 10) :

    _capacity(N),

    start(new T[N]),

    past_end(start + N)

{

    head = tail = start;

}

Another simplification might be to use the expression start + N wherever you had been using past_end. The performance difference should be negligible, and you reduce the memory footprint.

private:

    T start[N]; // As a template parameter, N is a compile-time constant

    T* head; // Or an index into the array

    T* tail; // Or an index into the array

Switching to indices also gives more reason to drop the past-the-end member. The past-the-end index becomes N, which is a compile time constant -- no need to waste storage space on it.

edited Nov 26 '18 at 22:59

answered Nov 26 '18 at 4:52

JaMiT

1,267113

Here's a timeline (measured in seconds):

5+n: Producer writes a value to the array, and Consumer reads a value from the array. If Producer goes first, the array's size temporarily goes up to 11, exceeding its capacity.

Look at the places where Consumer reads something different than what Producer wrote. Compare what Consumer read to what Producer wrote 10 steps later.

Now for the unasked-for general critique.

private:

    int const _capacity;

    T * const start;

    T * const past_end;

    T* head;

    T* tail;

Then the constructor could be more like:

CircularArray(int N = 10) :

    _capacity(N),

    start(new T[N]),

    past_end(start + N)

{

    head = tail = start;

}

Another simplification might be to use the expression start + N wherever you had been using past_end. The performance difference should be negligible, and you reduce the memory footprint.

private:

    T start[N]; // As a template parameter, N is a compile-time constant

    T* head; // Or an index into the array

    T* tail; // Or an index into the array

Switching to indices also gives more reason to drop the past-the-end member. The past-the-end index becomes N, which is a compile time constant -- no need to waste storage space on it.

edited Nov 26 '18 at 22:59

answered Nov 26 '18 at 4:52

JaMiT

1,267113

edited Nov 26 '18 at 22:59

answered Nov 26 '18 at 4:52

JaMiT

1,267113

answered Nov 26 '18 at 4:52

JaMiT

1,267113

answered Nov 26 '18 at 4:52

JaMiT

1,267113

I was being so careless. I don't know why I made the consumer sleeps for 5 seconds. I delete it and everything works fine. Thank you so much. I will award the bounty when it's available.

– Rick
Nov 26 '18 at 8:49

I would also like to reply to some of your critique. 1. Yes, I don't tempt to check the circular array is full or not and overriding (data have already been read) is completely allowed. I just need to avoid data race while keep producer and consumer fully asynchronous. 2. I have 2 identical pointers because for both push_right and pop_left operations, I need to check tail + 1 == past_end_ptr or head + 1 == past_end_ptr, if so, then reset it. So in order to avoid data race, I think I need 2 identical of these pointers. 3. _size is indeed not useful here, I merely wrote it casually.

– Rick
Nov 26 '18 at 9:04

There is no race condition when two threads read from the same memory location. (Race conditions occur only when there is a write of some sort.) As long as you are not writing to the pointers, there is no need to maintain duplicate copies of them.

– JaMiT
Nov 26 '18 at 22:30

OK, I got it. Thanks for telling that ;D

– Rick
Nov 27 '18 at 1:52

add a comment |

I was being so careless. I don't know why I made the consumer sleeps for 5 seconds. I delete it and everything works fine. Thank you so much. I will award the bounty when it's available.

– Rick
Nov 26 '18 at 8:49

I would also like to reply to some of your critique. 1. Yes, I don't tempt to check the circular array is full or not and overriding (data have already been read) is completely allowed. I just need to avoid data race while keep producer and consumer fully asynchronous. 2. I have 2 identical pointers because for both push_right and pop_left operations, I need to check tail + 1 == past_end_ptr or head + 1 == past_end_ptr, if so, then reset it. So in order to avoid data race, I think I need 2 identical of these pointers. 3. _size is indeed not useful here, I merely wrote it casually.

– Rick
Nov 26 '18 at 9:04

There is no race condition when two threads read from the same memory location. (Race conditions occur only when there is a write of some sort.) As long as you are not writing to the pointers, there is no need to maintain duplicate copies of them.

– JaMiT
Nov 26 '18 at 22:30

OK, I got it. Thanks for telling that ;D

– Rick
Nov 27 '18 at 1:52

I was being so careless. I don't know why I made the consumer sleeps for 5 seconds. I delete it and everything works fine. Thank you so much. I will award the bounty when it's available.

– Rick
Nov 26 '18 at 8:49

I would also like to reply to some of your critique. 1. Yes, I don't tempt to check the circular array is full or not and overriding (data have already been read) is completely allowed. I just need to avoid data race while keep producer and consumer fully asynchronous. 2. I have 2 identical pointers because for both push_right and pop_left operations, I need to check tail + 1 == past_end_ptr or head + 1 == past_end_ptr, if so, then reset it. So in order to avoid data race, I think I need 2 identical of these pointers. 3. _size is indeed not useful here, I merely wrote it casually.

– Rick
Nov 26 '18 at 9:04

There is no race condition when two threads read from the same memory location. (Race conditions occur only when there is a write of some sort.) As long as you are not writing to the pointers, there is no need to maintain duplicate copies of them.

– JaMiT
Nov 26 '18 at 22:30

OK, I got it. Thanks for telling that ;D

– Rick
Nov 27 '18 at 1:52

add a comment |

draft saved

draft discarded

Thanks for contributing an answer to Stack Overflow!

Please be sure to answer the question. Provide details and share your research!

But avoid …

Asking for help, clarification, or responding to other answers.

Making statements based on opinion; back them up with references or personal experience.

To learn more, see our tips on writing great answers.

draft saved

draft discarded

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Post as a guest

Name

Required, but never shown

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Name

Required, but never shown

Name

Required, but never shown

This page is only for reference, If you need detailed information, please check here

Search This Blog

Ufyukyu