using c++ packaged_task to construct producer-consumer pattern

Question

I am trying to packaged_task to create a producer-consumer patten code is as following : test_thread9_producer1 and test_thread9_producer2 push task into a queue and test_thread9_consumer1 retrieve task from the queue to execute

However when running test_thread9, it executes task correctly but finished with debug error : abort has been called. I'm not sure why ? Can anyone help me get more understanding of packaged_task?

A second issue: consumer is running with while(1) loop, I can not think of graceful way to let test_thread9_consumer1 exit when two producers finished pushing all tasks into queue and test_thread9_consumer1 finished executing all tasks in the queue. Can anyone give me some suggestion ?

void test_thread9()
{
    std::thread t1(test_thread9_producer1);
    std::thread t2(test_thread9_producer2);
    std::thread t3(test_thread9_consumer1);

    t1.join();
    t2.join();
    t3.join();
} 

std::deque<std::packaged_task<int()>>task_q;
std::mutex lock9;

int factial_calc2(int in)
{
    int ret = 1;
    for (int i = in; i > 1; i--)
    {
        ret = ret*i;
    }
    std::lock_guard<std::mutex> locker(lock9);
    std::cout << "input is " << in << "result is " << ret << std::endl;
    return ret;
}

void test_thread9_producer1()
{
    for (int i = 0; i < 10; i = i + 2)
    {
        std::lock_guard<std::mutex> locker(lock9);
        std::packaged_task<int()> t1(std::bind(factial_calc2, i));
        task_q.push_back(std::move(t1));
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
}

void test_thread9_producer2()
{
    for (int i = 1; i < 10; i = i + 2)
    {
        std::lock_guard<std::mutex> locker(lock9);
        std::packaged_task<int()> t1(std::bind(factial_calc2, i));
        task_q.push_back(std::move(t1));
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
}


void test_thread9_consumer1()
{
    std::packaged_task<int()>t;
    while (1)
    {
        {
            std::lock_guard<std::mutex> locker(lock9);
            if (!task_q.empty())
            {
                t = std::move(task_q.front());
                task_q.pop_front();
            }
        }
        t();
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
}

When you ran it in the debugger, where did the program stop? — kfsone, Jul 17 '16 at 18:01

Christophe · Answer 1 · 2016-07-17T18:38:21.623

Why does it crash ?

If your consumer thread finds an empty queue, it will nevertheless try to execute the packaged task despite it was not moved. This is UB and hence the runtime error !

You can improve this by checking if the packaged_task is valid:

while (1)
{
    std::packaged_task<int()>t;  // to make sure that valid() checks this iteration
    {
       ...
    }
    if (t.valid())
        t();  // execute only if it's a valid task
    ...
}

How to avoid endless looping ?

You have somehow to keep track of what's running. A simple technique is to used an atomic variable to manage a shared state information (which can be accessed concurrently without locking).

For example you could count the nimber of finished producers

std::atomic<int>finished{0};  // count the producers that are finished
...

void test_thread9_producerN() { cout <<"start producer"<

Then you can adapt your consumer to take inte account this information:

void test_thread9_consumer1()
{
    bool nothing_to_do{false}; 
    while (!nothing_to_do && finished<2)
    {
    ...   
        nothing_to_do=task_q.empty();  // in the lock protected section 
        if (!nothing_to_do)     
    ...
    }
}

Online demo

score 0 · Answer 2 · answered Jul 17 '16 at 18:55

void test_thread9_consumer1()
{
    std::packaged_task<int()>t;
    while (1)
    {
        {
            std::lock_guard<std::mutex> locker(lock9);
            if (!task_q.empty())
            {
                t = std::move(task_q.front());
                task_q.pop_front();
            }
        }
        t();
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
}

Lets trim this code down:

    std::packaged_task<int()>t;
    while (1)
    {
            if (!task_q.empty())
                t = std::move(task_q.front());
        t();
    }

Now we can clearly see the error: you try to call t() regardless of whether or not we fetched it.

void test_thread9_consumer1()
{
    std::packaged_task<int()>t;
    while (1)
    {
        std::this_thread::sleep_for(std::chrono::milliseconds(100));

        {
            std::lock_guard<std::mutex> locker(lock9);
            if (task_q.empty())
                continue;
            t = std::move(task_q.front());
            task_q.pop_front();
        }

        t();
    }
}

You may want to continue your exploration of threads by looking at "condition variables"

For the second part of your question, you could consider changing the main thread to join all of the providers and then set a global flag indicating the work is done.

std::atomic g_producing;

void test_thread9()
{
    std::thread t1(test_thread9_producer1);
    std::thread t2(test_thread9_producer2);
    g_producing = true;

    std::thread t3(test_thread9_consumer1);

    t1.join();
    t2.join();

    g_producing = false;

    t3.join();
} 

std::deque<std::packaged_task<int()>>task_q;
std::mutex lock9;

int factial_calc2(int in)
{
    int ret = 1;
    for (int i = in; i > 1; i--)
    {
        ret = ret*i;
    }
    std::lock_guard<std::mutex> locker(lock9);
    std::cout << "input is " << in << "result is " << ret << std::endl;
    return ret;
}

void test_thread9_producer1()
{
    for (int i = 0; i < 10; i = i + 2)
    {
        std::lock_guard<std::mutex> locker(lock9);
        std::packaged_task<int()> t1(std::bind(factial_calc2, i));
        task_q.push_back(std::move(t1));
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
}

void test_thread9_producer2()
{
    for (int i = 1; i < 10; i = i + 2)
    {
        std::lock_guard<std::mutex> locker(lock9);
        std::packaged_task<int()> t1(std::bind(factial_calc2, i));
        task_q.push_back(std::move(t1));
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
}

void test_thread9_consumer1()
{
    std::packaged_task<int()>t;
    for (;;)
    {
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
        {
            std::lock_guard<std::mutex> locker(lock9);
            if (task_q.empty()) {
                if (!g_producing)
                    break;
                continue;
            }
            t = std::move(task_q.front());
            task_q.pop_front();
        }
        t();
    }
}

using c++ packaged_task to construct producer-consumer pattern

2 Answers2