0

I'm facing a threads problem:

Here is a code to reproduce the error:

using namespace std;

#include <stdio.h>
#include <time.h>
#include <iostream>
#include <math.h>
#include <cstdlib>
#include <unistd.h>

#include <iostream>
#include <sstream>
#include <thread>
#include <vector>

#include "mychrono.hpp"

int main()

{
    
    std::vector<Chronometer*> car_crono;
    Chronometer chrono, output_chrono;
    std::vector<std::thread> threads;


    while (1) {
       

        
        for(int i = 0; i < 2; i++)
        {
            car_crono.push_back(new Chronometer);
        }

        for(int i = 0; i<2; i++)
            {
        
                    threads.push_back(std::thread(&Chronometer::start_chrono, car_crono[i], std::ref(chrono)));                     
            
             }

        //std::cout << "Threads size: " << threads.size();
        for (auto &th : threads) {
                    th.join();
                    }
            
       std::cout << "Hello-world" << std::endl;
    }
}

My chrono.cpp

#include "mychrono.hpp"

#include <time.h>
#include <iostream>
#include <cstdlib>
#include <unistd.h>

#include <sstream>
#include <thread>


//int Chronometer::hour(0), min(0), sec(0);

Chronometer::Chronometer() : hour(0), min(0), sec(0)
{

}

Chronometer& Chronometer::start_chrono(Chronometer& chrono)
{
 
  if(chrono.hour == 0 && chrono.min == 0 && chrono.sec == 0)
  {
    bool condition = true;
    while(condition) {
      sleep(1);
      chrono.sec++;

      if(sec > 59) {
        chrono.min++;
        chrono.sec = 0;

      }

      if(min > 59) {
        chrono.hour++;
        chrono.sec = 0;
        chrono.min = 0;
      }
      if(chrono.sec == 10)
      {
        condition = false;
      }

      std::cout << "chrono: " << chrono << std::endl;

   }

  }  

  return chrono;
}

My chrono.hpp

#include <time.h>
#include <iostream>
#include <sstream>

#ifndef mychrono_hpp
#define mychrono_hpp

class Chronometer
{
    private:
        int hour, min, sec;
        //std::stringstream ss;
        //Chronometer chrono;

    public:

        Chronometer();
        Chronometer& start_chrono(Chronometer& chrono);
        Chronometer& finish_chrono(Chronometer& chrono);
        friend std::ostream& operator<<(std::ostream& flux, Chronometer t);
        Chronometer& operator=(const Chronometer& other);
        ~Chronometer();

};


#endif

My purpose is just to launch independant several chronometers in background to get the result. For one instantiated Chronometer object the program crash at the end with the following output:

terminate called after throwing an instance of 'std::system_error'
  what():  Invalid argument
Abandon (core dumped) 

But for 2 chronometers for example, the treads increment one chronometer 2 times faster instead of increment two independent chronometer.

I'm a beginner in threads so I have no idea why is running like that

Thib
  • 11
  • 1
  • 4
  • Have you tried using a debugger? Please show a [mre] – Alan Birtles Aug 24 '21 at 15:04
  • @AlanBirtles I add easier code example below to reproduce the error and better explain the issue I'm facing of. – Thib Aug 24 '21 at 15:47
  • you should [edit](https://stackoverflow.com/posts/68909406/edit) that into your question rather than posting it as an answer – Alan Birtles Aug 24 '21 at 15:54
  • note that [`using namespace std`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) before including standard library headers is likely to lead to bugs/crashes/weird compilation errors – Alan Birtles Aug 24 '21 at 15:55
  • your new code works for me (once I'd fixed the linker errors due to missing function implementations), please show a [mre] – Alan Birtles Aug 24 '21 at 16:13
  • It's not working for me if I'm not clearing the threads vector after execution. Moreover if I print simple message as edited in my code above like "hello-world" outside the thread. Why the message is not printed in the same time as the thread should run in background ? – Thib Aug 24 '21 at 16:21
  • `"Hello World"` won't print as `th.join()` will block until the thread completes. Is this exactly the code you are running? It didn't link for me due to missing function implementations – Alan Birtles Aug 24 '21 at 16:25
  • Yes it's running sorry but it's not the result I was expecting for. Moroever, I need the thread to use as a background which is not the case in my code. I thought the thread could do that – Thib Aug 24 '21 at 16:34
  • How it's possible to run in background ? – Thib Aug 24 '21 at 16:41
  • just don't call `join` until you actually want to join the threads – Alan Birtles Aug 24 '21 at 16:50

1 Answers1

0

There are a few issues in your code.

  1. in your while loop in main after calling join you leave the threads in the threads vector, on the next iteration of the loop you will again call join on the same threads, join throws std::invalid_argument if you call join more than once. Call clear onthreads after joining the threads.
  2. Inside start_chrono you have multiple threads modifying chrono.sec with no synchronisation, this leads to a race condition where not all threads will satisfy the chrono.sec == 10 condition leading them to continue executing forever as the condition if (sec > 59) which should presumably be if (chrono.sec > 59) is never true as sec doesn't change.
  3. As you are calling join after creating the threads main will be blocked and can't perform any other work. You should only call join when you actually want to wait for the thread to stop executing.
Alan Birtles
  • 32,622
  • 4
  • 31
  • 60