Threading without the headache

[Onanymous]

I was intrigued by the post http://labs.qt.nokia.com/2010/06/17/...oing-it-wrong/ and really like that idea of new way of using threads. So I wrote a simple example, which... does not work

#ifndef THREADs_TEST_H
#define THREADs_TEST_H
 
#if defined(_WIN32)
	#include <windows.h>
#elif defined(__GNUC__)
	#include <unistd.h>
	#define Sleep(x) sleep((x)/1000)
#endif //_WIN32
 
#include <iostream>
#include <QThread>
#include <QObject>
#include <QDialog>
#include <QDialogButtonBox>
#include <QBoxLayout>
#include <QPushButton>
 
class Cycler : public QObject
{
	Q_OBJECT
	public:
		Cycler() : QObject() {};
		virtual ~Cycler(){stop();};
	signals:
		void done();
	public slots:
		void cycle() {
			arbeit = true;
			int i = 0;
			while(arbeit)  {
				Sleep(1000);
				std::cout << i++ << " " << std::flush; 				
			};
			emit done(); };
		void stop(){ arbeit = false;};
	private:
		volatile bool arbeit;
};
 
class CyclerDialog: public QDialog
{
	Q_OBJECT
	public:
		explicit CyclerDialog(QWidget* parent = 0, Qt::WindowFlags f = 0) : QDialog(parent, f){
			QDialogButtonBox *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); 
			QVBoxLayout *main_layout = new QVBoxLayout;
			main_layout->addWidget(buttons);
			setLayout(main_layout);
 
			cycl.moveToThread(&thrd);
			connect(buttons, SIGNAL(accepted()), &thrd, SLOT(start()));
			connect(&thrd, SIGNAL(started()), &cycl, SLOT(cycle()));
			connect(buttons, SIGNAL(rejected()), &cycl, SLOT(stop()));
			connect(&cycl, SIGNAL(done()), &thrd, SLOT(quit())); };
 
		virtual ~CyclerDialog(){thrd.wait(1000);};
	private:
		Cycler  cycl;
		QThread thrd;
};
 
#endif // THREADs_TEST_H

#ifndef THREADs_TEST_H
#define THREADs_TEST_H

#if defined(_WIN32)
	#include <windows.h>
#elif defined(__GNUC__)
	#include <unistd.h>
	#define Sleep(x) sleep((x)/1000)
#endif //_WIN32

#include <iostream>
#include <QThread>
#include <QObject>
#include <QDialog>
#include <QDialogButtonBox>
#include <QBoxLayout>
#include <QPushButton>

class Cycler : public QObject
{
	Q_OBJECT
	public:
		Cycler() : QObject() {};
		virtual ~Cycler(){stop();};
	signals:
		void done();
	public slots:
		void cycle() {
			arbeit = true;
			int i = 0;
			while(arbeit)  {
				Sleep(1000);
				std::cout << i++ << " " << std::flush; 				
			};
			emit done(); };
		void stop(){ arbeit = false;};
	private:
		volatile bool arbeit;
};

class CyclerDialog: public QDialog
{
	Q_OBJECT
	public:
		explicit CyclerDialog(QWidget* parent = 0, Qt::WindowFlags f = 0) : QDialog(parent, f){
			QDialogButtonBox *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); 
			QVBoxLayout *main_layout = new QVBoxLayout;
			main_layout->addWidget(buttons);
			setLayout(main_layout);
			
			cycl.moveToThread(&thrd);
			connect(buttons, SIGNAL(accepted()), &thrd, SLOT(start()));
			connect(&thrd, SIGNAL(started()), &cycl, SLOT(cycle()));
			connect(buttons, SIGNAL(rejected()), &cycl, SLOT(stop()));
			connect(&cycl, SIGNAL(done()), &thrd, SLOT(quit())); };
		
		virtual ~CyclerDialog(){thrd.wait(1000);};
	private:
		Cycler  cycl;
		QThread thrd;
};

#endif // THREADs_TEST_H

To copy to clipboard, switch view to plain text mode 

#include <QApplication>
#include "threads_test.h"
 
int main(int argc, char *argv[])
{
	QApplication app(argc, argv);
	CyclerDialog window;	
	window.show();
	return(app.exec());
}

#include <QApplication>
#include "threads_test.h"

int main(int argc, char *argv[])
{
	QApplication app(argc, argv);
	CyclerDialog window;	
	window.show();
	return(app.exec());
}

To copy to clipboard, switch view to plain text mode 

It is supposed to start printing numbers in std::cout when OK is clicked, and stop when CANCEL is clicked, but apparently stop() slot is never called. WHY????

[mcosta]

Simple.

Your slot cycle never returns, so the stop slot never be called by eventLoop

[Onanymous]

Indeed. How do I fix that?

[mcosta]

Use a QTimer and connect a slot to QTimer::timeout signal.

When the thread emits the started signal, call QTimer::start and when you want to stop it, call QTime::stop

[Onanymous]

I came up to another solution, basically to move stop() to CyclerDialog, and initiate Cycler with reference to CyclerDialog::arbeit flag. It seems to work.

class Cycler : public QObject
{
	Q_OBJECT
	public:
		Cycler(volatile bool &stop_flag) : QObject(), arbeit(stop_flag) {};
		virtual ~Cycler(){};
	signals:
		void done();
	public slots:
		void cycle() {
			arbeit = true;
			int i = 0;
			while(arbeit)  {
				Sleep(1000);
				std::cout << i++ << " " << std::flush; 				
			};
			emit done(); };
	private:
		volatile bool& arbeit;
};
 
class CyclerDialog: public QDialog
{
	Q_OBJECT
	public:
		explicit CyclerDialog(QWidget* parent = 0, Qt::WindowFlags f = 0) : QDialog(parent, f), cycl(arbeit){
			QDialogButtonBox *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); 
			QVBoxLayout *main_layout = new QVBoxLayout;
			main_layout->addWidget(buttons);
			setLayout(main_layout);
 
			cycl.moveToThread(&thrd);
			connect(buttons, SIGNAL(accepted()), &thrd, SLOT(start()));
			connect(&thrd, SIGNAL(started()), &cycl, SLOT(cycle()));
			connect(buttons, SIGNAL(rejected()), this, SLOT(stop()));
			connect(&cycl, SIGNAL(done()), &thrd, SLOT(quit())); };
 
		virtual ~CyclerDialog(){stop(); thrd.wait(1000);};
		public slots:
			void stop(){arbeit = false;};
	private:
		volatile bool arbeit;
		Cycler  cycl;
		QThread thrd;
};

class Cycler : public QObject
{
	Q_OBJECT
	public:
		Cycler(volatile bool &stop_flag) : QObject(), arbeit(stop_flag) {};
		virtual ~Cycler(){};
	signals:
		void done();
	public slots:
		void cycle() {
			arbeit = true;
			int i = 0;
			while(arbeit)  {
				Sleep(1000);
				std::cout << i++ << " " << std::flush; 				
			};
			emit done(); };
	private:
		volatile bool& arbeit;
};

class CyclerDialog: public QDialog
{
	Q_OBJECT
	public:
		explicit CyclerDialog(QWidget* parent = 0, Qt::WindowFlags f = 0) : QDialog(parent, f), cycl(arbeit){
			QDialogButtonBox *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel); 
			QVBoxLayout *main_layout = new QVBoxLayout;
			main_layout->addWidget(buttons);
			setLayout(main_layout);
			
			cycl.moveToThread(&thrd);
			connect(buttons, SIGNAL(accepted()), &thrd, SLOT(start()));
			connect(&thrd, SIGNAL(started()), &cycl, SLOT(cycle()));
			connect(buttons, SIGNAL(rejected()), this, SLOT(stop()));
			connect(&cycl, SIGNAL(done()), &thrd, SLOT(quit())); };
		
		virtual ~CyclerDialog(){stop(); thrd.wait(1000);};
		public slots:
			void stop(){arbeit = false;};
	private:
		volatile bool arbeit;
		Cycler  cycl;
		QThread thrd;
};

To copy to clipboard, switch view to plain text mode 

[mcosta]

In general, Synchronizing threads through a shared variable is not a good idea; you should, at least, protect the access with a mutex.

The problem of your solution is that the eventLoop of the thread cannot elaborate other events because it's blocked by your slot

[Onanymous]

In general you are right, I do not need a thread for the type of problem I made an example for, QTimer would be sufficient in fact.
But I am rather intertested in a situation when the thread is created just to run some heavy work, and I only need a tool to stop it, the flag then is not really shared, Cycler needs only read access to it, so no mutexes required. To ensure that I could probably make a const function in CyclerDialog returning bool, and initiate the Cycler with a const pointer to CyclerDialog.

[mcosta]

In the cases you descrided you can (and should) use QThreadPool/QRunnable or QtConcurrent.

For an overview read here

[MarekR22]

Don't use sleep use timer as mcosta suggested (in you case this is best solution).
Another solution is to call event loop inside your loop:

QApplication::processEvents();

QApplication::processEvents();

To copy to clipboard, switch view to plain text mode 

And last a bit strange solution is replace loop with queued meta calls.

void cycle() {
   if (arbeit)  {
      Sleep(1000);
      std::cout << i++ << " " << std::flush;
      metaObject->invokeMethod(this, "cycle", Qt::QueuedConnection);
   } else { 
           emit done();
   } 
}

void cycle() {
   if (arbeit)  {
      Sleep(1000);
      std::cout << i++ << " " << std::flush;
      metaObject->invokeMethod(this, "cycle", Qt::QueuedConnection);
   } else { 
           emit done();
   } 
}

To copy to clipboard, switch view to plain text mode