-2

I have a problem with threading in C++, I want to parallelly call same method on different class objects. However, program encounters some problem with that. From time to time it crashes and sometimes runs through code part, but always return a ridiculously large number as an ID, always in the place of 0. The class body:

class Gas{
public:
    Gas(const int id, const int imax, const int jmax){
        id_ = id;
        NX_ = imax;
        NR_ = jmax;
    }
private:
    int id_;                            //id of specimen
    int NX_;                            //quantity of elements in the X direction
    int NR_;                            //quantity of elements in the R direction

    std::vector<double> fr_;            //molar fraction
    std::vector<double> ms_;            //mass source
    std::vector<double> ms_old_;        //mass source value from previous iteration - for source relaxation
};

The class method:

double Gas::initialize(){
        int i, j, k;
        fr_.resize(NX_ * NR_);
        ms_.resize(NX_ * NR_);
        ms_old_.resize(NX_ * NR_);

        std::cout << "ID: " << id_ << '\n';


        k = 0;
        for(i = 0; i < NX_; i++){
            for(j = 0; j < NR_; j++){
                fr_[k] = 0.01;
                ms_[k] = 0.;
                k++;
            }
        }
    }

Here is the threading implementation in the code:

std::vector<Gas> gases;
std::vector<Gas*> ptr_gas(6);
for(i = 0; i < 6; i++){                                     //creating vector holding objects representing specific gases
        gases.push_back(Gas(i, 10, 5));
        ptr_gas[i] = &gases[i];
    }

std::vector<std::thread*> th_gas;
int i = 0;
for(auto &X : ptr_gas){
    std::thread *thr = new std::thread(&Gas::initialize, ptr_gas[i]);
    th_gas.push_back(thr);
    i++;
}

for(auto &X : th_gas){
    X->join();
    delete X;
}
th_gas.clear();     

The output of std::cout I put inside the method goes like this:

ID: ID: 1
4999616
ID: 5
ID: 4
ID: 2
ID: 3

It looks like there is some problem with joining of the first of created threads. Any suggestions on how to avoid that problem? I have already checked ptr_gas addresses and they are correct.

Padzak
  • 31
  • 6
  • Please show a [mre], what is wrong with your output? What do you expect it to be? What do you think the mutex in `initialise` does? – Alan Birtles Nov 04 '19 at 07:24
  • Mutex does not do anything. I forgot to clear it before posting te code – Padzak Nov 04 '19 at 07:53
  • 1
    In case the problem here is the 'invalid' ID that is printed, it is not possible to answer that from the code you already posted. It is some issue in the construction of the `Gas` instances, or this `gas_ptr` vector (?), the code for which is not included in your question. – Ton van den Heuvel Nov 04 '19 at 07:58
  • Updated the code, is it now possible to define what is the problem? – Padzak Nov 04 '19 at 08:18
  • This code would not pass any code review process I know. Please check all of it... (for instance having a range for loop and using an external `int i` counter; what do you need the range for loop for ?) Aside from that, you should allocate when you need and use smart pointers. – Blacktempel Nov 04 '19 at 08:22
  • please never allocate a `std::thread`, it is totally fine to store it by value! – JVApen Nov 04 '19 at 08:34
  • Any comments on why to do so? I have not encountered any problems with that before and I would only like to know why I should avoid this practice – Padzak Nov 04 '19 at 10:39

1 Answers1

2
std::vector<Gas> gases;
std::vector<Gas*> ptr_gas(6);
for(i = 0; i < 6; i++){                                     //creating vector holding objects representing specific gases
        gases.push_back(Gas(i, grid));
        ptr_gas[i] = &gases[i];
    }

Your pointers in ptr_gas get "invalid" because "gases" will reallocate when you push_back.

phön
  • 1,215
  • 8
  • 20