Threading returns unexpected result

I'm learning about threads for homework, and I've tried to implement threading on a simple program I've made. Without threading the program works perfectly, but when I thread the two random number generator functions, it returns incorrect results. The result always seems to be '42' for both number generators, not sure why this would be the case.

Also for context, I'm just starting with threads so I understand this program doesn't need multithreading. I'm doing it just for learning purposes.

Thanks for any help!

// struct for vector to use
struct readings {
    std::string name;
    int data;
};

// random generator for heat value - stores in vector of struct
void gen_heat(std::vector<readings>& storage) {
    readings h = {"Heat", rand() % 100   1};
    storage.insert(storage.begin(), h);
}

// random generator for light value - stores in vector of struct
void gen_light(std::vector<readings>& storage) {
    readings l = {"Light", rand() % 100   1};
    storage.insert(storage.begin(), l);
}

int main() {
    // vector of readings struct
    std::vector<readings> storage;

    srand(time(NULL));

    // initialising threads of random generators
    std::thread H(gen_heat, std::ref(storage));
    std::thread L(gen_light, std::ref(storage));

    // waiting for both to finish
    H.join();
    L.join();

    // print values in vec of struct
    for (const auto& e : storage) {
        std::cout << "Type: " << e.name << std::endl
                  << "Numbers:  " << e.data << std::endl;
    }

    // send to another function
    smartsensor(storage);

    return 0;
}

CodePudding user response：

Since you have several threads accessing a mutual resource, in this case the vector of readings, and some of them are modifying it, you need to make the accesses to that resource exclusive. There are many ways of synchronizing the access; one of them, simple enough and not going down to the use of mutexes, is a binary semaphore (since C 20). You basically:

own the access to the resource by acquiring the semaphore,
use the resource, and then,
release the semaphore so others can access the resource.

If a thread A tries to acquire the semaphore while other thread B is using the resource, thread A will block until the resource is freed.

Notice the semaphore is initialized to 1 indicating the resource is free. Once a thread acquires the semaphore, the count will go down to 0, and no other thread will be able to acquire it until the count goes back to 1 (what will happen after a release).

[Demo]

#include <cstdlib>  // rand
#include <iostream>  // cout
#include <semaphore>
#include <string>
#include <thread>
#include <vector>

std::binary_semaphore readings_sem{1};

// struct for vector to use
struct readings {
    std::string name;
    int data;
};

// random generator for heat value - stores in vector of struct
void gen_heat(std::vector<readings>& storage) {
    for (auto i{0}; i < 5;   i) {
        readings_sem.acquire();
        readings h = {"Heat", rand() % 100   1};
        storage.insert(storage.begin(), h);
        readings_sem.release();
    }
}

// random generator for light value - stores in vector of struct
void gen_light(std::vector<readings>& storage) {
    for (auto i{0}; i < 5;   i) {
        readings_sem.acquire();
        readings l = {"Light", rand() % 100   1};
        storage.insert(storage.begin(), l);
        readings_sem.release();
    }
}

int main() {
    // vector of readings struct
    std::vector<readings> storage;

    srand(time(NULL));

    // initialising threads of random generators
    std::thread H(gen_heat, std::ref(storage));
    std::thread L(gen_light, std::ref(storage));

    // waiting for both to finish
    H.join();
    L.join();

    // print values in vec of struct
    for (const auto& e : storage) {
        std::cout << "Type: " << e.name << std::endl
            << "Numbers:  " << e.data << std::endl;
    }
}

// Outputs (something like):
//
//   Type: Heat
//   Numbers:  5
//   Type: Light
//   Numbers:  83
//   Type: Light
//   Numbers:  40
//   ...

[Update on Ben Voigt's comment]

The acquisition and release of the resource can be encapsulated by using RAII (Resource Acquisition Is Initialization), a mechanism which is already provided by the language. E.g.:

Both threads still try and acquire a mutex to get access to the vector of readings resource.
But they acquire it by just creating a lock guard.
Once the lock guard goes out of scope and is destroyed, the mutex is released.

[Demo]

#include <mutex>  // lock_guard

std::mutex mtx{};

// random generator for heat value - stores in vector of struct
void gen_heat(std::vector<readings>& storage) {
    for (auto i{0}; i < 5;   i) {
        std::lock_guard<std::mutex> lg{ mtx };
        readings h = {"Heat", rand() % 100   1};
        storage.insert(storage.begin(), h);
    }
}