Search code examples
multithreadingpointersc++11c++14stdthread

std::thread context of execution (c++14)


The problem appear when an in/out variable of a function called by std::thread changes the value during the execution...


Function:

static int func(stThread_t *&pStThread)

Parameters

pStThread: It´s a struct that has a pointer to std::thread and other variables (some flags)

typedef struct stThread {
   stThread() noexcept {...};    
   stThread(const stThread &cRigth) noexcept {...};    
   stThread & operator = (const stThread &cRigth) noexcept {...};

   std::thread *pThread;
   volatile bool bBegin;
   volatile bool bEnd;

 } stThread_t;

The function func print the address of the std::thread of the parameter pStThread and the thread id

func before 1785280 this_id 21968

after making an this_thread::sleep for 2 seconds, it print it again

func afer ... this_id 21968

   static int func(stThread_t *&pStThread) {

      std::thread::id this_id = std::this_thread::get_id();

      long long p_begin = (long long)pStThread;
      std::cout << "func before " << std::to_string(p_begin) << " this_id " << this_id << "\n";
      std::cout.flush();

      pStThread->bBegin = true;

      std::this_thread::sleep_for(std::chrono::milliseconds(2000));

      this_id = std::this_thread::get_id();
      long long p_end = (long long)pStThread;
      std::cout << "func afer " << std::to_string(p_end) << " this_id " << this_id << "\n";
      std::cout.flush();

      pStThread->bEnd = true;

      return 1;
   };

The address of the pointer to std::thread it´s changed (corrutped, deleted..?)


The pStThread is pushing_back of a list of pointer struct stThread_t

std::list<stThread_t*> listOfThreads;
listOfThreads.push_back(pStThread);

I read about std::move, but does not work with pointers


At the end there is a thread "garbage collector" that it is trying to erase all the threads pending of execution.


Full code here

#include <string>
#include <list>
#include <vector>
#include <map>
#include <thread>
#include <mutex>
#include <atomic>

#include <iostream>

typedef struct stThread {
   stThread() noexcept {
      pThread = NULL;
      bBegin = false;
      bEnd = false;
   };

   stThread(const stThread &cRigth) noexcept {
      this->pThread = cRigth.pThread;
      this->bBegin = (bool)cRigth.bBegin;
      this->bEnd = (bool)cRigth.bEnd;
   };

   stThread & operator = (const stThread &cRigth) noexcept {
      this->pThread = cRigth.pThread;
      this->bBegin = (bool)cRigth.bBegin;
      this->bEnd = (bool)cRigth.bEnd;

      return *this;
   };

   std::thread *pThread;
   volatile bool bBegin;
   volatile bool bEnd;

} stThread_t;

class CMain
{
public:
   typedef std::list<stThread_t*> MyList_threads;
   MyList_threads listOfThreads;

public:
   CMain() {

      std::cout << std::boolalpha << "Ex1 is move-constructible? "
         << std::is_move_constructible<stThread_t>::value << '\n'
         << "Ex1 is trivially move-constructible? "
         << std::is_trivially_move_constructible<stThread_t>::value << '\n'
         << "Ex1 is nothrow move-constructible? "
         << std::is_nothrow_move_constructible<stThread_t>::value << '\n'
         << "Ex2 is trivially move-constructible? "
         << std::is_trivially_move_constructible<stThread_t>::value << '\n'
         << "Ex2 is nothrow move-constructible? "
         << std::is_nothrow_move_constructible<stThread_t>::value << '\n';
   };

   static int func(stThread_t *&pStThread) {

      std::thread::id this_id = std::this_thread::get_id();

      long long p_begin = (long long)pStThread;
      std::cout << "func before " << std::to_string(p_begin) << " this_id " << this_id << "\n";
      std::cout.flush();

      pStThread->bBegin = true;

      std::this_thread::sleep_for(std::chrono::milliseconds(2000));

      this_id = std::this_thread::get_id();
      long long p_end = (long long)pStThread;
      std::cout << "func afer " << std::to_string(p_end) << " this_id " << this_id << "\n";
      std::cout.flush();

      pStThread->bEnd = true;

      return 1;
   };

   int _createThreads() {
      for (int iIdx = 0; (iIdx < 5); iIdx++) {
         stThread_t *pStThread = new stThread_t;

         pStThread->pThread = new std::thread(&CMain::func,
            std::ref(pStThread));

         if (pStThread) {
            do {
               std::this_thread::sleep_for(std::chrono::milliseconds(100));
            } while (!pStThread->bBegin);
            listOfThreads.push_back(pStThread);

            std::string sLog;
            sLog = "\nlistOfThreads.push_back " + std::to_string((long long)pStThread) + "\n";
            std::cout << sLog;
            std::cout.flush();
         }

         std::this_thread::sleep_for(std::chrono::milliseconds(1));
      }
      return 1;
   };

   int _main() {

      _createThreads();

      std::thread thread_collector([=]() {
         bool bEnd = false;
         MyList_threads::iterator it;
         it = listOfThreads.end();

         do {
            stThread_t *pStThread = NULL;

            if (it == listOfThreads.end()) {
               it = listOfThreads.begin();
               if (it == listOfThreads.end()) bEnd = true;
            }
            else it++;

            if (it != listOfThreads.end()) {
               if ((*it)->bEnd) {
                  pStThread = *it;

                  listOfThreads.erase(it);
                  it = listOfThreads.begin();
               }
            }

            if (pStThread) {
               if (pStThread->pThread) {
                  if (pStThread->pThread->joinable()) {
                     pStThread->pThread->join();

                     std::cout << " element deleted  " << std::to_string((long long)pStThread) << "\n";
                     std::cout.flush();
                  }
                  delete pStThread->pThread;
                  pStThread->pThread = NULL;
               }
               delete pStThread;
            }
            pStThread = NULL;


            std::this_thread::sleep_for(std::chrono::milliseconds(1));

         } while (!bEnd);
      });



      if (thread_collector.joinable()) {
         thread_collector.join();
      }

      return 1;
   };
};

int main()
{
   CMain _main;
   _main._main();

   return 0;
}

Solution

  • You have a rather straightforward bug, that is mostly unrelated to threading:

    (1) func takes a reference to an stThread_t*.

    static int func(stThread_t *&pStThread);
    

    (2) You pass in a reference to pStThread

    std::thread(&CMain::func,std::ref(pStThread));
    

    (3) Which is a local variable whose lifetime ends as soon as the loop iteration is finished

      for (int iIdx = 0; (iIdx < 5); iIdx++) {
         stThread_t *pStThread = new stThread_t;
         //...
      }
    

    (4) And consequently, you get undefined behavior when your function tries to access the object after it is destroyed. (be careful! here, "the object" refers to the pointer in question, not the object the pointer is pointing to)

    It's unclear why you insist on passing pStThread by reference; your function doesn't actually modify the pointer (just what's pointed to), and you don't seem to intend to do any of the things that such a device is actually good for.