QThread: Destroyed while thread is still running

jech

Hello,

I'd like to ask you for help with threads. I'm working on a music player. I need to search for files in a directory and then read tags from the files. Obviously this has to run in a background thread so my UI won't get locked. I found a guide on how to use QThread and wrote this code:

mediascan.cpp
@#include "mediascan.h"

MediaScan::MediaScan(QObject *parent) :
QObject(parent)
{
}

void MediaScan::scan(const QStringList &directories) {
QThread* searchThread = new QThread;
FileSearch* search = new FileSearch(directories);
search->moveToThread(searchThread);

connect(searchThread, SIGNAL(started()), search, SLOT(start()));
connect(search, SIGNAL(fileCountUpdate(int)), this, SLOT(onFileCountUpdate(int)));
connect(search, SIGNAL(filesFound(QStringList)), this, SLOT(onFilesFound(QStringList)));
connect(search, SIGNAL(finished()), searchThread, SLOT(quit()));
connect(search, SIGNAL(finished()), search, SLOT(deleteLater()));
connect(search, SIGNAL(finished()), searchThread, SLOT(deleteLater()));
searchThread->start();

}
@

filesearch.cpp
@#include "filesearch.h"

FileSearch::FileSearch(const QStringList &dir, QObject *parent) :
QObject(parent)
{
directories = dir;
}

FileSearch::~FileSearch()
{
}

void FileSearch::start()
{
fileCount = 0;
for (int i=0; i<directories.length();i++) {
searchDirs(directories[i]);
}
emit filesFound(fileList);
emit finished();
}

void FileSearch::searchDirs(const QString &directory) {
QDir dir = directory;
dir.setFilter(QDir::Dirs | QDir::NoDotAndDotDot);
dir.setSorting(QDir::Name);
QFileInfoList list = dir.entryInfoList();
for (int i = 0; i < list.size(); ++i) {
QFileInfo fileInfo = list.at(i);
searchDirs(directory+"/"+fileInfo.fileName());
}
dir.setFilter(QDir::Files | QDir::NoDotAndDotDot);
dir.setSorting(QDir::Name);
list = dir.entryInfoList();
for (int i = 0; i < list.size(); ++i) {
QFileInfo fileInfo = list.at(i);
fileList.append(directory+"/"+fileInfo.fileName());
fileCount++;
emit fileCountUpdate(fileCount);
}
}
@

Short description what it does: The class MediaScan is connected to my QML UI and the function "scan" is called when I click a button. The class FileSearch walks recursively through all subdirectories and stores file paths to it's variable fileList. It also emits a signal fileCountUpdate() with the number of already found files and when searching is finished it emits filesFound() and finished() signals.

The code works usually well but sometimes I get following errors:

QThread: Destroyed while thread is still running
QMutex: destroying locked mutex

After this the application usually crashes (not always). What is wrong in my code? Sorry if it's something obvious I'm absolute beginner in C++.

KA51O

This line should be replaced:
@
connect(search, SIGNAL(finished()), searchThread, SLOT(deleteLater()));
@
by this
@
connect(searchThread, SIGNAL(finished()), searchThread, SLOT(deleteLater()));
@

this way you're using the finished() signal from QThread (which is emitted after the quit() slot terminated the event loop of the thread) to trigger the deleteLater() call.
In your implemetation deleteLater() is triggered by the finished signal of your worker object and might thus be called before the threads event loop has stopped.

NicuPopescu

@void MediaScan::scan(const QStringList &directories) {
//QThread* searchThread = new QThread;//move it as member in MediaScan : QThread searchThread ;
FileSearch* search = new FileSearch(directories);
search->moveToThread(&searchThread);

connect(searchThread, SIGNAL(started()), search, SLOT(start()));
connect(search, SIGNAL(fileCountUpdate(int)), this, SLOT(onFileCountUpdate(int)));
connect(search, SIGNAL(filesFound(QStringList)), this, SLOT(onFilesFound(QStringList)));

//connect(search-, SIGNAL(finished()), searchThread, SLOT(quit());//THIS potentially causes crashes : DELETE IT, searchThread is a class memmber
connect(searchThread, SIGNAL(finished()), search, SLOT(deleteLater()));//QThread has already a finished() signal
//connect(search, SIGNAL(finished()), searchThread, SLOT(deleteLater()));//searchThread is a class member in static memory now
searchThread->start();
}@

@void FileSearch::start()
{
fileCount = 0;
for (int i=0; i<directories.length();i++) {
searchDirs(directories[i]);
}
emit filesFound(fileList);
//emit finished(); //not needed
}@

in

@~MediaScan() {
searchThread.quit();
searchThread.wait();
}@

you can find a similar example in QThread class doc ... but this example is not the best use of QThread, is better to subclass QThread and reimplement run() method

KA51O

[quote] you can find a similar example in QThread class doc … but this example is not the best use of QThread, is better to subclass QThread and reimplement run() method [/quote]
I think for this use case the worker object approach is better suited then the subclassing approach. Another possible solution would be using QtConcurrent to run the FileSearch::searchDirs(..) function in another thread.

jech

KA51O> I tried your simple fix and it seems to work so far. I'll need to do more testing though.

NicuPopescu> I was thinking about moving the thread to a class member. If after KA510's fix I still run into problems, I'll implement this.

Many thanks to both of you. Btw. I was using the method of subclassing QThread in my previous PySide (Python) version. But I just recently read somewhere, that it's not the best way to go and using moveToThread is better.

JKSH

[quote]But I just recently read somewhere, that it’s not the best way to go and using moveToThread is better.[/quote]That was hotly debated for a while, but I think the consensus is, "it depends on what you're trying to do".

This new doc page describes all the ways to use threads in Qt and tries to help readers choose the most suitable one for their application: http://doc-snapshot.qt-project.org/qt5-stable/threads-technologies.html