Thread-GUI communication

KA51O

What kind of class are the bftdv1 objects created from and were exactly do you call the above code ? Also what is the parent of the QThread objects (the this pointer you pass in their constructor) ?

JKSH

[quote author="NullCoding" date="1350843257"]The output says
@Qthread: Destroyed while thread was still running.@
[/quote]It's complaining that the QThread got destroyed before the thread finished what it was doing. Often, it's because the user closed the GUI (by default, Qt auto-quits when the last window is closed), but the program didn't wait for the thread(s) to clean up.

[quote author="NullCoding" date="1350843257"]But it doesn't do anything at all.[/quote]From your code, your VerboseWindow::BeginNormalVerboseTest() function should be called each time you start() the thread... unless:

The VerboseWindows got destroyed too early (did they?), or
Your program doesn't have an event loop (does your main() function call QApplication::exec() ?)

[quote author="NullCoding" date="1350843257"]...this one's a bit of a trip because I'm trying to make threads communicate to the GUI.[/quote]Small subtlety, but the thread doesn't communicate with anybody (remember: QThread is not a thread... it is an object that controls a thread, and it informs you about the thread's state).

You want your worker object(s) to communicate with your GUI. In Qt, communication occurs via signals and slots.

[quote author="NullCoding" date="1350843257"]I know of queued connections but unsure how to implement[/quote]You don't have to do anything special to implement it. After you make your signal+slot connections, Qt takes care of the details: When a signal is emitted, Qt looks for the slot(s) to invoke. If the emitter (signal) and receiver (slot) are in different threads, Qt automatically uses a queued connection. (If they're in the same thread, Qt automatically uses a direct connection)

[quote author="NullCoding" date="1350843257"]pass the GUI as an object? I put the "GUI" in a class that is instantiated inside each class that controls a thread, and the UI is then passed into each instance as a pointer...gah it's too complicated. There simply must be an easier way. I am making Qt out to be some ridiculously complex language when in reality I'm sure it must be way easier.[/quote]I'm not sure what the Windows and Cocoa frameworks are like, but Qt thrives on event-driven programming. If your classes communicate by emitting signals and by reacting to signals, everything becomes quite clean. Your worker objects can be completely decoupled from your GUI, and you don't need to pass pointers to your GUI around.

Question: What do you mean when you say "class that controls a thread"?

JKSH

Well, that was plenty of talking. Now for an example:
@
class FactorialCalculator : public QObject
{
Q_OBJECT

signals:
void factorialFound(int factorial) const;

public slots:
void findFactorial(int value) const
{
int result = 1;

    while (value > 1) {
        result *= value;
        --value;
    }
    
    emit factorialFound(result);
}

public:
FactorialCalculator(QObject *parent = 0) : QObject(parent) {}
};

// Note: There are '&'s below because pointers to objects are needed
// In the Qt 5 docs you read, there are '&'s because pointers to functions are needed
int main(int argc, char **argv)
{
QApplication a(argc, argv);

QSpinBox numberBox;
QLabel resultLabel;
FactorialCalculator calculator;
QThread factorialThread;

// Make the FactorialCalculator process stuff in a separate thread    
calculator.moveToThread(&factorialThread);

// Set up communications -- this is all that's required!
// The label will be updated each time the user clicks on the spinbox
QObject::connect(&numberBox, SIGNAL(valueChanged(int)), &calculator, SLOT(findFactorial(int)));
QObject::connect(&calculator, SIGNAL(factorialFound(int)), &resultLabel, SLOT(setNum(int)));

// Let's roll
factorialThread.start();
numberBox.show();
resultLabel.show();

return a.exec(&#41;;

}
@

Note: This code will probably result in a "Destroyed while thread was still running" message when you close everything, but it's harmless in this case. We can look at how to prevent that in a future post

NullCoding

By "class that controls a thread," I mean I just made a class that contains all the functions necessary for the testing to work. There are several different ways to test a number in this program, so I figured for code cleanup purposes it made more sense to have classes full of the information needed - signals and slots, as well as the structures that contain the actual MPIR variables that are used for the numbers themselves.

So to connect signals + slots, would it look like this?

@connect(ui.ThreadProgress1, SIGNAL(valueChanged(int)), bftdv1, SLOT(UpdateProgressBar(int)));@

Because while that is not throwing any errors, it's also not doing anything. In fact it looks like when I press the button to start the test, nothing happens at all.

And I'm still a bit lost as to how I should update the GUI from another thread. Let's say it finds a factor. I have code that fills in text boxes and stuff like that, but unsure how to call it. Do I call the function itself? That seems redundant if I am also connecting stuff to things, but even then I don't know what I should connect!

Do I connect the text box to the function? There's more than one modified by that function. I'm very confused, actually.

JKSH

[quote author="NullCoding" date="1350940920"]By "class that controls a thread," I mean I just made a class that contains all the functions necessary for the testing to work. There are several different ways to test a number in this program, so I figured for code cleanup purposes it made more sense to have classes full of the information needed - signals and slots, as well as the structures that contain the actual MPIR variables that are used for the numbers themselves.

So to connect signals + slots, would it look like this?

@connect(ui.ThreadProgress1, SIGNAL(valueChanged(int)), bftdv1, SLOT(UpdateProgressBar(int)));@

Because while that is not throwing any errors, it's also not doing anything. In fact it looks like when I press the button to start the test, nothing happens at all. [/quote]Share your code (at least your class declarations/headers) -- it will be easier to see your plan, and to see what went wrong.

Also, do check this:[quote author="JKSH" date="1350909892"][quote author="NullCoding" date="1350843257"]But it doesn't do anything at all.[/quote]From your code, your VerboseWindow::BeginNormalVerboseTest() function should be called each time you start() the thread... unless:

The VerboseWindows got destroyed too early (did they?), or
Your program doesn't have an event loop (does your main() function call QApplication::exec() ?)[/quote]

[quote]And I'm still a bit lost as to how I should update the GUI from another thread. Let's say it finds a factor. I have code that fills in text boxes and stuff like that, but unsure how to call it. Do I call the function itself? That seems redundant if I am also connecting stuff to things, but even then I don't know what I should connect!

Do I connect the text box to the function? There's more than one modified by that function. I'm very confused, actually.[/quote]No, it's dangerous to make direct function calls across different threads -- QObjects are not thread-safe. A queued signal+slot connection automatically waits till it's safe to execute functions in a different thread.

When your Worker Object decides that it's time to update the GUI, it should emit a signal, which is connected to the slot that "fills in text boxes and stuff like that" (see also line #43 in my last example). Your example has the correct idea: [quote author="NullCoding" date="1350940920"]
@connect(ui.ThreadProgress1, SIGNAL(valueChanged(int)), bftdv1, SLOT(UpdateProgressBar(int)));@
[/quote]

We just need to debug the rest of your code to see why it's not doing what it's expected to do. Again, share your class declarations

i.g.chernyakov

Let's add some code at the example.

@ // Let's roll
factorialThread.start();
numberBox.show();
resultLabel.show();

int i = a.exec&#40;&#41;;

if (factorialThread.isRunning())
    factorialThread.terminate();

factorialThread.wait();

return i;

}
@

This code does not result in a “Destroyed while thread was still running” message.

JKSH

QThread::terminate() is a dangerous function... I'd use QThread::exit()

NullCoding

Ignore the VerboseInfo class. It contains only a pointer to the GUI which you have said is not necessary.

Class declaration:

@class VerboseInfo;

class VerboseWindow : public QObject
{
Q_OBJECT
public:

VerboseWindow(VerboseInfo * info = 0, int threadNum = 0);
~VerboseWindow();

int threadNum;

IIPStructV1 * piipstructv1;
IIPStructV2 * piipstructv2;
IIPStructV3 * piipstructv3;

VerboseInfo * info;

public slots:

void BeginNormalVerboseTest();
void UpdateProgressBar(int threadNum);
void StopIIPVerboseNormal();
void FactorFound(int threadNum, unsigned long int factor);
int CheckFactors();
void sayPrime(int threadNum);
void sayComposite(int threadNum, int numFacs);

signals:

void testBegun();
void progressBarUpdated();
void testsStopped();
void ReportFactor(int threadNum, unsigned long int factor);
void itIsPrime(int threadNum);
void itIsComposite(int threadNum);

private:

DWORD IIP_Test_Verbose_1(void * pv);
DWORD IIP_Test_Verbose_2(void * pv);
DWORD IIP_Test_Verbose_3(void * pv);

};
@

Example of a function I call when a factor is found:

@ void VerboseWindow::FactorFound(int threadNum, unsigned long int factor)
{
QString nt = QString("Found the factor %1\r\n").arg(factor);
info->ui.factors->append(nt);
QString nt2 = QString("%1\r\n").arg(factor);
if (threadNum == 1)
{
info->ui.ThreadStatus1->appendPlainText(nt2);
}
if (threadNum == 2)
{
info->ui.ThreadStatus2->appendPlainText(nt2);
}
if (threadNum == 3)
{
info->ui.ThreadStatus3->appendPlainText(nt2);
}
info->ui.factorCount->display(info->ui.factorCount->value() + 1);

int quolen = mpz_sizeinbase(piipstructv1->quotient, piipstructv1->base);
CHAR* TheQuotient = new CHAR [quolen+5];
WCHAR* TheQuotientW = new WCHAR [quolen+5];
gmp_sprintf(TheQuotient, "%Zd\r\n\r\n", piipstructv1->quotient);
mbstowcs(TheQuotientW, TheQuotient, quolen+5);
WCHAR ShowFac[1024];
swprintf(ShowFac, 1024, L"%lu's quotient is %ls", factor, TheQuotientW);
QString quoq = QString::fromWCharArray(ShowFac, -1);
info->ui.quotients->append(quoq);
QString thequo = QString::fromWCharArray(TheQuotientW, -1);
info->ui.messages->append(thequo);
ZeroMemory (TheQuotientW, CountOf(TheQuotientW));

emit VerboseWindow::ReportFactor(threadNum, factor);
} @

Still nothing is happening.

JKSH

Debugging questions:

Does FactorFound() actually get called? (print a debug message from inside that function, to check)

What is the ReportFactor() signal connected to?

Does your GUI show up?

Does your program have an event loop? (Does your main() function contain "a.exec()"?)

Also, it looks like VerboseWindow is the Worker, not the GUI. Is that correct? If so, then VerboseWindow shouldn't be updating the progress bar (or writing plain text, or displaying the factor count) -- that's the GUI's job. When you want to update your progress bar, emit a signal with the info, and connect that to a GUI slot:
@
// Inside the VerboseWorker: (I renamed it for clarity, since the worker is not a window)
void VerboseWorker::someFunction()
{
...
emit progressed(threadNum, percentageProgress);
}

// Externally:
void ProgramEngine::someFunction()
{
...
connect(verboseWorker, SIGNAL(progressed(int, int)), gui, SLOT(updateProgressBar(int, int)));
}

// Inside the GUI:
void Gui::updateProgressBar(int threadNum, int percentage)
{
switch (threadNum)
{
case 1:
this->progressBar1->setValue(percentage);
break;
case 2:
this->progressBar2->setValue(percentage);
break;
case 3:
this->progressBar3->setValue(percentage);
break;
}
}
@

Remember: The GUI can only be updated from the main thread. Qt won't let you update the GUI from any other thread.

NullCoding

According to the task manager, the test doesn't even run because it's not using any CPU. There certainly is an event loop, though.

@ int main(int argc, char *argv[])
{
QApplication a(argc, argv);
IsItPrimeQT_200 w; // <-- main application class
w.show();
return a.exec();
}
@

Hmm I'll re-work the whole class thing.

It appears I was using signals and slots completely backwards. Great. Well, I'm self-taught, and this is better than the rocky start I got off to with C++ to begin with (read: this program doesn't use 4GiB of RAM)

JKSH

Yeah, self-teaching does lead to all sorts of adventures :) But it's definitely worth it.

Keep us posted on your progress!

NullCoding

I've gotten the start function to work properly. By that I mean that all the signals and slots are connected properly. I re-wrote the entire testing procedure so that it communicates with the GUI through signals/slots as it's meant to. No more passing GUI as a pointer or silly stuff like that.

However I am still getting "QThread: Destroyed while thread was still running" and I have NO idea why! I never call QThread::stop() or something like that, nor do I explicitly delete the class instances used in the threads created.

What is it that causes a thread manager to destroy the threads prematurely?

NullCoding

For reference, here is how the threads are created:

@ QThread *vt1 = new QThread(this);
QThread *vt2 = new QThread(this);
QThread *vt3 = new QThread(this);

connect(vt1, SIGNAL(started()), v1, SLOT(BeginNormalVerboseTest()));
connect(vt2, SIGNAL(started()), v2, SLOT(BeginNormalVerboseTest()));
connect(vt3, SIGNAL(started()), v3, SLOT(BeginNormalVerboseTest()));

connect(vt1, SIGNAL(finished()), v1, SLOT(deleteLater()));
connect(vt2, SIGNAL(finished()), v2, SLOT(deleteLater()));
connect(vt3, SIGNAL(finished()), v3, SLOT(deleteLater()));

v1->moveToThread(vt1);
v2->moveToThread(vt2);
v3->moveToThread(vt3);

vt1->start();
vt2->start();
vt3->start(); @

Before that, of course, I make a lot of other connections, but figure you'd get the idea.

The BeginNormalVerboseTest() slot simply fills the testing structures from the GUI and then calls a DWORD function that performs the actual test. Once the test has run to completion, it simply cleans up memory and returns 0, which ought to end the thread from which it is called. It worked in Windows API, but I'm not sure what's different here.

KA51O

I'm not sure if this has any effect at all, but I ussually first call moveToThread and then do the connect statements.
@
QThread* myThread = QThread(this);
Worker* myWorker = Worker();
myWorker->moveToThread(myThread);
connect(myThread, SIGNAL(started()), myWorker, SLOT(doTheJob()));
connect(myWorker, SIGNAL(jobDone()), myThread, SLOT(quit()));
connect(myWorker, SIGNAL(jobDone()), myWorker, SLOT(deleteLater()));
connect(myThread, SIGNAL(finished()), myThread, SLOT(deleteLater()));
@

-If you don't expilicitly give the "connection type":http://qt-project.org/doc/qt-4.8/qt.html#ConnectionType-enum as a parameter in the connect statement Qt chooses the appropriate type for you. My fear is that if you first connect and then moveToThread, the connection type DirectConnection is selected since both objects up to this point live in the same thread. After the moveToThread the QueuedConnection type must be set and I'm not sure if this is reset after calling moveToThread.-

Edit: After having a second look at the Qt::ConnectionType docs I realize that the AutoConnection type seems to select the connection type during runtime (i.e. when the signal is emitted). So the order in which you call connect and moveToThread doesn't matter.

JKSH

[quote author="NullCoding" date="1351171052"]I've gotten the start function to work properly. By that I mean that all the signals and slots are connected properly. I re-wrote the entire testing procedure so that it communicates with the GUI through signals/slots as it's meant to. No more passing GUI as a pointer or silly stuff like that.[/quote]Great! That really helps with modularization in Qt-based programs.

[quote]However I am still getting "QThread: Destroyed while thread was still running" and I have NO idea why! I never call QThread::stop() or something like that, nor do I explicitly delete the class instances used in the threads created.

What is it that causes a thread manager to destroy the threads prematurely?[/quote]The warning is shown if the QThread object is destroyed prematurely, before the thread stops/finishes/quits. You'll need to find out why they're being deleted, and stop it from happening.

[quote]
@QThread *vt1 = new QThread(this);@
[/quote]Regarding your premature deletions, my guess is that the parent object that owns the vt1, vt2 and vt3 is getting deleted prematurely. (By passing "this" into the constructor, "this" becomes the parent. Qt uses a parent-child relationship for memory management -- deleting a parent will delete all its children)

[quote]Once the test has run to completion, it simply cleans up memory and returns 0, which ought to end the thread from which it is called. It worked in Windows API, but I’m not sure what’s different here.[/quote]The difference is, QThread-created threads each have an event loop -- without one, signals/slots don't work.

That means the thread doesn't stop when your function (slot) returns; it just idles, until the next signal comes along and invokes the next slot. To stop a QThread-created thread, you'd have to call (externally):
@
vt1->exit();
// or
vt1->quit();
@

NullCoding

I have gotten it to work! I also added a stopping function since there is a "stop" button in the GUI. It exits the threads safely and there are no more premature destructions.

Odd thing is, each thread seems to do everything three times. It's odd. Also, the GUI locks up. I thought by using separate threads, I would be avoiding exactly that.

All I'm trying to do is update the GUI as the tests are in progress in another thread. Is that too much to ask? Something tells me Qt doesn't really want to do that. It leaves the GUI accessible, but very very laggy.

JKSH

[quote author="NullCoding" date="1351259018"]Odd thing is, each thread seems to do everything three times.[/quote]Let's have a look at your code

[quote]Also, the GUI locks up. I thought by using separate threads, I would be avoiding exactly that.[/quote]Is it just the GUI, or your whole machine? If it's the latter, it just means your laptop doesn't have enough grunt, and no amount of multithreading will help.

[quote]All I'm trying to do is update the GUI as the tests are in progress in another thread. Is that too much to ask? Something tells me Qt doesn't really want to do that. It leaves the GUI accessible, but very very laggy.[/quote]How regularly is the GUI getting updated?