Signal and slot to synchronize variable between qthread

RausoV

Hi everybody, here is a issue that is causing me a lot of headaches.
There are two different myObject and my purpose is to keep their MyVar synchronized between them.
The signal and slot approach works well when the objects live in the same thread, but in this case it generates a loop as you can see in the output.
What I'm expecting is the first 4 rows of the output and stop, but with this code the output is an infinite loop.

Thank you for your help.
V.

myclass.cpp

#include "myclass.h"
#include <QDebug>
#include <QThread>

MyClass::MyClass(QObject *parent) : QObject(parent)
{

}

MyClass::~MyClass()
{

}
QString MyClass::getMyVar() const
{
	return myVar;
}

void MyClass::setMyVar(const QString &value)
{
	if(value != myVar)
	{
		qDebug() << QThread::currentThread() << value;
		myVar = value;
		emit myVarChanged(value);
	}
}

myclass.h

#ifndef MYCLASS_H
#define MYCLASS_H

#include <QObject>

class MyClass : public QObject
{
	Q_OBJECT
public:
	explicit MyClass(QObject *parent = 0);
	~MyClass();

	QString getMyVar() const;

signals:
	myVarChanged(QString value);

public slots:
	void setMyVar(const QString &value);

private:
	QString myVar;
};

#endif // MYCLASS_H

worker.cpp

#include "worker.h"

Worker::Worker(QObject *parent) : QObject(parent)
{

}

Worker::~Worker()
{

}

void Worker::init()
{
	myObject = new MyClass();
	connect(myObject, &MyClass::myVarChanged, this, &Worker::myVarChanged);
	setMyVar("first");
	setMyVar("second");
}

void Worker::setMyVar(const QString &value)
{
	myObject->setMyVar(value);
}

worker.h

#ifndef WORKER_H
#define WORKER_H

#include <QObject>
#include "myclass.h"

class Worker : public QObject
{
	Q_OBJECT
public:
	explicit Worker(QObject *parent = 0);
	~Worker();

signals:
	void myVarChanged(QString value);
public slots:
	void init();
	void setMyVar(const QString &value);
private:
	MyClass *myObject;
};

#endif // WORKER_H

main.cpp

#include <QCoreApplication>
#include <QThread>
#include "worker.h"
#include "myclass.h"

int main(int argc, char *argv[])
{
	QCoreApplication a(argc, argv);

	QThread thread;
	Worker worker;
	MyClass myObject;

	worker.moveToThread(&thread);
	QObject::connect(&thread, &QThread::started, &worker, &Worker::init);
	QObject::connect(&myObject, &MyClass::myVarChanged, &worker, &Worker::setMyVar);
	QObject::connect(&worker, &Worker::myVarChanged, &myObject, &MyClass::setMyVar);

	thread.start();

	return a.exec();
}

OUTPUT:

QThread(0x28fe50) "first"
QThread(0x3f7308) "first"
QThread(0x28fe50) "second"
QThread(0x3f7308) "second"
QThread(0x28fe50) "first"
QThread(0x3f7308) "first"
QThread(0x28fe50) "second"
QThread(0x3f7308) "second"
.......

SGaist

Hi and welcome to devnet,

There are mainly two reasons: your MyClass objects have both undefined values for myVar which means they will both change when setMyVar is called.

Then, AFAIK, it's because of the queued connection that results from connecting two objects in different threads. The signal emissions will queue events to the event loop of the thread to which the object has affinity with thus your first call to setMyVar will generate an event sent to the main thread object that will then generate another event that will be sent back to the worker object which will have generated another one in between with the second call to setMyVar.

RausoV

Hi SGaist, thank you for your answer.
I think that the main reason is number two: queued connections. In fact even if I initialize myVar (obviously with a value different from "first", the first set) the infinite loop remains.
This kind of connections it's the only possible between different threads; I tried blocking connections also because I thought that a thread is blocked and cannot send another setMyVar signal before the other thread finished it's myVar update.
Obviously this is only a sample code of what I'm doing, maybe it's a design mistake and so here I'm looking for the right solution.

SGaist

Initialised with a value other than first and second will have the same effect has having them uninitialised.

What exactly are you trying to accomplish ?

RausoV

Like I said, the information that myVar contains has to be equal in two different threads. If one changes so the other has to be updated.
Obviously the code above is a simplification of a bigger one, but it's the problem's core.

J.Hilk

@RausoV Well that problem should be easy enough to solve. Expand void myVarChanged(QString value); by a thread Inique Identifier.

void myVarChanged(QString value, int UniqueID);

then in your slot compare if the ID fits the own ID.
if it does -> do nothing, else accept new value, but don't emit the Signal.

void Worker::setMyVar(const QString &value, int id)
{
       if(id == ownID)
	  myObject->setMyVar(value); //set Variable with signal?
      else
          myObject->setVarWithoutSignal(value);
}

RausoV

Hi J.Hilk,
thank you for joining this discussion. Your idea works well on this example and I had already taken it in consideration, but it changes the myClass implementation. In my project there are several existent classes and I'd like to avoid to change them.
I'd like to change only the worker class or add a new class dedicated to the synchronization purpose.

BjornW

@J.Hilk

Does that truly work, though?

Imagine, in two different treads, the value is set to "A" and "B" respectively and simultaneously. From each thread the corresponding event is posted to the other thread.

A->B: setMyVar("A", id_a)
B->A: setMyVar("B", id_b)

on the receiving end, as the event queue is processed:

A: setMyVar("B", id_b) --> "not my thread" -> set without signal
B: setMyVar("A", id_a) --> "not my thread" -> set without signal

A and B are now out of sync.

What did I miss?

kshegunov

QObject::connect(&myObject, &MyClass::myVarChanged, &worker, &Worker::setMyVar, Qt::BlockingQueuedConnection);
QObject::connect(&worker, &Worker::myVarChanged, &myObject, &MyClass::setMyVar);

Should work fine, but I really doubt the wisdom of doing those shenanigans ...

BjornW

I would separate the objects into two types. One type that is the "master" object and other objects which interact with it.

Disclaimer: I didn't compile this stuff

class MyObject : public QObject
{
   Q_OBJECT
public:
   Q_SIGNAL void valueChanged(QString);
   Q_SLOT void setValue(QString value) //Intended to be called by business logic
   {
      m_value = value;
      emit valueChanged(value);
   }
   Q_SLOT void sync(QString value) //Intended to be called by the "master" object.
   {
      m_value = value;
   }
private:
   QString m_value;
};
...

main.cpp:

MyObject objectA;  //Goes to thread A
MyObject objectB;  //Goes to thread B
MyObject master;   //Goes "somewhere neutral"

//Notify the master object when A/B changes
QObject::connect(&objectA, &MyObject::valueChanged, &master, &MyObject::setValue);
QObject::connect(&objectB, &MyObject::valueChanged, &master, &MyObject::setValue);

//Sync A/B with master
QObject::connect(&master, &MyObject::valueChanged, &objectA, &MyObject::sync);
QObject::connect(&master, &MyObject::valueChanged, &objectB, &MyObject::sync);

QThread threadA;
QThread threadB;
objectA.moveToThread(&threadA);
objectB.moveToThread(&threadB);
threadA.start();
threadB.start();

kshegunov

@BjornW said in Signal and slot to synchronize variable between qthread:

I would separate the objects into two types. One type that is the "master" object and other objects which interact with it.

That's the correct thinking, I consider this whole question to be a design issue. The problem I see is that a dependency is introduced between two QObjects that are in different threads, that is very unnatural to begin with - the two objects shouldn't depend on one another implicitly; it's no mistake that a QObject instance can't have a parent that's living in another thread. In my mind either the dependency should be declared explicitly (e.g. manually synchronizing the objects across the threads in some fashion) or it shouldn't exist at all.