Why does add function have no effect in c++ 11 thread?

Question

I am trying to learn the c++ 11 thread and have following code :

#include <iostream>
#include <thread>
#include <vector>
#include <mutex>
#include <algorithm>

void add(int&  i){
    std::mutex some_mutex;
   // std::cout << " I am " << std::endl;
    std::lock_guard<std::mutex> guard(some_mutex); 
    i++;
}


int main(){
    int i = 0;
    std::vector<std::thread> vec_threads;

    for(int i = 0; i < 10; i++){
        vec_threads.push_back(std::thread(add,std::ref(i)));
    }

    std::for_each(vec_threads.begin(),vec_threads.end(),
            std::mem_fn(&std::thread::join));
    std::cout<< " i = " << i << std::endl;
return 0;
}

I have created a vector that holds std::thread and I call the add function from each thread and pass i by ref. After what I assumed that the thread would do (the adding of i = i+1), the end result does not reflect what I wanted.

output: i = 0

expected output: i = 10

@Ajay but user1887915 was also wright about my mutex being created for each thread ... which was terrible idea. — pokche, Jun 24 '16 at 05:28

user1887915 · Accepted Answer · 2016-06-24T04:38:38.080

28

Mutex needs to be shared between threads to get the correct result. And ibeing shadowed by loop variable, replace it with j.

#include <iostream>
#include <thread>
#include <vector>
#include <mutex>
#include <algorithm>

void add(int&  i, std::mutex &some_mutex){
   // std::cout << " I am " << std::endl;
    std::lock_guard<std::mutex> guard(some_mutex); 
    i++;
}


int main(){
    int i = 0;
    std::vector<std::thread> vec_threads;
    std::mutex some_mutex;

    for(int j = 0; j < 10; j++){
        vec_threads.push_back(std::thread(add,std::ref(i), std::ref(some_mutex)));
    }

    std::for_each(vec_threads.begin(),vec_threads.end(),
            std::mem_fn(&std::thread::join));
    std::cout<< " i = " << i << std::endl;
    return 0;
}

edited Jun 24 '16 at 04:38

answered Jun 24 '16 at 04:34

user1887915

1,299
10
13

hmm .. .but I still get i = 0 .. was expecting 10 – pokche Jun 24 '16 at 04:37
3

@pokche, apart from un-shadowing `i` by other variable, you can also make `some_mutex` as `static` in your original code. To me that seems more viable option then passing the `std::mutex` everytime. – iammilind Jun 24 '16 at 04:41
2

@iammilind: IMO that's a terrible design. It lets you do one thing conveniently, but anything else will have poor performance or be impossible (e.g. different groups of threads working with different `int` objects, or threads calling both `add` and `sub` that need to be synchronized). – Jun 24 '16 at 04:50
2

@Hurkyl, your argument is correct when we are dealing with different `int i` variables. However, I believe that above is just a toy code. Because in real time, different groups of threads with different `int i`s will have different functions also. Moreover for this specific case, the `int i` itself will be encapsulated within the `add()` (or some class) as a `static` variable. Having a `static mutex` within a method is a common design pattern. – iammilind Jun 24 '16 at 05:03
@Hurkyl I kind of like the idea of making mutex static inside the add func ... but not sure why u opposed it ... could you make your statement a little clear if you don't mind or elaborate the example I did not get it – pokche Jun 24 '16 at 05:31
2

@pokche: Because it permits one and exactly one synchronization model. It's great if that's the model you want. But if threads are working on independent data and don't need synchronization... you force them to synchronize anyways. And if you want calls to both `add` and `sub` to be synchronized... you couldn't use the existing mutex and so you would have to add an extra, external layer of synchronization on top of it. – Jun 24 '16 at 05:36
3

@pokche: The global-embedded-in-the-function makes sense if it's actually protecting a truly global resource from concurrent access (and if the function is the only access point for the resource). But to use it in other circumstances (e.g. protecting access to a user-supplied function parameter) is unnecessarily restrictive. – Jun 24 '16 at 05:40

Why does add function have no effect in c++ 11 thread?

1 Answers1