Question

I can't share code but theres a sort of example below. Here's my problem. Imagine I have a controller class that has an instance of QThreadPool. This class keeps a list of worker objects. When I start a runnable, I create a worker and give it to the runnable. The runnable operates each worker on the different threads. When the runnable is done, it fires a signal back to the controller object. The controller object then reads the results from the worker and deletes the worker. The controller keeps a list of all workers so that it can set the cancel flag on the worker, so that the thread can exit early if necessary.

The problem is that the QThreadPool will wait for all threads to finish when it's destroyed, which will be in the controller destructor. Since signals are queued when fired from the threads, then potentially canceling the workers in the controller destructor will queue the signals to be delivered to the controller. But since the controller will be in the context of the destructor, those signals won't hit the controller slots, and therefore the worker objects won't get destroyed.

Maybe I'm not dealing with the thread pool the way it should be. If I destroy the worker objects in the runnable, then there is potential for the worker to be destroyed before the another thread tries to cancel a worker. Can anyone recommend a good way of creating a cancelable thread pool?

class Controller : public QObject {
     Q_OBJECT
public:
     virtual ~Controller();
     QThreadPool pool;
     void startTask();
     Q_SLOT void onWorkerDone(Worker * worker);
     QList<Worker*> workers;
     void cancelAll();
}
void Controller::startTask() {
     Worker * worker = new Worker();
     connect(pool, SIGNAL(done(Worker *)), this, SLOT(onWorkerDone(Worker *));
     workers << worker;
     pool.start(new Runnable(worker);
}
void Controller::onWorkerDone(Worker * worker) {
    if ( worker ) {
        // read worker...
        delete worker;
    }
}
Controller::~Controller() {
     cancelAll();
     // Destructor now kills pool object.
     // The pool will wait until all threads complete.
     // If there are any workers still running on the
     // other threads, their done() signals will be 
     // queued to this instance, and since we're in 
     // the dtor, they won't make it to the slot, 
     // and worker won't get deleted.
}
void Controller::cancelAll() {
    // loop the list and call cancel.
}
Was it helpful?

Solution 2

The simple solution is to subclass QThreadPool and add an aboutToWait signal to that class. The signal would need to be emitted in the destructor. You can then hook up the signal to the controller's cancelAll() slot.

For this to work, the pool must be declared after the workers list!

Suppose you destroy the controller (this). The following happens:

  1. Body of this.~Controller executes.

    At the end, the object is still an instance of Controller. Now it has to be torn down into an instance of the base class. The members will be destructed in reverse of the order of declaration.

  2. threadPool is destroyed first. threadPool.~MyThreadPool emits aboutToWait.

  3. this.cancelAll runs, since it's directly connected. The call originates in the moc-generated implementation of the signal method. It lets the workers know that they should stop.

  4. Remainder of threadPool.~MyThreadPool runs, and members (if any) are destructed in reverse order of declaration. this is now an instance of QThreadPool.

  5. threadPool.~QThreadPool runs and waits for the workers to stop. As each worker stops, the controller.onWorkerDone is invoked, since it's directly connected to relevant thread pool's signals. Eventually, threadPool.~QObject runs the pool is fully destructed.

  6. workers.~QList runs.

  7. this.~QObject runs.

  8. The controller is fully destructed.

OTHER TIPS

  1. First mistake. You should not create a QThreadPool. This is application whide object so you should use QThreadPool::globalInstance()
  2. If you want to cancel job you have to provide some functionality in Worker class to do so. You need some loop in Worker::run() method which will do your job and pool for value which should change tread safely when loop should finish.
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top