0

I am trying to implement queue to synchronize producer-consumer problem. However I cannot figure out why my solution deadlocks.

What it should do: Queue with fixed size containing tasks for consumer threads, producers should be blocked until there is space in the queue. (m producers, n consumers)

Technical requirements: POSIX threads (no C++11)

   void taskProducer(ArData &a) {
        while (true) {
            Work work = a.workGenerator();
            if (work == NULL) {
                a.queue.finished(); 
                return;
            }
            WorkTask task(work);
            a.queue.push(task);
        }
    }

    void consumer(TaskQueue &queue) {
        while (true) {
            Task task = queue.pop(); 
            if (task.end) return;
            task.run();
        }
    }

    #define NUM_OF_PRODUCERS 2

    //Task ktery muze vlakno zpracovat
    class Task {
    public:
        Task() : end(false){ }

        //Execute task
        virtual void run() { }

        //Mark task as ending Task, consumer should exit thread
        bool end;
    };

    //Synchronous queue
    class TaskQueue {
    private:
        //Number of consumer threads
        int numOfConsumers;

        //Number of producers that finished producing
        int doneProducers;

        //Producers wait, Consumers post
        sem_t producerNotificator;

        //Consumers wait, Producers post
        sem_t consumerNotificator;

        //Mutex for entire TaskQueue
        pthread_mutex_t mut;

        //Data storage
        std::queue<Task> queue;

    public:
        explicit TaskQueue(int m_numOfConsumers) : numOfConsumers(m_numOfConsumers), doneProducers(0) {
            pthread_mutex_t mut;
            pthread_mutex_init(&mut, NULL);

            sem_init(&consumerNotificator, 0, 0);
            sem_init(&producerNotificator, 0, numOfConsumers *10);
        }

        ~TaskQueue() {
            pthread_mutex_destroy(&mut);
            sem_destroy(&consumerNotificator);
            sem_destroy(&producerNotificator);
        }

        //Waits for empty slot in queue before pushing
        void push(Task &task) {

            //Wait for slot in queue
            sem_wait(&producerNotificator);

            //Lock before any manipulation
            pthread_mutex_lock(&mut);

            queue.push(task);

            pthread_mutex_unlock(&mut);

            //Notify consumer of waiting Task
            sem_post(&consumerNotificator);
        }

        //Waits before item is in queue and pops it
        Task pop() {

            //Wait for Task in queue
            sem_wait(&consumerNotificator);

            //Lock before any manipulation
            pthread_mutex_lock(&mut);


            Task task = queue.front();
            queue.pop();

            pthread_mutex_unlock(&mut);

            //Notify producer about empty slot in queue
            sem_post(&producerNotificator);

            return task;
        }

        //Handle finishing producers
        void finished() {
            //Lock before any manipulation
            pthread_mutex_lock(&mut);

            //Check if it is not last producer
            if (NUM_OF_PRODUCERS > ++doneProducers) {
                pthread_mutex_unlock(&mut);
                return;
            }


            //If it was last producer end consumers by adding end tasks into queue
            for (int i = 0; i < numOfConsumers; ++i) {
                Task t;
                t.end = true;
                queue.push(t);
            }
            pthread_mutex_unlock(&mut);


            //Notify all consumers about new Tasks
            for (int i = 0; i < numOfConsumers; ++i) {
                sem_post(&consumerNotificator);
            }
        }
    };

As far as I can tell, when second producer calls finished, it deadlocks at pthread_mutex_lock(&mut);. No idea why.

Jindra
  • 780
  • 13
  • 39
  • 3
    `pthread_mutex_t mut;` - in your constructor hides the member variable of the same name, which remains uninitialized. That is probably not intended. – WhozCraig Mar 29 '16 at 16:20
  • To prevent such a name clash, it's a good idea to use special names for private member variables. I prefer to use the underscore postfix here, e.g., `pthread_mutex_t mut_;`. – Daniel Langr Mar 29 '16 at 16:24
  • Thank you, that was real silly mistake. Otherwise is the code correct? – Jindra Mar 29 '16 at 17:39
  • If you create an answer from your comment I will accept it. – Jindra Dec 18 '18 at 13:36

0 Answers0