CODEWITHSUNDEEP
c++multithreading
Seph
Seph

Reputation: 1094

c++ thread does not execute

The thread1 function does not seem to get executed

   #include <iostream>
   #include <fstream>
   #include <thread>
   #include <condition_variable>
   #include <queue>

   std::condition_variable cv;
   std::mutex mu;
   std::queue<int> queue;
   bool ready;

    static void thread1() {
        while(!ready) {std::this_thread::sleep_for(std::chrono::milliseconds(10));}

        while(ready && queue.size() <= 4) {
                std::unique_lock<std::mutex> lk(mu);
                cv.wait(lk, [&]{return !queue.empty();});

                queue.push(2);
        }
    }

    int main() {
        ready = false;
        std::thread t(thread1);

        while(queue.size() <= 4) {
            {
                std::lock_guard<std::mutex> lk(mu);
                queue.push(1);
            }

            ready = true;
            cv.notify_one();
        }

        t.join();

        for(int i = 0; i <= queue.size(); i++) {
                int a = queue.front();
                std::cout << a << std::endl;
                queue.pop();
        }

        return 0;
}

On my Mac the output is 1 2 1 2 but in my ubuntu its 1 1 1. I'm compiling with g++ -std=c++11 -pthread -o thread.out thread.cpp && ./thread.out. Am I missing something?

Upvotes: 3

Views: 1873

Answers (2)

Seph
Seph

Reputation: 1094

I was able to solve this by making the second thread wait on a separate predicate on a separate conditional variable. I'm not sure if queue.size() is thread safe.

#include <iostream>
#include <fstream>
#include <thread>
#include <condition_variable>
#include <queue>

std::condition_variable cv;
std::condition_variable cv2;
std::mutex mu;
std::queue<int> queue;
bool tick;
bool tock;

static void thread1() {
while(queue.size() < 6) {
    std::unique_lock<std::mutex> lk(mu);
    cv2.wait(lk, []{return tock;});

    queue.push(1);

    tock = false;
    tick = true;
    cv.notify_one();
}
}

int main() {
tick = false;
tock = true;
std::thread t(thread1);

while(queue.size() < 6) {
    std::unique_lock<std::mutex> lk(mu);
    cv.wait(lk, []{return tick;});

    queue.push(2);

    tick = false;
    tock = true;
    cv2.notify_one();
}

t.join();

while(!queue.empty()) {
    int r = queue.front();
    queue.pop();

    std::cout << r << std::endl;
}

return 0;
}

Upvotes: 0

Nir Friedman
Nir Friedman

Reputation: 17704

This:

for(int i = 0; i <= queue.size(); i++) {
    int a = queue.front();
    std::cout << a << std::endl;
    queue.pop();
}

Is undefined behavior. A for loop that goes from 0 to size runs size+1 times. I would suggest that you write this in the more idiomatic style for a queue:

while(!queue.empty()) {
    int a = queue.front();
    std::cout << a << std::endl;
    queue.pop();
}

When I run this on coliru, which I assume runs some kind of *nix machine, I get 4 1's: http://coliru.stacked-crooked.com/a/8de5b01e87e8549e.

Again, you haven't specified anything that would force each thread to run a certain amount of times. You only (try to*) cause an invariant where the queue will reach size 4, either way. It just happens to be that on the machines that we ran it on, thread 2 never manages to acquire the mutex.

This example will be more interesting if you add more work or even (just for pedagogical purposes) delays at various points. Simulating that the two threads are actually doing work. If you add sleeps at various points you can ensure that the two threads alternate, though depending where you add them you may see your invariant of 4 elements in the thread break!

*Note that even your 4 element invariant on the queue, is not really an invariant. It is possible (though very unlikely) that both threads pass the while condition at the exact same moment, when there are 3 elements in the queue. One acquires the lock first and pushes, and then the other. So you can end up with 5 elements in the queue! (as you can see, asynchronous programming is tricky). In particular you really need to check the queue size when you have the lock in order for this to work.

Upvotes: 2

Related Questions