CODEWITHSUNDEEP
c++multithreading
Chris
Chris

Reputation: 568

Second thread is never triggered

I've been struggling with a multithreading issue for a bit. I've written some simple code to try and isolate the issue and I'm not finding it. What's happening is that the first thread is being woken up with data being sent to it, but second one never does. They each have their own condition_variable yet it doesn't seem to matter. Ultimately, what I'm trying to do is have a few long running threads that do a single dedicated task when needed, and staying in a wait state when not needed. And running them each in their own thread is important and a requirement.

Here's the code:

#include <glib.h>
#include <string>
#include <mutex>
#include <condition_variable>
#include <unistd.h>

#define NUM_THREADS 2

bool DEBUG = true;

pthread_t threads[NUM_THREADS];
std::mutex m_0;
std::mutex m_1;
std::condition_variable cov_0;
std::condition_variable cov_1;
bool dataReady_0 = false;
bool dataReady_1 = false;
bool keepRunning[NUM_THREADS] = { true };

void date_update (guint source_id, const char *json_data) {
    if (DEBUG) {
        start_threads(2);
        sleep(2);
        DEBUG = false;
    }

    g_print("From source id=%d\n", source_id);

    switch (source_id) {
        case 0:
            dataReady_0 = true;
            cov_0.notify_one();
            break;
        case 1:
            dataReady_1 = true;
            cov_1.notify_one();
            break;
    }
}

void start_threads (int thread_count) {
    int rc;
    switch (thread_count) {
        case 2:
            rc = pthread_create(&threads[1], nullptr, custom_thread_1, nullptr);
            if (rc) {
                g_print("Error:unable to create thread(1), return code(%d)\n", rc);
            }
        case 1:
            rc = pthread_create(&threads[0], nullptr, custom_thread_0, nullptr);
            if (rc) {
                g_print("Error:unable to create thread(0), return code(%d)\n", rc);
            }
    }

}

void *custom_thread_0 (void *pVoid) {
    g_print("Created thread for source id=0\n");
    while (keepRunning[0]) {
        // Wait until date_update() sends data
        std::unique_lock<std::mutex> lck(m_0);
        cov_0.wait(lck, [&]{return dataReady_0;});
        dataReady_0 = false;

        g_print("THREAD=0, DATA RECEIVED\n");

        lck.unlock();
    }
    pthread_exit(nullptr);
}

void *custom_thread_1 (void *pVoid) {
    g_print("Created thread for source id=1\n");
    while (keepRunning[1]) {
        // Wait until date_update() sends data
        std::unique_lock<std::mutex> lck(m_1);
        cov_1.wait(lck, [&]{return dataReady_1;});
        dataReady_1 = false;

        g_print("THREAD=1, DATA RECEIVED\n");

        lck.unlock();
    }
    pthread_exit(nullptr);
}

Here's the output. As you can see the data_update function gets the "data" from the calling function for both source 0 and source 1, but only thread 0 ever seems to process anything. I'm at a bit of a loss as to the source of the problem.

Sending data for source id=1
From source id=1
Sending data for source id=0
From source id=0
THREAD=0, DATA RECEIVED
Sending data for source id=1
From source id=1
Sending data for source id=0
From source id=0
THREAD=0, DATA RECEIVED
Sending data for source id=1
From source id=1
Sending data for source id=0
From source id=0
THREAD=0, DATA RECEIVED

I'm sure I'm just missing a minor detail somewhere, but I'm fully willing to accept that perhaps I do not understand C/C++ threading correctly.

Upvotes: 1

Views: 215

Answers (2)

TrentP
TrentP

Reputation: 4682

The 2nd thread is exiting because the keepRunning state flag is false. It's usually a good first step in debugging threads to log the start and exit of all threads.

But you have a much less obvious problem.

It does not appear that the appropriate mutex is held when the value of the condition variable's predicate is changed in date_update().

I'll break that down a bit more.

When cov_0.wait() is called, the predicate used is [&]{return dataReady_0;} (*), and the unique_lock passed is holding the mutex m_0. This means that whenever the value of the predicate might change, the mutex m_0 must be held.

This predicate is quite simple and will change value whenever the global variable dataReady_0 changes value.

In date_update() there is code to change the value of dataReady_0 and the mutex m_0 is not held when doing this. There should be a scoped_lock or unique_lock in the block that changes the global variable's state.

It will still mostly work without this, but you have a race! It will fail eventually!

The condition variable may check and see that the predicate is false, then another thread changes the predicate's value and does a notify, and then the first thread waits on the condition variable. It misses the notify because it was not yet waiting when it was sent. The use of the mutex to prevent the predicate from changing in a way that races with the notification is a critical component of what makes this work.

(*) You don't need the capture [&] here. This lambda could be stateless.

Upvotes: 1

Alireza Abbasi
Alireza Abbasi

Reputation: 190

You should initialize all elements of the built-in array:

bool keepRunning[2] = { true, true };

Upvotes: 1

Related Questions