Clean-up shared pointers when stopping instances, plus support tracking

Author: Lord-Grey

include/api/JsonCallbacks.h

@@ -179,6 +179,14 @@ private slots:
 	///
 	void handleEventUpdate(const Event &event);
 
+	///
+	/// @brief Handle whenever the state of a instance (HyperionIManager) changes according to enum instanceState
+	/// @param instaneState  A state from enum
+	/// @param instanceId    The index of instance
+	/// @param name          The name of the instance, just available with H_CREATED
+	///
+	void handleInstanceStateChange(InstanceState state, quint8 instanceId, const QString &name = QString());
+
 private:
 
 	/// construct callback msg
@@ -194,7 +202,7 @@ private slots:
 	QString _peerAddress;
 
 	/// pointer of comp register
-	ComponentRegister* _componentRegister;
+	QSharedPointer<ComponentRegister> _componentRegister;
 
 	/// priority muxer instance
 	QSharedPointer<PriorityMuxer> _prioMuxer;

include/effectengine/EffectEngine.h

@@ -9,9 +9,6 @@
 #include <QJsonArray>
 #include <QEventLoop>
 
-// Hyperion includes
-#include <hyperion/Hyperion.h>
-
 // Effect engine includes
 #include <effectengine/EffectDefinition.h>
 #include <effectengine/Effect.h>
@@ -21,6 +18,7 @@
 #include <hyperion/LinearColorSmoothing.h>
 
 // pre-declaration
+class Hyperion;
 class Effect;
 class EffectFileHandler;
 

include/hyperion/BGEffectHandler.h

@@ -2,12 +2,13 @@
 #define BGEFFECTHANDLER_H
 
 #include <utils/Logger.h>
-#include <hyperion/Hyperion.h>
 #include <utils/settings.h>
 
 #include <QObject>
 #include <QJsonDocument>
 
+class Hyperion;
+
 ///
 /// @brief Handle the background Effect settings, reacts on runtime to settings changes
 ///

include/hyperion/Hyperion.h

@@ -15,6 +15,7 @@
 #include <QScopedPointer>
 #include <QSharedPointer>
 #include <QElapsedTimer>
+#include <QThread>
 
 // hyperion-utils includes
 #include <utils/Image.h>
@@ -25,10 +26,19 @@
 // Hyperion includes
 #include <hyperion/LedString.h>
 #include <hyperion/PriorityMuxer.h>
+#include <hyperion/MultiColorAdjustment.h>
 #include <hyperion/ColorAdjustment.h>
 #include <hyperion/ComponentRegister.h>
 
+#include <hyperion/SettingsManager.h>
+#include <hyperion/CaptureCont.h>
+#include <hyperion/BGEffectHandler.h>
+
+#include <leddevice/LedDeviceWrapper.h>
+#include <boblightserver/BoblightServer.h>
+
 #if defined(ENABLE_EFFECTENGINE)
+#include <effectengine/EffectEngine.h>
 #include <effectengine/ActiveEffectDefinition.h>
 #endif
 
@@ -38,22 +48,8 @@
 #include <utils/settings.h>
 
 // Forward class declaration
-class HyperionDaemon;
-class HyperionIManager;
 class ImageProcessor;
 class LinearColorSmoothing;
-#if defined(ENABLE_EFFECTENGINE)
-class EffectEngine;
-#endif
-class MultiColorAdjustment;
-class ColorAdjustment;
-class SettingsManager;
-class BGEffectHandler;
-class CaptureCont;
-#if defined(ENABLE_BOBLIGHT_SERVER)
-class BoblightServer;
-#endif
-class LedDeviceWrapper;
 class Logger;
 
 ///
@@ -69,9 +65,10 @@ class Hyperion : public QObject
 	using InputInfo = PriorityMuxer::InputInfo;
 
 	///
-	/// Destructor; cleans up resources
+	/// @brief Constructs the Hyperion instance
+	/// @param  instance  The instance index
 	///
-	~Hyperion() override;
+	explicit Hyperion(quint8 instance, QObject* parent = nullptr);
 
 	ImageProcessor* getImageProcessor() const { return _imageProcessor.get(); }
 
@@ -205,7 +202,7 @@ public slots:
 	/// @brief Get a pointer to the effect engine
 	/// @return     EffectEngine instance pointer
 	///
-	EffectEngine* getEffectEngineInstance() const { return _effectEngine.get(); }
+	QSharedPointer<EffectEngine> getEffectEngineInstance() const { return _effectEngine; }
 
 	/// Run the specified effect on the given priority channel and optionally specify a timeout
 	/// @param effectName Name of the effec to run
@@ -321,7 +318,7 @@ public slots:
 	/// @brief Get the component Register
 	/// return Component register pointer
 	///
-	ComponentRegister* getComponentRegister() const { return _componentRegister.get(); }
+	QSharedPointer<ComponentRegister> getComponentRegister() const { return _componentRegister; }
 
 	///
 	/// @brief Called from components to update their current state. DO NOT CALL FROM USERS
@@ -512,15 +509,6 @@ private slots:
 	void handleSourceAvailability(int priority);
 
 private:
-	friend class HyperionDaemon;
-
-	///
-	/// @brief Constructs the Hyperion instance, just accessible for HyperionIManager
-	/// @param  instance  The instance index
-	///
-	explicit Hyperion(quint8 instance, QObject* parent = nullptr);
-
-	friend class HyperionIManager; // Grant HyperionIManager access to private members
 
 	void updateLedColorAdjustment(int ledCount, const QJsonObject& colors);
 	void updateLedLayout(const QJsonArray& ledLayout);
@@ -556,7 +544,7 @@ private slots:
 
 #if defined(ENABLE_EFFECTENGINE)
 	/// Effect engine
-	QScopedPointer<EffectEngine, QScopedPointerDeleteLater> _effectEngine;
+	QSharedPointer<EffectEngine> _effectEngine;
 #endif
 
 	/// Logger instance

include/utils/TrackedMemory.h

@@ -0,0 +1,107 @@
+#ifndef TRACKEDMEMORY_H
+#define TRACKEDMEMORY_H
+
+#include <QSharedPointer>
+#include <QObject>
+#include <QThread>
+#include <QString>
+#include <typeinfo>
+#include <type_traits>
+#include <utils/Logger.h>
+
+#define ENABLE_MEMORY_TRACKING 0
+
+#define USE_TRACKED_SHARED_PTR ENABLE_MEMORY_TRACKING
+#define USE_TRACKED_DELETE_LATER ENABLE_MEMORY_TRACKING
+
+#if USE_TRACKED_SHARED_PTR
+	#define MAKE_TRACKED_SHARED(T, ...) makeTrackedShared<T>(__VA_ARGS__)
+#else
+	#define MAKE_TRACKED_SHARED(T, ...) QSharedPointer<T>(new T(__VA_ARGS__), &QObject::deleteLater)
+#endif
+
+#if USE_TRACKED_DELETE_LATER
+	#define DELETE_LATER_FN(T) trackedDeleteLater<T>
+#else
+	#define DELETE_LATER_FN(T) &QObject::deleteLater
+#endif
+
+// Deleter function template
+template<typename T>
+void trackedDeleteLater(T* ptr)
+{
+	if (!ptr)
+		return;
+
+	QString subComponent = "__";
+	QString typeName;
+
+	if constexpr (std::is_base_of<QObject, T>::value)
+	{
+		QVariant prop = ptr->property("instance");
+		if (prop.isValid() && prop.canConvert<QString>())
+		{
+			subComponent = prop.toString();
+		}
+		typeName = ptr->metaObject()->className();
+	}
+	else
+	{
+		typeName = typeid(T).name();
+	}
+
+	Logger* log = Logger::getInstance("MEMORY", subComponent);
+
+	Debug(log, "Deleting object of type '%s' at %p", QSTRING_CSTR(typeName), static_cast<void*>(ptr));
+
+	if constexpr (std::is_base_of<QObject, T>::value)
+	{
+		QThread* thread = ptr->thread();
+		if (thread && thread->isRunning())
+		{
+			ptr->deleteLater();
+			Debug(log, "QObject<%s>::deleteLater() scheduled on thread '%s'", QSTRING_CSTR(typeName), QSTRING_CSTR(thread->objectName()));
+		}
+		else
+		{
+			delete ptr;
+			Debug(log, "QObject<%s> deleted immediately (thread not running).", QSTRING_CSTR(typeName));
+		}
+	}
+	else
+	{
+		delete ptr;
+		Debug(log, "Non-QObject<%s> deleted immediately.", QSTRING_CSTR(typeName));
+	}
+}
+
+// Factory function template to create tracked QSharedPointer
+template<typename T, typename... Args>
+QSharedPointer<T> makeTrackedShared(Args&&... args)
+{
+	T* rawPtr = new T(std::forward<Args>(args)...);
+
+	QString subComponent = "__";
+	QString typeName;
+
+	if constexpr (std::is_base_of<QObject, T>::value)
+	{
+		QVariant prop = rawPtr->property("instance");
+		if (prop.isValid() && prop.canConvert<QString>())
+		{
+			subComponent = prop.toString();
+		}
+		typeName = rawPtr->metaObject()->className();
+	}
+	else
+	{
+		typeName = typeid(T).name();
+	}
+
+	Logger* log = Logger::getInstance("MEMORY", subComponent);
+	Debug(log, "Creating object of type '%s' at %p", QSTRING_CSTR(typeName), static_cast<void*>(rawPtr));
+
+	return QSharedPointer<T>(rawPtr, DELETE_LATER_FN(T));
+}
+
+#endif // TRACKEDMEMORY_H

libsrc/api/JsonAPI.cpp

@@ -1896,17 +1896,6 @@ void JsonAPI::handleInstanceStateChange(InstanceState state, quint8 instanceID,
 {
 	switch (state)
 	{
-	case InstanceState::H_ON_STOP:
-	{
-		if (!_hyperion.isNull() && _hyperion->getInstanceIndex() == instanceID)
-		{
-			_hyperion.reset(nullptr);
-			_jsonCB->setSubscriptionsTo(NO_INSTANCE_ID);
-		}
-	}
-	break;
-
-	case InstanceState::H_STARTING:
 	case InstanceState::H_STARTED:
 	{
 		quint8 const currentInstance = _currInstanceIndex;
@@ -1916,7 +1905,17 @@ void JsonAPI::handleInstanceStateChange(InstanceState state, quint8 instanceID,
 			_jsonCB->setSubscriptionsTo(instanceID);
 		}
 	}
+	break;
+
 	case InstanceState::H_STOPPED:
+	{
+		//Release reference to stopped Hyperion instance
+		_hyperion.clear();
+	}
+	break;
+
+	case InstanceState::H_STARTING:
+	case InstanceState::H_ON_STOP:
 	case InstanceState::H_CREATED:
 	case InstanceState::H_DELETED:
 	break;