Errors with multithreading in for loop

Question

I am trying to add a simple for loop with threads, yet still something is not working out. I've checked a number of reasons and I cannot find any solution to that.

I have a simple class with two methods A() and B(). From the other class, I'm calling method A(). This is how it looks:

void MyClass::A()
{
    std::vector<std::thread> threads;
    for(int i=0;i<2;i++)
    {
        threads.push_back(std::thread(&MyClass::B, this));
    }
    for(auto &t : threads)
    {
        if(t.joinable())
            t.join();
    }
}

void MyClass::B()
{
}

And yet I am still receiving some errors:

#0 ??   ?? () (??:??)
#1 00446D62 pthread_create_wrapper () (??:??)
#2 75327FB0 msvcrt!_cexit() (C:\Windows\SysWOW64\msvcrt.dll:??)
#3 040C8710 ?? () (??:??)
#4 753280F5 msvcrt!_beginthreadex() (C:\Windows\SysWOW64\msvcrt.dll:??)
#5 75B17C04 KERNEL32!BaseThreadInitThunk() (C:\Windows\SysWOW64\kernel32.dll:??)
#6 77ABAB8F ?? () (??:??)
#7 77ABAB5A ?? () (??:??)
#8 ??   ?? () (??:??)

Does someone have any idea what is wrong?

Just to add one more thing. This:

void MyClass::A()
{
    std::thread t(&MyClass::B, this);
    if(t.joinable())
        t.join();
}

void MyClass::B()
{
}

works without any problems.

Answer 1

I would suggest using new and a pointer instead of references.

While references are nice, the number of comments illustrate the confusion caused in this case. How can you prove that the third party library implementation (eg. std) will work the way you are expecting it to? and across platforms?

If you use:

std::vector<std::thread *> threads;

and:

threads.push_back(new std::thread(&MyClass::B(), this));

And of course with a 'delete' when you are done with the thread, then the confusion is eliminated.

Also, the code is loading the SAME instance of the 'A' class onto the array for all threads. If you use member variables in A within function 'B', without any sort of multithread protection (eg semaphores, critical sections, etc) your code will never work correctly. Using a 'new' for the thread instance creation avoids the problems that copy constructors or assignment operators cause with the vector class.

#include <SDKDDKVer.h>
#include <vector>
#include <thread>
#include <iostream>

using namespace std;

class MyClass {
  public:
    void A() {
      cout << "starting\n";
      cout << "creating threads\n";
      vector<thread*> threads;
      for (int i = 0;i<4;i++) {
        thread * t = new thread(&MyClass::B, this);
        cout << "creating thread "<<t<<" id:"<<t->get_id()<<"\n";
        threads.push_back(t);
      }
      cout << "waiting for threads to complete\n";
      for (thread * t : threads) {
        if (t->joinable()) {
          t->join();
        }
        cout << "destroying "<<t<<"\n";
        delete t;
      }
      cout << "done\n";
   }

   void B() {
     // now it would be very bad if this function used 
     // member variables that are part of this instance of A
     cout << "hello from " << this << "\n";
   }
}; 

int main() {
  MyClass cls;
  cls.A();
  return 0;
}