Solved Threads problems
-
Replace
@DoubleC122 said in Threads problems:
QThread* thread = QThread::create(&unzip_v2, filename);
thread->start();with
std::async(std::launch::async,&unzip_v2,filename).get();
no need to use Qt for somethingstd
does probably better already.Your problem is that you are not waiting for the thread to finish, that shouldn't be a real problem as you mention a GUI in your question so the event look should be able to keep the program alive while the thread is running. you just need to make sure you join all threads (here I do it with the final
.get()
before the main finishes -
Indeed, by using
std::async
I can definitely feel the performance is improved. One inconvenient is that the gui keeps freezing. By freezing I mean just what the gui displays, not that becomes completely unresponsive and then the program crashes. The program just keeps going while I'm unable to do other things inside the gui. Is that a problem that comes within Qt itself? It's quite strange to me. -
Are you calling
get()
on the future returned bystd::async
? that's what freezes the GUI.You can change it to:
QObject* finishObject= new QObject; QObject::connect(finishObject,&QObject::destroyed,[](){std::cout << "unzip_v2 finished";}); std::async(std::launch::async,[finishObject](QByteArray filename){unzip_v2(filename.constData()); delete finishObject;},filename);
So that your GUI does not stop and you have control on when the process actually finished.
I usedQByteArray
so that you don't have to worry about the lifetime of the argument passed tounzip_v2
One last note: never change your GUI directly from a secondary thread
-
Hm, doesn't seem to work. My unzip function is called from a signal function though, could that mean something related to the gui freezing?
-
The code above is GUI-less so it's hard to guess without knowing what your code looks like
-
Well it works like this. First, I have a signal called
on_listView_customContextMenuRequested(const QPoint &pos)
which will pop a context menu on right click. Then, based on what I select, I'll call the function that actually pops the menu,showContextMenu(pos)
. There, I assemble my menu with things like:myMenu.addAction("Unzip here")
and other options. I guess it's worth to mention that I useQSignalMapper
to connect some slots that have arguments (yes, I know it's deprecated, but I found that this way worked fine for me, might this actually be the issue? :/ ) and then connect everything like this:connect(action, SIGNAL(triggered(bool)), mapper, SLOT(map())); connect(mapper, SIGNAL(mapped(QString)), this, SLOT(unzip(QString))); mapper->setMapping(action, Fpath);
And finally, in the
unzip
slot I call theunzip_v2
function. If there is the need of more explicit code, I'll post it, didn't want to make this reply too long. And also in theunzip
slot is where I use thestd::async
. -
What I think it's happening is that the
QString
argument that you pass tounzip_v2
asconst char*
goes out of scope beforeunzip_v2
completed. could you show us the content ofunzip
?P.S.
QSignalMapper is not the problem here. If you really want to get rid of it you can replaceconnect(action, SIGNAL(triggered(bool)), mapper, SLOT(map())); connect(mapper, SIGNAL(mapped(QString)), this, SLOT(unzip(QString))); mapper->setMapping(action, Fpath);
with
connect(action,&QAction::triggered,this,std::bind(&MyClass::unzip,this,Fpath));
(whereMyClass
is the type of*this
) -
Here is
unzip
:void MainWindow::unzip(QString filePath) { boost::filesystem::path tempP(filePath.toStdString()); chdir(tempP.parent_path().string().c_str()); std::async(std::launch::async,&zipUtils::unzip_v2, tempP.filename().string().c_str()).get(); }
-
Does this code even compile? is
zipUtils
a namespace?Try changing
std::async(std::launch::async,&zipUtils::unzip_v2, tempP.filename().string().c_str()).get();
tostd::async(std::launch::async,[](std::string nameString){zipUtils::unzip_v2(nameString.c_str());}, tempP.filename().string());
-
Yes, it does compile just fine and indeed,
zipUtils
is just a namespace. Even after changing those, it still continues to freeze. This problem seems to keep bugging me for a while now. -
@DoubleC122 said in Threads problems:
it still continues to freeze.
Did you notice I'm not calling
get()
at the end? that's the prime suspect for the freeze, did you remove it as well? -
I actually added it at the end since I noticed it wasn't there.
-
Yep, remove it. the fact that was missing was intentional.
To explain a bit more, I'm using a lambda here to prevent the data pointed by
c_str()
going out of scope -
I tested it right now once again but still won't unfreeze. I know that that
for(;;)
inunzip_v2
is quite heavy to compute with a large zip but that is why I thought it would get better with threads. -
I guess I found out a way to do this. It was just by chance, but, based on what you wrote here, I thought I'd try and use a lambda expression with QThread and QtConcurrent and to my surprise both worked, and I stuck with QThread in the end. The result looks like this:
QThread* thread = QThread::create([](std::string nameString){zipUtils::unzip_v2(nameString.c_str());}, tempP.filename().string()); thread->start();
This won't freeze the gui anymore and performance is alright (although from time to time the system seems to struggle a bit, maybe it's something I can fix later). Anyway, thank you for your help, without this whole insight I would still be stuck :).