Fix Memory leaks (#1678)

* Refactor to fix #1671

* Add GUI/NonGUI mode to info page

* Do not show lock config, if in non-UI mode

* Updae Changelog

* Correct includes

* Ensure key member initialization - RGB Channels

* Ensure key member initialization - WebServer

* Update RGBChannels

* Fix initialization order

* Fix key when inserting new logger in LoggerMap,
Prepare logBuffer-JSON snapshot view in LoggerManager,
Increase buffered loglines to 500

* Fix Memory leak in GrabberWrapper

* Fix Memory leak in BlackBorderProcessor

* Fix Memory leak in BlackBorderProcessor

* use ninja generator under macos

* Fix BGEffectHandler destruction

* Fix Mdns code

* Clear list after applying qDeleteAll

* Fix deletion of CecHandler

* Fix memory leak caused by wrong buffer allocation

* Remove extra pixel consistently

* Change mDNS to Qt SmartPointers

* Correct removal

* Fix usage of _width/_height (they are the output resolution, not the screen resolution)
That avoids unnecessary resizing of the output image with every transferFrame call

* Move main non Thread Objects to Smart Pointers

* Revert "Move main non Thread Objects to Smart Pointers"

This reverts commit 26102ca963.

* Add missing deletes

* Revert MdnsBrowser chnage

* Revert MdnsBrowser change

* Fix memory leaks related standalone grabber

* Address CodeQL finding

* delete pointer OsxFrameGrabber

---------

Co-authored-by: Paulchen-Panther <16664240+Paulchen-Panther@users.noreply.github.com>

include/blackborder/BlackBorderProcessor.h

@@ -1,4 +1,7 @@
-#pragma once
+#ifndef BLACK_BORDER_PROCESSOR_H
+#define BLACK_BORDER_PROCESSOR_H
+
+#include <memory>
 
 // QT includes
 #include <QJsonObject>
@@ -24,7 +27,7 @@ namespace hyperion
 		Q_OBJECT
 	public:
 		BlackBorderProcessor(Hyperion* hyperion, QObject* parent);
-		~BlackBorderProcessor() override;
+
 		///
 		/// Return the current (detected) border
 		/// @return The current border
@@ -141,7 +144,7 @@ namespace hyperion
 		QString _detectionMode;
 
 		/// The black-border detector
-		BlackBorderDetector* _detector;
+		std::unique_ptr<BlackBorderDetector> _detector;
 
 		/// The current detected border
 		BlackBorder _currentBorder;
@@ -162,3 +165,5 @@ namespace hyperion
 
 	};
 } // end namespace hyperion
+
+#endif // BLACK_BORDER_PROCESSOR_H

include/commandline/ValidatorOption.h

@@ -36,6 +36,11 @@ public:
         : Option(other), validator(validator)
     {}
 
+	~ValidatorOption()
+	{
+		delete validator;
+	}
+
     virtual const QValidator *getValidator() const;
     virtual bool validate(Parser & parser, QString &value) override;
 };

include/grabber/qt/QtGrabber.h

@@ -113,8 +113,8 @@ private:
 	int _display;
 	int _numberOfSDisplays;
 
-	int _calculatedWidth;
-	int _calculatedHeight;
+	int _screenWidth;
+	int _screenHeight;
 	int _src_x;
 	int _src_y;
 	int _src_x_max;

include/grabber/x11/X11Grabber.h

@@ -57,7 +57,7 @@ public:
 	///
 	/// @brief Apply new width/height values, overwrite Grabber.h implementation as X11 doesn't use width/height, just pixelDecimation to calc dimensions
 	///
-	bool setWidthHeight(int width, int height) override { return true; }
+	bool setWidthHeight(int /*width*/, int /*height*/) override { return true; }
 
 	///
 	/// @brief Apply new pixelDecimation
@@ -109,19 +109,19 @@ private:
 	Picture _srcPicture;
 	Picture _dstPicture;
 
-	int _XRandREventBase;
+	int _xRandREventBase;
 
 	XTransform _transform;
 
-	unsigned _calculatedWidth;
-	unsigned _calculatedHeight;
-	unsigned _src_x;
-	unsigned _src_y;
+	int _screenWidth;
+	int _screenHeight;
+	int _src_x;
+	int _src_y;
 
-	bool _XShmAvailable;
-	bool _XShmPixmapAvailable;
-	bool _XRenderAvailable;
-	bool _XRandRAvailable;
+	bool _xShmAvailable;
+	bool _xShmPixmapAvailable;
+	bool _xRenderAvailable;
+	bool _xRandRAvailable;
 	bool _isWayland;
 
 	Logger * _logger;

include/hyperion/BGEffectHandler.h

@@ -43,6 +43,16 @@ public:
 		handleSettingsUpdate(settings::BGEFFECT, _hyperion->getSetting(settings::BGEFFECT));
 	}
 
+	///
+	/// @brief Disconnect from connected signals
+	/// Disconnect should be done before other priorities invoke methods during shutdown
+	///
+	void disconnect()
+	{
+		QObject::disconnect(_prioMuxer, &PriorityMuxer::prioritiesChanged, nullptr, nullptr);
+		QObject::disconnect(_hyperion, nullptr, nullptr, nullptr);
+	}
+
 	///
 	/// @brief Check, if background effect processing is enabled.
 	/// @return True, background effect processing is enabled.

include/hyperion/ColorAdjustment.h

@@ -16,21 +16,21 @@ public:
 	QString _id;
 
 	/// The BLACK (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbBlackAdjustment;
+	RgbChannelAdjustment _rgbBlackAdjustment {0, 0, 0, "black"};
 	/// The WHITE (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbWhiteAdjustment;
+	RgbChannelAdjustment _rgbWhiteAdjustment{255, 255, 255, "white"};
 	/// The RED (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbRedAdjustment;
+	RgbChannelAdjustment _rgbRedAdjustment {255, 0, 0 , "red"};
 	/// The GREEN (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbGreenAdjustment;
+	RgbChannelAdjustment _rgbGreenAdjustment {0, 255, 0, "green"};
 	/// The BLUE (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbBlueAdjustment;
+	RgbChannelAdjustment _rgbBlueAdjustment {0, 0, 255, "blue"};
 	/// The CYAN (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbCyanAdjustment;
+	RgbChannelAdjustment _rgbCyanAdjustment {0, 255, 255, "cyan"};
 	/// The MAGENTA (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbMagentaAdjustment;
+	RgbChannelAdjustment _rgbMagentaAdjustment {255, 0, 255, "magenta"};
 	/// The YELLOW (RGB-Channel) adjustment
-	RgbChannelAdjustment _rgbYellowAdjustment;
+	RgbChannelAdjustment _rgbYellowAdjustment {255, 255, 0, "yellow"};
 
 	RgbTransform _rgbTransform;
 	OkhsvTransform _okhsvTransform;

include/hyperion/GrabberWrapper.h

@@ -6,6 +6,7 @@
 #include <QString>
 #include <QStringList>
 #include <QMultiMap>
+#include <QScopedPointer>
 
 #include <utils/Logger.h>
 #include <utils/Components.h>
@@ -91,8 +92,8 @@ public:
 	template <typename Grabber_T>
 	bool transferFrame(Grabber_T &grabber)
 	{
-		unsigned w = grabber.getImageWidth();
-		unsigned h = grabber.getImageHeight();
+		int w = grabber.getImageWidth();
+		int h = grabber.getImageHeight();
 		if ( _image.width() != w || _image.height() != h)
 		{
 			_image.resize(w, h);
@@ -185,7 +186,7 @@ protected:
 	Logger * _log;
 
 	/// The timer for generating events with the specified update rate
-	QTimer* _timer;
+	QScopedPointer<QTimer> _timer;
 
 	/// The calculated update rate [ms]
 	int _updateInterval_ms;

include/mdns/MdnsBrowser.h

@@ -2,7 +2,6 @@
 #define MDNS_BROWSER_H
 
 #include <chrono>
-#include <type_traits>
 
 #include <qmdnsengine/server.h>
 #include <qmdnsengine/service.h>
@@ -16,6 +15,9 @@
 // Qt includes
 #include <QObject>
 #include <QByteArray>
+#include <QMap>
+#include <QJsonArray>
+#include <QSharedPointer>
 
 // Utility includes
 #include <utils/Logger.h>
@@ -24,15 +26,12 @@
 namespace {
 	constexpr std::chrono::milliseconds DEFAULT_DISCOVER_TIMEOUT{ 500 };
 	constexpr std::chrono::milliseconds DEFAULT_ADDRESS_RESOLVE_TIMEOUT{ 1000 };
-
-} //End of constants
+} // End of constants
 
 class MdnsBrowser : public QObject
 {
 	Q_OBJECT
 
-		// Run MdnsBrowser as singleton
-
 private:
 	///
 	/// @brief Browse for hyperion services in bonjour, constructed from HyperionDaemon
@@ -58,7 +57,6 @@ public:
 	QMdnsEngine::Service getFirstService(const QByteArray& serviceType, const QString& filter = ".*", const std::chrono::milliseconds waitTime = DEFAULT_DISCOVER_TIMEOUT) const;
 	QJsonArray getServicesDiscoveredJson(const QByteArray& serviceType, const QString& filter = ".*", const std::chrono::milliseconds waitTime = std::chrono::milliseconds{ 0 }) const;
 
-
 	void printCache(const QByteArray& name = QByteArray(), quint16 type = QMdnsEngine::ANY) const;
 
 public slots:
@@ -109,8 +107,9 @@ private:
 
 	QMdnsEngine::Server _server;
 	QMdnsEngine::Cache  _cache;
+	QSharedPointer<QMdnsEngine::Resolver> _resolver;
 
-	QMap<QByteArray, QMdnsEngine::Browser*> _browsedServiceTypes;
+	QMap<QByteArray, QSharedPointer<QMdnsEngine::Browser>> _browsedServiceTypes;
 };
 
 #endif // MDNSBROWSER_H

include/mdns/MdnsProvider.h

@@ -9,6 +9,7 @@
 // Qt includes
 #include <QObject>
 #include <QByteArray>
+#include <QScopedPointer>
 
 // Utility includes
 #include <utils/Logger.h>
@@ -41,11 +42,11 @@ private:
 	/// The logger instance for mDNS-Service
 	Logger* _log;
 
-	QMdnsEngine::Server* _server;
-	QMdnsEngine::Hostname* _hostname;
+	QScopedPointer<QMdnsEngine::Server> _server;
+	QScopedPointer<QMdnsEngine::Hostname> _hostname;
 
 	/// map of services provided
-	QMap<QByteArray, QMdnsEngine::Provider*> _providedServiceTypes;
+	QMap<QByteArray, QSharedPointer<QMdnsEngine::Provider>> _providedServiceTypes;
 };
 
 #endif // MDNSPROVIDER_H

include/utils/Image.h

@@ -15,9 +15,8 @@ public:
 	{
 	}
 
-	Image(unsigned width, unsigned height) :
+	Image(int width, int height) :
 		Image(width, height, Pixel_T())
-
 	{
 	}
 
@@ -28,7 +27,7 @@ public:
 	/// @param height The height of the image
 	/// @param background The color of the image
 	///
-	Image(unsigned width, unsigned height, const Pixel_T background) :
+	Image(int width, int height, const Pixel_T background) :
 		_d_ptr(new ImageData<Pixel_T>(width, height, background))
 	{
 	}
@@ -78,7 +77,7 @@ public:
 	///
 	/// @return The width of the image
 	///
-	inline unsigned width() const
+	inline int width() const
 	{
 		return _d_ptr->width();
 	}
@@ -88,7 +87,7 @@ public:
 	///
 	/// @return The height of the image
 	///
-	inline unsigned height() const
+	inline int height() const
 	{
 		return _d_ptr->height();
 	}
@@ -111,7 +110,7 @@ public:
 	///
 	/// @return const reference to specified pixel
 	///
-	uint8_t blue(unsigned pixel) const
+	uint8_t blue(int pixel) const
 	{
 		return _d_ptr->blue(pixel);
 	}
@@ -121,7 +120,7 @@ public:
 	///
 	/// @param x The x index
 	/// @param y The y index
-	const Pixel_T& operator()(unsigned x, unsigned y) const
+	const Pixel_T& operator()(int x, int y) const
 	{
 		return _d_ptr->operator()(x, y);
 	}
@@ -129,7 +128,7 @@ public:
 	///
 	/// @return reference to specified pixel
 	///
-	Pixel_T& operator()(unsigned x, unsigned y)
+	Pixel_T& operator()(int x, int y)
 	{
 		return _d_ptr->operator()(x, y);
 	}
@@ -137,7 +136,7 @@ public:
 	/// Resize the image
 	/// @param width The width of the image
 	/// @param height The height of the image
-	void resize(unsigned width, unsigned height)
+	void resize(int width, int height)
 	{
 		_d_ptr->resize(width, height);
 	}
@@ -198,12 +197,11 @@ private:
 	///
 	/// @return The index into the underlying data-vector
 	///
-	inline unsigned toIndex(unsigned x, unsigned y) const
+	inline int toIndex(int x, int y) const
 	{
 		return _d_ptr->toIndex(x, y);
 	}
 
-private:
 	QSharedDataPointer<ImageData<Pixel_T>>  _d_ptr;
 };
 

include/utils/ImageData.h

@@ -1,12 +1,9 @@
 #pragma once
 
 // STL includes
-#include <vector>
 #include <cstdint>
 #include <cstring>
 #include <algorithm>
-#include <cassert>
-#include <type_traits>
 #include <utils/ColorRgb.h>
 
 // QT includes
@@ -24,10 +21,10 @@ class ImageData : public QSharedData
 public:
 	typedef Pixel_T pixel_type;
 
-	ImageData(unsigned width, unsigned height, const Pixel_T background) :
+	ImageData(int width, int height, const Pixel_T background) :
 		_width(width),
 		_height(height),
-		_pixels(new Pixel_T[width * height + 1])
+		_pixels(new Pixel_T[static_cast<size_t>(width) * static_cast<size_t>(height)])
 	{
 		std::fill(_pixels, _pixels + width * height, background);
 	}
@@ -36,9 +33,9 @@ public:
 		QSharedData(other),
 		_width(other._width),
 		_height(other._height),
-		_pixels(new Pixel_T[other._width * other._height + 1])
+		_pixels(new Pixel_T[static_cast<size_t>(other._width) * static_cast<size_t>(other._height)])
 	{
-		memcpy(_pixels, other._pixels, static_cast<ulong>(other._width) * static_cast<ulong>(other._height) * sizeof(Pixel_T));
+		memcpy(_pixels, other._pixels, static_cast<size_t>(other._width) * static_cast<size_t>(other._height) * sizeof(Pixel_T));
 	}
 
 	ImageData& operator=(ImageData rhs)
@@ -74,52 +71,57 @@ public:
 		delete[] _pixels;
 	}
 
-	inline unsigned width() const
+	inline int width() const
 	{
 		return _width;
 	}
 
-	inline unsigned height() const
+	inline int height() const
 	{
 		return _height;
 	}
 
-	uint8_t red(unsigned pixel) const
+	uint8_t red(int pixel) const
 	{
 		return (_pixels + pixel)->red;
 	}
 
-	uint8_t green(unsigned pixel) const
+	uint8_t green(int pixel) const
 	{
 		return (_pixels + pixel)->green;
 	}
 
-	uint8_t blue(unsigned pixel) const
+	uint8_t blue(int pixel) const
 	{
 		return (_pixels + pixel)->blue;
 	}
 
-	const Pixel_T& operator()(unsigned x, unsigned y) const
+	const Pixel_T& operator()(int x, int y) const
 	{
 		return _pixels[toIndex(x,y)];
 	}
 
-	Pixel_T& operator()(unsigned x, unsigned y)
+	Pixel_T& operator()(int x, int y)
 	{
 		return _pixels[toIndex(x,y)];
 	}
 
-	void resize(unsigned width, unsigned height)
+	void resize(int width, int height)
 	{
 		if (width == _width && height == _height)
-			return;
-
-		if ((width * height) > unsigned((_width * _height)))
 		{
-			delete[] _pixels;
-			_pixels = new Pixel_T[width*height + 1];
+			return;
 		}
 
+		// Allocate a new buffer without initializing the content
+		Pixel_T* newPixels = new Pixel_T[static_cast<size_t>(width) * static_cast<size_t>(height)];
+
+		// Release the old buffer without copying data
+		delete[] _pixels;
+
+		// Update the pointer to the new buffer
+		_pixels = newPixels;
+
 		_width = width;
 		_height = height;
 	}
@@ -137,11 +139,13 @@ public:
 	void toRgb(ImageData<ColorRgb>& image) const
 	{
 		if (image.width() != _width || image.height() != _height)
+		{
 			image.resize(_width, _height);
+		}
 
-		const unsigned imageSize = _width * _height;
+		const int imageSize = _width * _height;
 
-		for (unsigned idx = 0; idx < imageSize; idx++)
+		for (int idx = 0; idx < imageSize; idx++)
 		{
 			const Pixel_T & color = _pixels[idx];
 			image.memptr()[idx] = ColorRgb{color.red, color.green, color.blue};
@@ -157,26 +161,22 @@ public:
 	{
 		if (_width != 1 || _height != 1)
 		{
-			_width = 1;
-			_height = 1;
-			delete[] _pixels;
-			_pixels = new Pixel_T[2];
+			resize(1,1);
 		}
-
-		memset(_pixels, 0, static_cast<unsigned long>(_width) * static_cast<unsigned long>(_height) * sizeof(Pixel_T));
+		// Set the single pixel to the default background
+		_pixels[0] = Pixel_T();
 	}
 
 private:
-	inline unsigned toIndex(unsigned x, unsigned y) const
+	inline int toIndex(int x, int y) const
 	{
 		return y * _width + x;
 	}
 
-private:
 	/// The width of the image
-	unsigned _width;
+	int _width;
 	/// The height of the image
-	unsigned _height;
+	int _height;
 	/// The pixels of the image
 	Pixel_T* _pixels;
 };

include/utils/Logger.h

@@ -6,6 +6,7 @@
 #include <QMap>
 #include <QAtomicInteger>
 #include <QList>
+#include <QJsonArray>
 
 #if (QT_VERSION >= QT_VERSION_CHECK(5, 14, 0))
 	#include <QRecursiveMutex>
@@ -14,8 +15,6 @@
 #endif
 
 // stl includes
-#include <stdio.h>
-#include <stdarg.h>
 #ifdef _WIN32
 #include <stdexcept>
 #endif
@@ -59,7 +58,7 @@ public:
 		QString      function;
 		unsigned int line;
 		QString      fileName;
-		uint64_t     utime;
+		qint64       utime;
 		QString      message;
 		LogLevel     level;
 		QString      levelString;
@@ -74,7 +73,7 @@ public:
 	void     setMinLevel(LogLevel level) { _minLevel = static_cast<int>(level); }
 	LogLevel getMinLevel() const { return static_cast<LogLevel>(int(_minLevel)); }
 	QString  getName() const { return _name; }
-	QString  getSubName() const { return _subname; }
+	QString  getSubName() const { return _subName; }
 
 signals:
 	void newLogMessage(Logger::T_LOG_MESSAGE);
@@ -95,7 +94,7 @@ private:
 	static QAtomicInteger<int>   GLOBAL_MIN_LOG_LEVEL;
 
 	const QString                _name;
-	const QString                _subname;
+	const QString                _subName;
 	const bool                   _syslogEnabled;
 	const unsigned               _loggerId;
 
@@ -109,15 +108,15 @@ class LoggerManager : public QObject
 
 public:
 	static LoggerManager* getInstance();
-	const QList<Logger::T_LOG_MESSAGE>* getLogMessageBuffer() const { return &_logMessageBuffer; }
 
 public slots:
 	void handleNewLogMessage(const Logger::T_LOG_MESSAGE&);
+	QJsonArray getLogMessageBuffer(Logger::LogLevel filter=Logger::UNSET) const;
 
 signals:
 	void newLogMessage(const Logger::T_LOG_MESSAGE&);
 
-protected:
+private:
 	LoggerManager();
 
 	QList<Logger::T_LOG_MESSAGE>   _logMessageBuffer;

include/utils/hyperion.h

@@ -96,7 +96,7 @@ namespace hyperion {
 			static_cast<uint8_t>(channelConfig[0].toInt(defaultR)),
 			static_cast<uint8_t>(channelConfig[1].toInt(defaultG)),
 			static_cast<uint8_t>(channelConfig[2].toInt(defaultB)),
-			"ChannelAdjust_" + channelName.toUpper()
+			channelName
 		);
 	}
 

include/webserver/WebServer.h

@@ -92,11 +92,6 @@ private:
 	StaticFileServing*   _staticFileServing;
 	QtHttpServer*        _server;
 	bool                 _inited = false;
-
-	const QString        WEBSERVER_DEFAULT_PATH	    = ":/webconfig";
-	const QString        WEBSERVER_DEFAULT_CRT_PATH = ":/hyperion.crt";
-	const QString        WEBSERVER_DEFAULT_KEY_PATH = ":/hyperion.key";
-	quint16              WEBSERVER_DEFAULT_PORT     = 8090;
 };
 
 #endif // WEBSERVER_H