Improperly terminating a thread?

Question

I have an n-body simulation that I'm parallelizing. I am using QT, so I have a custom QObject class that has the process that I have parallelized. It is controlled by another QObject inside a thread that just handles the thread. The problem is that every time one object collides with another, all the threads have to be stopped so that they can be deleted, and recreated with a different system of planets.

Here is the thread handler class source:

#include "threadhandler.h"
#include <QEventLoop>
#include "subprocess.h"
#include <QThread>
#include <iostream>
#include <vector>
#include <thread>
#include <chrono>

ThreadHandler::ThreadHandler(double scopeX, double scopeY)
{
int size = 100;
int idealNum = QThread::idealThreadCount();
int sizeForEach = size/idealNum;

for(int i = 0 ; i< idealNum ;i++){
   SubProcess* tempSub = new SubProcess(i*sizeForEach, (i+1)*sizeForEach, scopeX, scopeY);
   QThread* tempThread = new QThread;
   tempSub->moveToThread(tempThread);
   QEventLoop::connect(tempSub, SIGNAL(finished()), tempThread, SLOT(deleteLater()));
   QEventLoop::connect(tempThread, SIGNAL(started()), tempSub, SLOT(process()));
   QEventLoop::connect(tempThread, SIGNAL(finished()), tempSub, SLOT(deleteLater()));
   QEventLoop::connect(tempSub, SIGNAL(collided()), this,
                       SLOT(refactorObjects()));
   ThreadHandler::threads.push_back(tempThread);
   ThreadHandler::objects.push_back(tempSub);
}
}

void ThreadHandler::process(){
std::cout << ThreadHandler::threads.size() << std::endl;
for(int i = 0; i < ThreadHandler::threads.size(); i++){
    std::cout << "starting " << i+1 << std::endl;
    ThreadHandler::threads.at(i)->start();
}
}

void ThreadHandler::refactorObjects(){
SubProcess::condition = false;
std::this_thread::sleep_for(std::chrono::seconds(1));
for(int i = 0 ; i < ThreadHandler::threads.size() ; i++){
    ThreadHandler::threads.at(i)->terminate();
}
ThreadHandler::objects.clear();
SubProcess::sys.push_back(Body::collide(SubProcess::collidedAr.at(0), SubProcess::collidedAr.at(1)));
SubProcess::sys.erase(std::remove(SubProcess::sys.begin(), SubProcess::sys.end(),
                                  SubProcess::collidedAr.at(0)), SubProcess::sys.end());
SubProcess::sys.erase(std::remove(SubProcess::sys.begin(), SubProcess::sys.end(),
                                  SubProcess::collidedAr.at(1)), SubProcess::sys.end());

int idealNum = QThread::idealThreadCount();
int sizeForEach = SubProcess::sys.size() / idealNum;
for(int i = 0 ; i < idealNum ; i++){
    SubProcess* tempSub = new SubProcess(i*sizeForEach, (i+1)*sizeForEach);
    tempSub->moveToThread(ThreadHandler::threads.at(i));
    ThreadHandler::objects.push_back(tempSub);
    QEventLoop::connect(tempSub, SIGNAL(finished()), ThreadHandler::threads.at(i), SLOT(deleteLater()));
    QEventLoop::connect(ThreadHandler::threads.at(i), SIGNAL(started()), tempSub, SLOT(process()));
    QEventLoop::connect(ThreadHandler::threads.at(i), SIGNAL(finished()), tempSub, SLOT(deleteLater()));
    QEventLoop::connect(tempSub, SIGNAL(collided()), this,
                        SLOT(refactorObjects()));
}

for(int i = 0; i < idealNum; i++){
    ThreadHandler::threads.at(i)->start();
}
std::cout << "refactored" << std::endl;
}

Here is the sub process source:

#include "subprocess.h"
#include <iostream>

std::vector<Body> SubProcess::sys;
std::vector<Body> SubProcess::collidedAr;
double SubProcess::timeScale;
bool SubProcess::condition = true;


SubProcess::SubProcess(double start, double end, double scopeX, double scopeY){
for(int i = start; i< end; i++){
    SubProcess::sys.push_back(Body::createRandomBody(scopeX, scopeY));
}
this->start = start;
this->end = end;
}
SubProcess::SubProcess(double start, double end){
this->start = start;
this->end = end;
}

void SubProcess::process(){
while(SubProcess::condition){
    for(int i = start; i < end ; i++){
        for(int j = 0; j < SubProcess::SubProcess::sys.size() ; j++){
            if(!(SubProcess::sys.at(i)==SubProcess::sys.at(j))){
                double F = Body::getForce(SubProcess::sys.at(i), SubProcess::sys.at(j));
                if (F!=-1){
                    double dX = SubProcess::sys.at(i).getX()-SubProcess::sys.at(j).getX();
                    double dY = SubProcess::sys.at(i).getY()-SubProcess::sys.at(j).getY();
                    SubProcess::sys.at(i).exertForce(-dY, -dX, F, timeScale);
                } else {
                    SubProcess::collidedAr.clear();
                    SubProcess::collidedAr.push_back(SubProcess::sys.at(i));
                    SubProcess::collidedAr.push_back(SubProcess::sys.at(j));
                    SubProcess::condition = false;
                    emit collided();
                }
            }
        }
        SubProcess::sys.at(i).tick(timeScale);
    }
}
emit finished();
}

Whenever I run it, it works perfectly up to the point of a collision where I get:

starting 1
starting 2
starting 3
starting 4
refactored
refactored
QThread: Destroyed while thread is still running
QThread: Destroyed while thread is still running
QThread: Destroyed while thread is still running
QThread: Destroyed while thread is still running

So what doesn't make sense to me is that the threads are deleted in the beginning of the refactoring process, and that creates no error. But for some reason AFTER the new threads are started, something is ending them. I have no idea what is going on here at all. I think that I am improperly disposing and recreating the threads and that there should be a better method to this.

score 2 · Accepted Answer · edited May 23 '17 at 12:09

2

Generally speaking, terminating a running thread leads to undefined behavior (as opposed to a thread that's blocked in a state where other data is consistent). You shouldn't be terminating the threads. Simply execute the deleteLater on the subprocesses, and they'll dispose of themselves automatically. Their threads won't be affected. See this answer for why it's OK to call deleteLater on objects in other threads.

You should keep a pool of threads for reuse, no need to recreate them. You can even reuse the QThreadQueue for that.

A default QThread is very much unsafe to destroy, and you should use a version without that deficiency instead, if you can.

edited May 23 '17 at 12:09

Community

1
1

answered Apr 20 '16 at 17:25

Kuba hasn't forgotten Monica

95,931
16
151
313

But if I call deleteLater on them, then how can I reuse them? Doesn't that delete them? Or is a temporary stop? – Chris Apr 20 '16 at 18:41
@user1563074 You delete the worker objects (the subprocesses), not the threads, of course. – Kuba hasn't forgotten Monica Apr 20 '16 at 18:42
If I don't need to destroy/dispose of the threads, then why do I need a custom Thread class that is safer to destroy. Also, I have updated the code above, and it still yields the same errors. – Chris Apr 23 '16 at 23:30
@user1563074 You'll dispose of them *eventually*, I'd think :) – Kuba hasn't forgotten Monica Apr 25 '16 at 13:03

Improperly terminating a thread?

1 Answers1