Fix some data races (#890)

m-seker · web-flow · commit 126c18e0037f · 2020-07-22T16:43:24.000+02:00
diff --git a/include/effectengine/Effect.h b/include/effectengine/Effect.h
@@ -18,6 +18,8 @@
 #include <utils/Components.h>
 #include <utils/Image.h>
 
+#include <atomic>
+
 class Hyperion;
 class Logger;
 
@@ -88,7 +90,7 @@ class Effect : public QThread
 
 	Logger *_log;
 	// Reflects whenever this effects should interupt (timeout or external request)
-	bool _interupt = false;
+	std::atomic<bool> _interupt {};
 
 	QSize           _imageSize;
 	QImage          _image;
diff --git a/include/hyperion/Hyperion.h b/include/hyperion/Hyperion.h
@@ -79,11 +79,6 @@ class Hyperion : public QObject
 	///
 	const quint8 & getInstanceIndex() { return _instIndex; }
 
-	///
-	/// Returns the number of attached leds
-	///
-	unsigned getLedCount() const;
-
 	///
 	/// @brief Return the size of led grid
 	///
@@ -92,8 +87,6 @@ class Hyperion : public QObject
 	/// gets the methode how image is maped to leds
 	const int & getLedMappingType();
 
-	int getLatchTime() const;
-
 	/// forward smoothing config
 	unsigned addSmoothingConfig(int settlingTime_ms, double ledUpdateFrequency_hz=25.0, unsigned updateDelay=0);
 	unsigned updateSmoothingConfig(unsigned id, int settlingTime_ms=200, double ledUpdateFrequency_hz=25.0, unsigned updateDelay=0);
@@ -113,6 +106,12 @@ class Hyperion : public QObject
 	LedDevice * getActiveDevice() const;
 
 public slots:
+	int getLatchTime() const;
+
+	///
+	/// Returns the number of attached leds
+	///
+	unsigned getLedCount() const;
 
 	///
 	/// @brief  Register a new input by priority, the priority is not active (timeout -100 isn't muxer recognized) until you start to update the data with setInput()
diff --git a/include/webserver/WebServer.h b/include/webserver/WebServer.h
@@ -39,16 +39,6 @@ class WebServer : public QObject {
 	void start();
 	void stop();
 
-	quint16 getPort() const { return _port; }
-
-	/// check if server has been inited
-	bool isInited() const { return _inited; }
-
-	///
-	/// @brief Set a new description, if empty the description is NotFound for clients
-	///
-	void setSSDPDescription(const QString & desc);
-
 signals:
 	///
 	/// @emits whenever server is started or stopped (to sync with SSDPHandler)
@@ -78,6 +68,16 @@ public slots:
 	///
 	void handleSettingsUpdate(const settings::type& type, const QJsonDocument& config);
 
+	///
+	/// @brief Set a new description, if empty the description is NotFound for clients
+	///
+	void setSSDPDescription(const QString & desc);
+
+	/// check if server has been inited
+	bool isInited() const { return _inited; }
+
+	quint16 getPort() const { return _port; }
+
 private:
 	QJsonDocument        _config;
 	bool				 _useSsl;
diff --git a/libsrc/effectengine/Effect.cpp b/libsrc/effectengine/Effect.cpp
@@ -53,6 +53,9 @@ Effect::Effect(Hyperion *hyperion, int priority, int timeout, const QString &scr
 
 Effect::~Effect()
 {
+	requestInterruption();
+	wait();
+
 	delete _painter;
 	_imageStack.clear();
 }
@@ -82,10 +85,14 @@ void Effect::run()
 	PyModule_AddObject(module, "__effectObj", PyCapsule_New((void*)this, "hyperion.__effectObj", nullptr));
 
 	// add ledCount variable to the interpreter
-	PyObject_SetAttrString(module, "ledCount", Py_BuildValue("i", _hyperion->getLedCount()));
+	unsigned ledCount = 0;
+	QMetaObject::invokeMethod(_hyperion, "getLedCount", Qt::BlockingQueuedConnection, Q_RETURN_ARG(unsigned, ledCount));
+	PyObject_SetAttrString(module, "ledCount", Py_BuildValue("i", ledCount));
 
 	// add minimumWriteTime variable to the interpreter
-	PyObject_SetAttrString(module, "latchTime", Py_BuildValue("i", _hyperion->getLatchTime()));
+	int latchTime = 0;
+	QMetaObject::invokeMethod(_hyperion, "getLatchTime", Qt::BlockingQueuedConnection, Q_RETURN_ARG(int, latchTime));
+	PyObject_SetAttrString(module, "latchTime", Py_BuildValue("i", latchTime));
 
 	// add a args variable to the interpreter
 	PyObject_SetAttrString(module, "args", EffectModule::json2python(_args));
diff --git a/libsrc/effectengine/EffectModule.cpp b/libsrc/effectengine/EffectModule.cpp
@@ -144,7 +144,7 @@ PyObject* EffectModule::wrapSetColor(PyObject *self, PyObject *args)
 		{
 			getEffect()->_colors.fill(color);
 			QVector<ColorRgb> _cQV = getEffect()->_colors;
-			getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);
+			emit getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);
 			Py_RETURN_NONE;
 		}
 		return nullptr;
@@ -163,7 +163,7 @@ PyObject* EffectModule::wrapSetColor(PyObject *self, PyObject *args)
 					char * data = PyByteArray_AS_STRING(bytearray);
 					memcpy(getEffect()->_colors.data(), data, length);
 					QVector<ColorRgb> _cQV = getEffect()->_colors;
-					getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);
+					emit getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);
 					Py_RETURN_NONE;
 				}
 				else
@@ -218,7 +218,7 @@ PyObject* EffectModule::wrapSetImage(PyObject *self, PyObject *args)
 				Image<ColorRgb> image(width, height);
 				char * data = PyByteArray_AS_STRING(bytearray);
 				memcpy(image.memptr(), data, length);
-				getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);
+				emit getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);
 				Py_RETURN_NONE;
 			}
 			else
@@ -375,7 +375,7 @@ PyObject* EffectModule::wrapImageShow(PyObject *self, PyObject *args)
 	}
 
 	memcpy(image.memptr(), binaryImage.data(), binaryImage.size());
-	getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);
+	emit getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);
 
 	return Py_BuildValue("");
 }
diff --git a/libsrc/ssdp/SSDPHandler.cpp b/libsrc/ssdp/SSDPHandler.cpp
@@ -54,7 +54,10 @@ void SSDPHandler::initServer()
 	}
 
 	// startup if localAddress is found
-	if(!_localAddress.isEmpty() && _webserver->isInited())
+	bool isInited = false;
+	QMetaObject::invokeMethod(_webserver, "isInited", Qt::BlockingQueuedConnection, Q_RETURN_ARG(bool, isInited));
+
+	if(!_localAddress.isEmpty() && isInited)
 	{
 		handleWebServerStateChange(true);
 	}
@@ -100,14 +103,14 @@ void SSDPHandler::handleWebServerStateChange(const bool newState)
 	if(newState)
 	{
 		// refresh info
-		_webserver->setSSDPDescription(buildDesc());
+		QMetaObject::invokeMethod(_webserver, "setSSDPDescription", Qt::BlockingQueuedConnection, Q_ARG(QString, buildDesc()));
 		setDescriptionAddress(getDescAddress());
 		if(start())
 			sendAnnounceList(true);
 	}
 	else
 	{
-		_webserver->setSSDPDescription("");
+		QMetaObject::invokeMethod(_webserver, "setSSDPDescription", Qt::BlockingQueuedConnection, Q_ARG(QString, ""));
 		sendAnnounceList(false);
 		stop();
 	}
@@ -124,7 +127,7 @@ void SSDPHandler::handleNetworkConfigurationChanged(const QNetworkConfiguration
 
 		// update desc & notify new ip
 		_localAddress = localAddress;
-		_webserver->setSSDPDescription(buildDesc());
+		QMetaObject::invokeMethod(_webserver, "setSSDPDescription", Qt::BlockingQueuedConnection, Q_ARG(QString, buildDesc()));
 		setDescriptionAddress(getDescAddress());
 		sendAnnounceList(true);
 	}
@@ -177,7 +180,9 @@ QString SSDPHandler::getDescAddress() const
 
 QString SSDPHandler::getBaseAddress() const
 {
-	return QString("http://%1:%2/").arg(_localAddress).arg(_webserver->getPort());
+	quint16 port = 0;
+	QMetaObject::invokeMethod(_webserver, "getPort", Qt::BlockingQueuedConnection, Q_RETURN_ARG(quint16, port));
+	return QString("http://%1:%2/").arg(_localAddress).arg(port);
 }
 
 QString SSDPHandler::buildDesc() const
@@ -195,3 +200,4 @@ void SSDPHandler::sendAnnounceList(const bool alive)
 		alive ? SSDPServer::sendAlive(entry) : SSDPServer::sendByeBye(entry);
 	}
 }
+
diff --git a/libsrc/webserver/WebServer.cpp b/libsrc/webserver/WebServer.cpp
@@ -12,7 +12,6 @@
 // netUtil
 #include <utils/NetUtils.h>
 
-
 WebServer::WebServer(const QJsonDocument& config, const bool& useSsl, QObject * parent)
 	:  QObject(parent)
 	, _config(config)
diff --git a/src/hyperiond/hyperiond.cpp b/src/hyperiond/hyperiond.cpp
@@ -175,31 +175,32 @@ void HyperionDaemon::freeObjects()
 
 	// destroy network first as a client might want to access hyperion
 	delete _jsonServer;
-	_flatBufferServer->thread()->quit();
-	_flatBufferServer->thread()->wait(1000);
-	delete _flatBufferServer->thread();
-	delete _flatBufferServer;
 
-	_protoServer->thread()->quit();
-	_protoServer->thread()->wait(1000);
-	delete _protoServer->thread();
-	delete _protoServer;
+	auto flatBufferServerThread = _flatBufferServer->thread();
+	flatBufferServerThread->quit();
+	flatBufferServerThread->wait();
+	delete flatBufferServerThread;
+
+	auto protoServerThread = _protoServer->thread();
+	protoServerThread->quit();
+	protoServerThread->wait();
+	delete protoServerThread;
 
 	//ssdp before webserver
-	_ssdp->thread()->quit();
-	_ssdp->thread()->wait(1000);
-	delete _ssdp->thread();
-	delete _ssdp;
+	auto ssdpThread = _ssdp->thread();
+	ssdpThread->quit();
+	ssdpThread->wait();
+	delete ssdpThread;
 
-	_webserver->thread()->quit();
-	_webserver->thread()->wait(1000);
-	delete _webserver->thread();
-	delete _webserver;
+	auto webserverThread =_webserver->thread();
+	webserverThread->quit();
+	webserverThread->wait();
+	delete webserverThread;
 
-	_sslWebserver->thread()->quit();
-	_sslWebserver->thread()->wait(1000);
-	delete _sslWebserver->thread();
-	delete _sslWebserver;
+	auto sslWebserverThread =_sslWebserver->thread();
+	sslWebserverThread->quit();
+	sslWebserverThread->wait();
+	delete sslWebserverThread;
 
 #ifdef ENABLE_CEC
 	_cecHandler->thread()->quit();
@@ -247,6 +248,7 @@ void HyperionDaemon::startNetworkServices()
 	fbThread->setObjectName("FlatBufferServerThread");
 	_flatBufferServer->moveToThread(fbThread);
 	connect(fbThread, &QThread::started, _flatBufferServer, &FlatBufferServer::initServer);
+	connect(fbThread, &QThread::finished, _flatBufferServer, &FlatBufferServer::deleteLater);
 	connect(this, &HyperionDaemon::settingsChanged, _flatBufferServer, &FlatBufferServer::handleSettingsUpdate);
 	fbThread->start();
 
@@ -256,6 +258,7 @@ void HyperionDaemon::startNetworkServices()
 	pThread->setObjectName("ProtoServerThread");
 	_protoServer->moveToThread(pThread);
 	connect(pThread, &QThread::started, _protoServer, &ProtoServer::initServer);
+	connect(pThread, &QThread::finished, _protoServer, &ProtoServer::deleteLater);
 	connect(this, &HyperionDaemon::settingsChanged, _protoServer, &ProtoServer::handleSettingsUpdate);
 	pThread->start();
 
@@ -265,6 +268,7 @@ void HyperionDaemon::startNetworkServices()
 	wsThread->setObjectName("WebServerThread");
 	_webserver->moveToThread(wsThread);
 	connect(wsThread, &QThread::started, _webserver, &WebServer::initServer);
+	connect(wsThread, &QThread::finished, _webserver, &WebServer::deleteLater);
 	connect(this, &HyperionDaemon::settingsChanged, _webserver, &WebServer::handleSettingsUpdate);
 	wsThread->start();
 
@@ -274,6 +278,7 @@ void HyperionDaemon::startNetworkServices()
 	sslWsThread->setObjectName("SSLWebServerThread");
 	_sslWebserver->moveToThread(sslWsThread);
 	connect(sslWsThread, &QThread::started, _sslWebserver, &WebServer::initServer);
+	connect(sslWsThread, &QThread::finished, _sslWebserver, &WebServer::deleteLater);
 	connect(this, &HyperionDaemon::settingsChanged, _sslWebserver, &WebServer::handleSettingsUpdate);
 	sslWsThread->start();
 
@@ -283,6 +288,7 @@ void HyperionDaemon::startNetworkServices()
 	ssdpThread->setObjectName("SSDPThread");
 	_ssdp->moveToThread(ssdpThread);
 	connect(ssdpThread, &QThread::started, _ssdp, &SSDPHandler::initServer);
+	connect(ssdpThread, &QThread::finished, _ssdp, &SSDPHandler::deleteLater);
 	connect(_webserver, &WebServer::stateChange, _ssdp, &SSDPHandler::handleWebServerStateChange);
 	connect(this, &HyperionDaemon::settingsChanged, _ssdp, &SSDPHandler::handleSettingsUpdate);
 	ssdpThread->start();

Original file line number	Diff line number	Diff line change
`@@ -144,7 +144,7 @@ PyObject* EffectModule::wrapSetColor(PyObject self, PyObject args)`
`144`	`144`	`{`
`145`	`145`	`getEffect()->_colors.fill(color);`
`146`	`146`	`QVector<ColorRgb> _cQV = getEffect()->_colors;`
`147`		`- getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);`
	`147`	`+ emit getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);`
`148`	`148`	`Py_RETURN_NONE;`
`149`	`149`	`}`
`150`	`150`	`return nullptr;`
`@@ -163,7 +163,7 @@ PyObject* EffectModule::wrapSetColor(PyObject self, PyObject args)`
`163`	`163`	`char * data = PyByteArray_AS_STRING(bytearray);`
`164`	`164`	`memcpy(getEffect()->_colors.data(), data, length);`
`165`	`165`	`QVector<ColorRgb> _cQV = getEffect()->_colors;`
`166`		`- getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);`
	`166`	`+ emit getEffect()->setInput(getEffect()->_priority, std::vector<ColorRgb>( _cQV.begin(), _cQV.end() ), timeout, false);`
`167`	`167`	`Py_RETURN_NONE;`
`168`	`168`	`}`
`169`	`169`	`else`
`@@ -218,7 +218,7 @@ PyObject* EffectModule::wrapSetImage(PyObject self, PyObject args)`
`218`	`218`	`Image<ColorRgb> image(width, height);`
`219`	`219`	`char * data = PyByteArray_AS_STRING(bytearray);`
`220`	`220`	`memcpy(image.memptr(), data, length);`
`221`		`- getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);`
	`221`	`+ emit getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);`
`222`	`222`	`Py_RETURN_NONE;`
`223`	`223`	`}`
`224`	`224`	`else`
`@@ -375,7 +375,7 @@ PyObject* EffectModule::wrapImageShow(PyObject self, PyObject args)`
`375`	`375`	`}`
`376`	`376`
`377`	`377`	`memcpy(image.memptr(), binaryImage.data(), binaryImage.size());`
`378`		`- getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);`
	`378`	`+ emit getEffect()->setInputImage(getEffect()->_priority, image, timeout, false);`
`379`	`379`
`380`	`380`	`return Py_BuildValue("");`
`381`	`381`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,10 @@ void SSDPHandler::initServer()`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`// startup if localAddress is found`
`57`		`- if(!_localAddress.isEmpty() && _webserver->isInited())`
	`57`	`+ bool isInited = false;`
	`58`	`+ QMetaObject::invokeMethod(_webserver, "isInited", Qt::BlockingQueuedConnection, Q_RETURN_ARG(bool, isInited));`
	`59`	`+`
	`60`	`+ if(!_localAddress.isEmpty() && isInited)`
`58`	`61`	`{`
`59`	`62`	`handleWebServerStateChange(true);`
`60`	`63`	`}`
`@@ -100,14 +103,14 @@ void SSDPHandler::handleWebServerStateChange(const bool newState)`
`100`	`103`	`if(newState)`
`101`	`104`	`{`
`102`	`105`	`// refresh info`
`103`		`- _webserver->setSSDPDescription(buildDesc());`
	`106`	`+ QMetaObject::invokeMethod(_webserver, "setSSDPDescription", Qt::BlockingQueuedConnection, Q_ARG(QString, buildDesc()));`
`104`	`107`	`setDescriptionAddress(getDescAddress());`
`105`	`108`	`if(start())`
`106`	`109`	`sendAnnounceList(true);`
`107`	`110`	`}`
`108`	`111`	`else`
`109`	`112`	`{`
`110`		`- _webserver->setSSDPDescription("");`
	`113`	`+ QMetaObject::invokeMethod(_webserver, "setSSDPDescription", Qt::BlockingQueuedConnection, Q_ARG(QString, ""));`
`111`	`114`	`sendAnnounceList(false);`
`112`	`115`	`stop();`
`113`	`116`	`}`
`@@ -124,7 +127,7 @@ void SSDPHandler::handleNetworkConfigurationChanged(const QNetworkConfiguration`
`124`	`127`
`125`	`128`	`// update desc & notify new ip`
`126`	`129`	`_localAddress = localAddress;`
`127`		`- _webserver->setSSDPDescription(buildDesc());`
	`130`	`+ QMetaObject::invokeMethod(_webserver, "setSSDPDescription", Qt::BlockingQueuedConnection, Q_ARG(QString, buildDesc()));`
`128`	`131`	`setDescriptionAddress(getDescAddress());`
`129`	`132`	`sendAnnounceList(true);`
`130`	`133`	`}`
`@@ -177,7 +180,9 @@ QString SSDPHandler::getDescAddress() const`
`177`	`180`
`178`	`181`	`QString SSDPHandler::getBaseAddress() const`
`179`	`182`	`{`
`180`		`- return QString("http://%1:%2/").arg(_localAddress).arg(_webserver->getPort());`
	`183`	`+ quint16 port = 0;`
	`184`	`+ QMetaObject::invokeMethod(_webserver, "getPort", Qt::BlockingQueuedConnection, Q_RETURN_ARG(quint16, port));`
	`185`	`+ return QString("http://%1:%2/").arg(_localAddress).arg(port);`
`181`	`186`	`}`
`182`	`187`
`183`	`188`	`QString SSDPHandler::buildDesc() const`
`@@ -195,3 +200,4 @@ void SSDPHandler::sendAnnounceList(const bool alive)`
`195`	`200`	`alive ? SSDPServer::sendAlive(entry) : SSDPServer::sendByeBye(entry);`
`196`	`201`	`}`
`197`	`202`	`}`
	`203`	`+`