From 74f6131c0a67e0b1d0a656ba80f5fd542d12e743 Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Thu, 16 Nov 2017 11:28:18 +0100
Subject: Removed ResourceAccess::Private leak and resolved resulting memory
 corruption.

It looks like the memory corruption (malloc started to crash) was coming QLocalSocket related
signals. According to the docs it's not safe (whatever that means), to
delete a QObject with pending signals, so we use deleteLater to schedule
it's deletion. This resolved the crashes.
---
 common/resourceaccess.cpp | 16 ++++++++++++++--
 common/resourceaccess.h   |  6 +++++-
 suppressions.lsan         |  2 --
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/common/resourceaccess.cpp b/common/resourceaccess.cpp
index dab6335..201c023 100644
--- a/common/resourceaccess.cpp
+++ b/common/resourceaccess.cpp
@@ -82,6 +82,7 @@ class ResourceAccess::Private
 {
 public:
     Private(const QByteArray &name, const QByteArray &instanceIdentifier, ResourceAccess *ra);
+    ~Private();
     KAsync::Job<void> tryToConnect();
     KAsync::Job<void> initializeSocket();
     void abortPendingOperations();
@@ -105,6 +106,9 @@ ResourceAccess::Private::Private(const QByteArray &name, const QByteArray &insta
     : resourceName(name), resourceInstanceIdentifier(instanceIdentifier), messageId(0), openingSocket(false)
 {
 }
+ResourceAccess::Private::~Private()
+{
+}
 
 void ResourceAccess::Private::abortPendingOperations()
 {
@@ -144,7 +148,7 @@ void ResourceAccess::Private::callCallbacks()
 // Connects to server and returns connected socket on success
 KAsync::Job<QSharedPointer<QLocalSocket>> ResourceAccess::connectToServer(const QByteArray &identifier)
 {
-    auto s = QSharedPointer<QLocalSocket>::create();
+    auto s = QSharedPointer<QLocalSocket>{new QLocalSocket, &QObject::deleteLater};
     return KAsync::start<QSharedPointer<QLocalSocket>>([identifier, s](KAsync::Future<QSharedPointer<QLocalSocket>> &future) {
         s->setServerName(identifier);
         auto context = new QObject;
@@ -251,6 +255,7 @@ ResourceAccess::~ResourceAccess()
     if (!d->resultHandler.isEmpty()) {
         SinkWarning() << "Left jobs running while shutting down ResourceAccess: " << d->resultHandler.keys();
     }
+    delete d;
 }
 
 QByteArray ResourceAccess::resourceName() const
@@ -662,6 +667,13 @@ bool ResourceAccess::processMessageBuffer()
     return d->partialMessageBuffer.size() >= headerSize;
 }
 
+
+
+ResourceAccessFactory::ResourceAccessFactory()
+{
+
+}
+
 ResourceAccessFactory &ResourceAccessFactory::instance()
 {
     static ResourceAccessFactory *instance = 0;
@@ -682,7 +694,7 @@ Sink::ResourceAccess::Ptr ResourceAccessFactory::getAccess(const QByteArray &ins
         }
         if (!mCache.contains(instanceIdentifier)) {
             // Create a new instance if necessary
-            auto sharedPointer = Sink::ResourceAccess::Ptr::create(instanceIdentifier, resourceType);
+            auto sharedPointer = Sink::ResourceAccess::Ptr{new Sink::ResourceAccess(instanceIdentifier, resourceType), &QObject::deleteLater};
             QObject::connect(sharedPointer.data(), &Sink::ResourceAccess::ready, sharedPointer.data(), [this, instanceIdentifier](bool ready) {
                 if (!ready) {
                     //We want to remove, but we don't want shared pointer to be destroyed until end of the function as this might trigger further steps.
diff --git a/common/resourceaccess.h b/common/resourceaccess.h
index 7df6a1b..9387e99 100644
--- a/common/resourceaccess.h
+++ b/common/resourceaccess.h
@@ -44,6 +44,7 @@ public:
     typedef QSharedPointer<ResourceAccessInterface> Ptr;
 
     ResourceAccessInterface()
+        : QObject()
     {
     }
     virtual ~ResourceAccessInterface()
@@ -110,7 +111,7 @@ public:
     typedef QSharedPointer<ResourceAccess> Ptr;
 
     ResourceAccess(const QByteArray &resourceInstanceIdentifier, const QByteArray &resourceType);
-    ~ResourceAccess();
+    virtual ~ResourceAccess();
 
     QByteArray resourceName() const;
     bool isReady() const;
@@ -167,6 +168,9 @@ public:
     static ResourceAccessFactory &instance();
     Sink::ResourceAccess::Ptr getAccess(const QByteArray &instanceIdentifier, const QByteArray resourceType);
 
+private:
+    ResourceAccessFactory();
+
     QHash<QByteArray, QWeakPointer<Sink::ResourceAccess>> mWeakCache;
     QHash<QByteArray, Sink::ResourceAccess::Ptr> mCache;
     QHash<QByteArray, QSharedPointer<QTimer>> mTimer;
diff --git a/suppressions.lsan b/suppressions.lsan
index cb7c857..9737bd4 100644
--- a/suppressions.lsan
+++ b/suppressions.lsan
@@ -2,8 +2,6 @@ leak:mdb_env_open
 leak:mdb_dbi_open
 #Catch everything from lmdb for now
 leak:liblmdb.so
-#We're currently leaking ResourceAccess::Private
-leak:ResourceAccess
 #There seems to be a tiny leak in qrand that we can't do anything about
 leak:qrand
 leak:ApplicationDomain::getTypeName
-- 
cgit v1.2.3