From aac3214d4be2ba31c852619bc8d3666391dcd02a Mon Sep 17 00:00:00 2001
From: Robert Adam <dev@robert-adam.de>
Date: Sun, 5 Apr 2020 09:33:47 +0200
Subject: [PATCH] src/mumble/Overlay*: Don't hook into any process before the
 Overlay is active

Right now the Overlay will start hooking into processes on its
construction. This will cause this to happen even if the overlay is
actually disabled. This is a rather intrusive behavior and can lead to
warnings of the OS (for now macOS only - #3552).

This commit refactors the Overlay class so that it will only start
hooking into processes, if it is actually enabled (that is setActive is
called with true).
---
 src/mumble/Overlay.cpp      | 63 +++++++++++++++++++++++--------------
 src/mumble/Overlay.h        |  8 +++++
 src/mumble/Overlay_macx.mm  |  7 +++--
 src/mumble/Overlay_unix.cpp |  2 +-
 src/mumble/Overlay_win.cpp  |  7 +++--
 5 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/src/mumble/Overlay.cpp b/src/mumble/Overlay.cpp
index 11023d9b9..055f47e03 100644
--- a/src/mumble/Overlay.cpp
+++ b/src/mumble/Overlay.cpp
@@ -185,11 +185,45 @@ QRectF OverlayGroup::boundingRect() const {
 }
 
 Overlay::Overlay() : QObject() {
-	d = NULL;
+	d = nullptr;
 
-	platformInit();
-	forceSettings();
+	m_initialized.store(false);
 
+	qlsServer = nullptr;
+
+	QMetaObject::connectSlotsByName(this);
+}
+
+Overlay::~Overlay() {
+	setActive(false);
+	if (d) {
+		delete d;
+	}
+
+	// Need to be deleted first, since destructor references lingering QLocalSockets
+	foreach(OverlayClient *oc, qlClients)
+	{
+		// As we're the one closing the connection, we do not need to be
+		// notified of disconnects. This is important because on disconnect we
+		// also remove (and 'delete') the overlay client.
+		disconnect(oc->qlsSocket, SIGNAL(disconnected()), this, SLOT(disconnected()));
+		disconnect(oc->qlsSocket, SIGNAL(error(QLocalSocket::LocalSocketError)), this, SLOT(error(QLocalSocket::LocalSocketError)));
+		delete oc;
+	}
+}
+
+void Overlay::setActive(bool act) {
+	if (!m_initialized.load()) {
+		platformInit();
+		forceSettings();
+	
+		m_initialized.store(true);
+	}
+
+	setActiveInternal(act);
+}
+
+void Overlay::createPipe() {
 	qlsServer = new QLocalServer(this);
 	// Allow anyone to access the pipe in order to communicate with the overlay
 	qlsServer->setSocketOptions(QLocalServer::WorldAccessOption);
@@ -224,31 +258,12 @@ Overlay::Overlay() : QObject() {
 		qWarning() << "Overlay: Listening on" << qlsServer->fullServerName();
 		connect(qlsServer, SIGNAL(newConnection()), this, SLOT(newConnection()));
 	}
-
-	QMetaObject::connectSlotsByName(this);
-}
-
-Overlay::~Overlay() {
-	setActive(false);
-	delete d;
-
-	// Need to be deleted first, since destructor references lingering QLocalSockets
-	foreach(OverlayClient *oc, qlClients)
-	{
-		// As we're the one closing the connection, we do not need to be
-		// notified of disconnects. This is important because on disconnect we
-		// also remove (and 'delete') the overlay client.
-		disconnect(oc->qlsSocket, SIGNAL(disconnected()), this, SLOT(disconnected()));
-		disconnect(oc->qlsSocket, SIGNAL(error(QLocalSocket::LocalSocketError)), this, SLOT(error(QLocalSocket::LocalSocketError)));
-		delete oc;
-	}
 }
 
 void Overlay::newConnection() {
-	while (true) {
+	while (qlsServer && qlsServer->hasPendingConnections()) {
 		QLocalSocket *qls = qlsServer->nextPendingConnection();
-		if (! qls)
-			break;
+
 		OverlayClient *oc = new OverlayClient(qls, this);
 		qlClients << oc;
 
diff --git a/src/mumble/Overlay.h b/src/mumble/Overlay.h
index c899a5b55..fb1bfb5a1 100644
--- a/src/mumble/Overlay.h
+++ b/src/mumble/Overlay.h
@@ -15,6 +15,8 @@
 #include "OverlayText.h"
 #include "../../overlay/overlay.h"
 
+#include <atomic>
+
 #include "ui_OverlayEditor.h"
 
 #ifdef Q_OS_MAC
@@ -84,8 +86,14 @@ class Overlay : public QObject {
 
 		QSet<unsigned int> qsQueried;
 		QSet<unsigned int> qsQuery;
+		/// A flag indicating if the platformInit has been called already
+		std::atomic<bool> m_initialized;
 
+		// These 2 functions (among others) are implemented by the system-specific backend
 		void platformInit();
+		void setActiveInternal(bool act);
+
+		void createPipe();
 
 		QMap<QString, QString> qmOverlayHash;
 		QLocalServer *qlsServer;
diff --git a/src/mumble/Overlay_macx.mm b/src/mumble/Overlay_macx.mm
index d12203bbd..a4a51ca16 100644
--- a/src/mumble/Overlay_macx.mm
+++ b/src/mumble/Overlay_macx.mm
@@ -175,8 +175,11 @@ void Overlay::platformInit() {
 	d = new OverlayPrivateMac(this);
 }
 
-void Overlay::setActive(bool act) {
-	static_cast<OverlayPrivateMac *>(d)->setActive(act);
+void Overlay::setActiveInternal(bool act) {
+	if (d) {
+		/// Only act if the private instance has been created already
+		static_cast<OverlayPrivateMac *>(d)->setActive(act);
+	}
 }
 
 bool OverlayConfig::supportsInstallableOverlay() {
diff --git a/src/mumble/Overlay_unix.cpp b/src/mumble/Overlay_unix.cpp
index e16ce833f..a2b51890c 100644
--- a/src/mumble/Overlay_unix.cpp
+++ b/src/mumble/Overlay_unix.cpp
@@ -10,7 +10,7 @@ void Overlay::platformInit() {
 	d = NULL;
 }
 
-void Overlay::setActive(bool) {
+void Overlay::setActiveInternal(bool) {
 }
 
 bool OverlayConfig::supportsInstallableOverlay() {
diff --git a/src/mumble/Overlay_win.cpp b/src/mumble/Overlay_win.cpp
index 044c70303..6da0304ec 100644
--- a/src/mumble/Overlay_win.cpp
+++ b/src/mumble/Overlay_win.cpp
@@ -320,8 +320,11 @@ void Overlay::platformInit() {
 	d = new OverlayPrivateWin(this);
 }
 
-void Overlay::setActive(bool act) {
-	static_cast<OverlayPrivateWin *>(d)->setActive(act);
+void Overlay::setActiveInternal(bool act) {
+	if (d) {
+		// Only act if the private instance has been created already
+		static_cast<OverlayPrivateWin *>(d)->setActive(act);
+	}
 }
 
 bool OverlayConfig::supportsInstallableOverlay() {