From 9b5a391c868c3d96c8d1e4552561e64bd52e98ed Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 9 Mar 2020 10:05:01 -0500
Subject: [PATCH] [libcalamares] Factor out Python helper

- the strange construction of Helper and treating it as a singleton
  can be factored out into a separate singleton-handling instance()
  function. The Helper should never be destroyed.
---
 src/libcalamares/PythonHelper.cpp | 66 +++++++++++++++----------------
 src/libcalamares/PythonHelper.h   |  9 ++---
 src/libcalamares/PythonJob.cpp    | 17 +-------
 src/libcalamares/PythonJob.h      |  1 -
 4 files changed, 37 insertions(+), 56 deletions(-)

diff --git a/src/libcalamares/PythonHelper.cpp b/src/libcalamares/PythonHelper.cpp
index d9db8581e..0b5d77ac1 100644
--- a/src/libcalamares/PythonHelper.cpp
+++ b/src/libcalamares/PythonHelper.cpp
@@ -204,8 +204,6 @@ variantHashFromPyDict( const boost::python::dict& pyDict )
 }
 
 
-Helper* Helper::s_instance = nullptr;
-
 static inline void
 add_if_lib_exists( const QDir& dir, const char* name, QStringList& list )
 {
@@ -221,48 +219,46 @@ add_if_lib_exists( const QDir& dir, const char* name, QStringList& list )
     }
 }
 
-Helper::Helper( QObject* parent )
-    : QObject( parent )
+Helper::Helper()
+    : QObject( nullptr )
 {
     // Let's make extra sure we only call Py_Initialize once
-    if ( !s_instance )
+    if ( !Py_IsInitialized() )
     {
-        if ( !Py_IsInitialized() )
-        {
-            Py_Initialize();
-        }
-
-        m_mainModule = bp::import( "__main__" );
-        m_mainNamespace = m_mainModule.attr( "__dict__" );
-
-        // If we're running from the build dir
-        add_if_lib_exists( QDir::current(), "libcalamares.so", m_pythonPaths );
-
-        QDir calaPythonPath( CalamaresUtils::systemLibDir().absolutePath() + QDir::separator() + "calamares" );
-        add_if_lib_exists( calaPythonPath, "libcalamares.so", m_pythonPaths );
-
-        bp::object sys = bp::import( "sys" );
-
-        foreach ( QString path, m_pythonPaths )
-        {
-            bp::str dir = path.toLocal8Bit().data();
-            sys.attr( "path" ).attr( "append" )( dir );
-        }
-    }
-    else
-    {
-        cWarning() << "creating PythonHelper more than once. This is very bad.";
-        return;
+        Py_Initialize();
     }
 
-    s_instance = this;
+    m_mainModule = bp::import( "__main__" );
+    m_mainNamespace = m_mainModule.attr( "__dict__" );
+
+    // If we're running from the build dir
+    add_if_lib_exists( QDir::current(), "libcalamares.so", m_pythonPaths );
+
+    QDir calaPythonPath( CalamaresUtils::systemLibDir().absolutePath() + QDir::separator() + "calamares" );
+    add_if_lib_exists( calaPythonPath, "libcalamares.so", m_pythonPaths );
+
+    bp::object sys = bp::import( "sys" );
+
+    foreach ( QString path, m_pythonPaths )
+    {
+        bp::str dir = path.toLocal8Bit().data();
+        sys.attr( "path" ).attr( "append" )( dir );
+    }
 }
 
-Helper::~Helper()
+Helper::~Helper() {}
+
+Helper*
+Helper::instance()
 {
-    s_instance = nullptr;
-}
+    static Helper* s_helper = nullptr;
 
+    if ( !s_helper )
+    {
+        s_helper = new Helper;
+    }
+    return s_helper;
+}
 
 boost::python::dict
 Helper::createCleanNamespace()
diff --git a/src/libcalamares/PythonHelper.h b/src/libcalamares/PythonHelper.h
index 418c75e5f..7528732a0 100644
--- a/src/libcalamares/PythonHelper.h
+++ b/src/libcalamares/PythonHelper.h
@@ -50,16 +50,15 @@ class Helper : public QObject
 {
     Q_OBJECT
 public:
-    virtual ~Helper();
-
     boost::python::dict createCleanNamespace();
 
     QString handleLastError();
 
+    static Helper* instance();
+
 private:
-    friend Helper* Calamares::PythonJob::helper();
-    explicit Helper( QObject* parent = nullptr );
-    static Helper* s_instance;
+    virtual ~Helper();
+    explicit Helper();
 
     boost::python::object m_mainModule;
     boost::python::object m_mainNamespace;
diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp
index d94a20981..11a963df0 100644
--- a/src/libcalamares/PythonJob.cpp
+++ b/src/libcalamares/PythonJob.cpp
@@ -233,7 +233,7 @@ PythonJob::exec()
 
     try
     {
-        bp::dict scriptNamespace = helper()->createCleanNamespace();
+        bp::dict scriptNamespace = CalamaresPython::Helper::instance()->createCleanNamespace();
 
         bp::object calamaresModule = bp::import( "libcalamares" );
         bp::dict calamaresNamespace = bp::extract< bp::dict >( calamaresModule.attr( "__dict__" ) );
@@ -299,7 +299,7 @@ PythonJob::exec()
         QString msg;
         if ( PyErr_Occurred() )
         {
-            msg = helper()->handleLastError();
+            msg = CalamaresPython::Helper::instance()->handleLastError();
         }
         bp::handle_exception();
         PyErr_Clear();
@@ -315,17 +315,4 @@ PythonJob::emitProgress( qreal progressValue )
     emit progress( progressValue );
 }
 
-
-CalamaresPython::Helper*
-PythonJob::helper()
-{
-    auto ptr = CalamaresPython::Helper::s_instance;
-    if ( !ptr )
-    {
-        ptr = new CalamaresPython::Helper;
-    }
-    return ptr;
-}
-
-
 }  // namespace Calamares
diff --git a/src/libcalamares/PythonJob.h b/src/libcalamares/PythonJob.h
index 7cd1b7165..ba2b0a75f 100644
--- a/src/libcalamares/PythonJob.h
+++ b/src/libcalamares/PythonJob.h
@@ -57,7 +57,6 @@ private:
     friend class CalamaresPython::PythonJobInterface;
     void emitProgress( double progressValue );
 
-    CalamaresPython::Helper* helper();
     QString m_scriptFile;
     QString m_workingPath;
     QString m_description;