From a01539b664b0ecad0a99e8f17e50cd58518fa4cb Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Thu, 13 Jun 2019 22:30:21 +0200
Subject: [PATCH] [libcalamares] Fix memory ownership of KPMManager

 - The InternalManager object should have at most one living
   instance at a time.
 - getInternal() hands out shared_ptr<>s to the one living instance,
   or creates a new one.
 - The creation of a new InternalManager shouldn't count as a reference
   to it, and it mustn't be deleted after the shared_ptr<>s have done
   their work.
 - So static shared_ptr<InternalManager> was the wrong choice,
   since that leads to double deletes.
 - While here, be a little more chatty when loading KPMCore.
---
 src/libcalamares/partition/KPMManager.cpp | 33 +++++++++++++++--------
 src/libcalamares/partition/KPMManager.h   |  1 -
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/libcalamares/partition/KPMManager.cpp b/src/libcalamares/partition/KPMManager.cpp
index cb30247b6..061884f10 100644
--- a/src/libcalamares/partition/KPMManager.cpp
+++ b/src/libcalamares/partition/KPMManager.cpp
@@ -20,6 +20,7 @@
 
 #include "utils/Logger.h"
 
+#include <kpmcore/backend/corebackend.h>
 #include <kpmcore/backend/corebackendmanager.h>
 #if defined( WITH_KPMCORE4API )
 #include <kpmcore/util/externalcommand.h>
@@ -41,16 +42,23 @@ public:
 };
 
 static bool s_kpm_loaded = false;
-static bool s_loaded = false;
-static std::shared_ptr< InternalManager > s_backend;
+
+/*
+ * We have one living InternalManager object at a time.
+ * It is managed by shared_ptr<>s help by KPMManager
+ * objects, but since we can create KPMManager objects
+ * independent of each other, all of which share ownership
+ * of the same InternalManager, hang on to one extra reference
+ * to the InternalManager so we can hand it out in getInternal().
+ */
+static std::weak_ptr< InternalManager > s_backend;
 
 InternalManager::InternalManager()
 {
-    Q_ASSERT( !s_loaded );
-    s_loaded = true;
-    s_backend.reset( this );
-
     cDebug() << "KPMCore backend starting ..";
+
+    Q_ASSERT( s_backend.expired() );
+
     if ( !s_kpm_loaded )
     {
         QByteArray backendName = qgetenv( "KPMCORE_BACKEND" );
@@ -61,6 +69,8 @@ InternalManager::InternalManager()
         }
         else
         {
+            auto* backend_p = CoreBackendManager::self()->backend();
+            cDebug() << Logger::SubEntry << "Backend @" << (void *)backend_p << backend_p->id() << backend_p->version();
             s_kpm_loaded = true;
         }
     }
@@ -70,8 +80,6 @@ InternalManager::~InternalManager()
 {
     cDebug() << "Cleaning up KPMCore backend ..";
 
-    s_loaded = false;
-
 #if defined( WITH_KPMCORE4API )
     auto backend_p = CoreBackendManager::self()->backend();
     if ( backend_p )
@@ -84,12 +92,15 @@ InternalManager::~InternalManager()
 std::shared_ptr< InternalManager >
 getInternal()
 {
+    cDebug() << "KPMCore internal" << s_backend.use_count();
 
-    if ( !s_loaded )
+    if ( s_backend.expired() )
     {
-        return std::make_shared< InternalManager >();
+        auto p = std::make_shared< InternalManager >();
+        s_backend = p;
+        return p;
     }
-    return s_backend;
+    return s_backend.lock();
 }
 
 KPMManager::KPMManager()
diff --git a/src/libcalamares/partition/KPMManager.h b/src/libcalamares/partition/KPMManager.h
index 2a23122ec..de8ea787b 100644
--- a/src/libcalamares/partition/KPMManager.h
+++ b/src/libcalamares/partition/KPMManager.h
@@ -52,7 +52,6 @@ public:
     operator bool() const;
 private:
     std::shared_ptr< InternalManager > m_d;
-
 };
 
 }  // namespace Partition