C++ detached thread: terminate called without an active exception

Question

I'm creating a thread pool system in C++ and I'm getting a weird exception when destroying all of my threads. This is what's happening:

terminate called without an active exception

This is the code for my Worker class:

  Queue<Job> Worker::job_queue = Queue<Job>();
  std::atomic<bool> Worker::run(false);
  std::vector<Worker*> Worker::workers = std::vector<Worker*>();
  std::mutex Worker::queue_mutex;

  Worker::Worker() : worker_thread(Worker::loop, this){
    worker_thread.detach();
    workers.push_back(this);
  }

  void Worker::loop(){
    while(Worker::run){
      try{
        queue_mutex.lock();
        Job todo = job_queue.pop();
        queue_mutex.unlock();
        todo.job(todo.params);
        todo.isDone = true;
      } catch(...){
        queue_mutex.unlock();
      }
    }
  }

  void Worker::init(){ //Static method; called when the program starts
    run = true;
    for(int i=0;i<NUM_OF_WORKERS;i++){
      workers.push_back(new Worker());
    }
  }

  void Worker::uninit(){ //Static method; called when the program is about to terminate
    run = false;
    for(int i=0;i<workers.size();i++){
      delete workers[i];
    }
  }

Why is this happening and how can I fix it?

https://stackoverflow.com/questions/7381757/c-terminate-called-without-an-active-exception — Vlad Feinstein, Apr 06 '21 at 21:05
@VladFeinstein "...So if you don't want to terminate your program, make sure you join (or detach) every thread." I detached my threads yet this is still happening — Serket, Apr 06 '21 at 21:06
You seem to be adding two copies of every worker into the vector. That is not going to work if you call delete on all of them. — NathanOliver, Apr 06 '21 at 21:07
Side note: Destroying a locked mutex [can go boom](https://en.cppreference.com/w/cpp/language/ub). You should reconsider `detach`ing. — user4581301, Apr 06 '21 at 21:08
You definitely don't want to `detach()` the threads in this situation, `join()` them instead. And use `std::lock_guard` to lock/unlock the `mutex` safely, get rid of the `try..catch` altogether — Remy Lebeau, Apr 06 '21 at 21:11

score 3 · Answer 1 · edited Apr 06 '21 at 21:24

3

You are inserting the same object into workers twice - first at Worker() constructor, and then when operator new returns. So at the end, you are getting a double free - you should have used RAII with smart pointers instead.
You are manually locking and unlocking the mutex, and also intercept and swallow all exceptions - you should have used RAII with ::std::lock objects instead.
Detaching threads is not a good idea in general, see When should I use std::thread::detach?. Destroying objects still used by those threads leads to troubles.

edited Apr 06 '21 at 21:24

Remy Lebeau

555,201
31
458
770

answered Apr 06 '21 at 21:07

user7860670

35,849
4
58
84

Thanks! Should I get rid of my threads by joining them or detaching them? – Serket Apr 06 '21 at 21:08
1

@Serket I'd `join` them. If you destroy `Worker` while it holds a locked `mutex` [things can get bad](https://en.cppreference.com/w/cpp/language/ub). – user4581301 Apr 06 '21 at 21:10
I don't destroy Worker with a locked mutex; run = false comes before deleting any workers – Serket Apr 06 '21 at 21:10
1

@Serket Executing `run = false;` does not mean that worker loop is going to exit strictly prior to deleting. – user7860670 Apr 06 '21 at 21:13
Well I took your suggestion and changed it to ```join()``` anyways :) – Serket Apr 06 '21 at 21:14
That won't help as much as you'd like. You can easily set `run` and `delete` a `Worker` while it's still working. `Worker` tests `run`, enters loop, locks mutex, and gets gets interrupted. Interrupting thread sets `run` set to `false` and deletes the `Worker`. – user4581301 Apr 06 '21 at 21:16

C++ detached thread: terminate called without an active exception

1 Answers1