Yue Wang
Yue Wang

Reputation: 11

how to correct the following producer consumer code for c++11

I am writing a consumer/producer class with a simple semaphore implementation for c++11. However, the following code fails to compile. If I remove the producer_consumer class and make producer and consumer global functions, and move thread creation part (std::thread t1(consumer), t2(consumer); t1.join(); t2.join();)to the main function, it compiles but the implementation is still not correct and will eventually lead into segmentation fault. How can I correct the code? thanks.

#include <iostream>
#include <thread>
#include <mutex>

class semaphore {
private:
    std::mutex m;
    std::condition_variable cond;
    unsigned long n;
public:
    semaphore(int i) : n(i) {}

    void up() {
        std::unique_lock <std::mutex> lock (m);
        ++n;
        cond.notify_one();
    }

    void down() {
        std::unique_lock <std::mutex> lock (m);
        while(!n) cond.wait(lock);
        --n;
    }
};
class producer_consumer{
private:
    semaphore full, empty;
    int i = 0;
    std::thread t1, t2;
public:
    producer_consumer(int n): full(0), empty(n), i(0){}
    void run(){
        t1 = std::thread(&producer_consumer::producer, *this); 
        t2 = std::thread(&producer_consumer::consumer, *this);
    }
    void stop(){
        t1.join();
        t2.join();
    }
    void producer (){
        while (true){
            empty.down();
            i ++;
            std::cout << "[p]" << i << std::endl;
            full.up();  
        }
    }
    void consumer (){
        while (true){
            full.down();
            i --;
            std::cout << "[c]" << i << std::endl;
            empty.up(); 
        }
    }
};

int main(){
    producer_consumer pc(5);
    pc.run();
    pc.stop();
    return 0;
}

I use clang++ to compile the file:

clang++ -std=c++0x -stdlib=libc++ pc.cpp ; ./a.out

error message:

In file included from file_name.cpp:1:
In file included from /usr/bin/../lib/c++/v1/iostream:38:
In file included from /usr/bin/../lib/c++/v1/ios:216:
In file included from /usr/bin/../lib/c++/v1/__locale:15:
In file included from /usr/bin/../lib/c++/v1/string:434:
In file included from /usr/bin/../lib/c++/v1/algorithm:591:
/usr/bin/../lib/c++/v1/type_traits:1423:12: error: call to implicitly-deleted
      copy constructor of 'typename decay<producer_consumer &>::type'
      (aka 'producer_consumer')
    return _VSTD::forward<_Tp>(__t);
           ^~~~~~~~~~~~~~~~~~~~~~~~

Update: @DanqiWang solved the compilation problem by changing *this to this. Now it seems that the semaphore is not working properly and finally will crash the program:

./a.out
[p]1
[p]2
[p]3
[p]4
[p]5
....
[p]2
[p]3
[p]4
1
[c]3
[p[]c3]
3
[[c]3
p][2c
][p]3
[p]4
4
[c]3
[c]2
Segmentation fault: 11

Upvotes: 0

Views: 1548

Answers (1)

Danqi Wang
Danqi Wang

Reputation: 1637

I have the following suggestions for your code:

  • Although iostream becomes thread-safe in C++11, it should be locked to avoid mangled output.
  • use std::atomic instead of naked int.
  • If there is a segment fault, dump a core, use gdb to debug it, or post the stack trace here.

Hope these help.

Upvotes: 3

Related Questions