Fix crash at exit by allocating QObjects on the heap

[joanbm]

In multiple commits between 1.6.6 and 1.6.7, some objects that derive from QObject were added which do not have their own heap allocation, but rather are global variables or live inside another object. However, because ultimately Qt owns those objects and is responsible for releasing them, they must have their own allocation. Otherwise, Qt will attempt to call free on an invalid pointer, which will most likely cause a crash on exit.

This commit should not introduce a leak since the objects are released by Qt. Additionally, AFAICT the call to std::atexit() can be safely removed, since the settings are already saved at VodManager's destructor, which happens at application exit.

Fixes: #302

CC @mrgreywater

[ahjolinna]

seems to work & builds just fine https://build.opensuse.org/package/show/home:ahjolinna/orion

[mrgreywater]

Thank you for contributing. I'll look into this PR/issue when I have some time. Just my two cents after glancing over it:

• The way you're creating the singleton now looses thread-safety.
• Please verify the destructor of the vodmanager is called properly by the qt before removing the std::atexit callback on assumption.

[joanbm]

* The way you're creating the singleton now looses thread-safety.

I've gone ahead and updated my PR to make both the singleton initializers for VodManager and SettingsManager thread safe (and still lazy), by initializing them using a static local variables. I've also updated the rest of the singletons (except NetworkManager which requires a more complex implementation) to follow this pattern in a new commit, as to have both uniform and thread-safe singletons everywhere.

Note that also thread-safety wasn't currently an issue for VodManager and SettingsManager anyway since were they were created early on src/main.cpp:main before creating any more threads ourselves. I assume it wasn't also an issue for the other ones, since they were not thread-safe before, but making them thread-safe now can't make things worse.

* Please verify the destructor of the vodmanager is called properly by the qt before removing the std::atexit callback on assumption.

The VodManager destructor is called at exit time since its parent QObject is set to the QApplication (see the call to src/main.cpp:registerQmlComponents from src/main.cpp:main), so it is destroyed (using the QObject ownership mechanism) at the end of main, when the application is closed. The destructors for the other objects are also called as well when their parent is destroyed by the same mechanism.

I have also done a primitive test to verify this works in practice (i.e. all destructors are called at the right time and don't leak) by printing constructor/destructor events and checking that they all match up and are called at the right time:

diff --git a/src/model/imageprovider.cpp b/src/model/imageprovider.cpp
index 3810dbb..3f847f4 100644
--- a/src/model/imageprovider.cpp
+++ b/src/model/imageprovider.cpp
@@ -217,6 +217,7 @@ void DownloadHandler::replyFinished() {

 // CachedImageProvider
 CachedImageProvider::CachedImageProvider(ImageProvider const* provider) : QQuickImageProvider(QQuickImageProvider::Image), _provider(provider) {
+    printf("CREATE CACHEDIMAGEPROVIDER\n");

 }

diff --git a/src/model/imageprovider.h b/src/model/imageprovider.h
index fab4271..d5472e2 100644
--- a/src/model/imageprovider.h
+++ b/src/model/imageprovider.h
@@ -52,6 +52,7 @@ class ImageProvider;
 class CachedImageProvider : public QQuickImageProvider {
 public:
     CachedImageProvider(ImageProvider const* provider);
+    ~CachedImageProvider() { printf("DESTROY CACHEDIMAGEPROVIDER\n"); }
     QImage requestImage(const QString &id, QSize * size, const QSize & requestedSize);
 private:
     ImageProvider const* _provider;
diff --git a/src/model/settingsmanager.cpp b/src/model/settingsmanager.cpp
index 60d8705..bdc5005 100644
--- a/src/model/settingsmanager.cpp
+++ b/src/model/settingsmanager.cpp
@@ -5,6 +5,7 @@
 SettingsManager::SettingsManager(QObject *parent) :
     QObject(parent), settings(QCoreApplication::organizationName(), QCoreApplication::applicationName(), this)
 {
+    printf("CREATE SETTINGSMANAGER\n");
     load();
     //Connections
     connect(HttpServer::getInstance(), &HttpServer::codeReceived, this, &SettingsManager::setAccessToken);
diff --git a/src/model/settingsmanager.h b/src/model/settingsmanager.h
index d756add..b564380 100644
--- a/src/model/settingsmanager.h
+++ b/src/model/settingsmanager.h
@@ -81,6 +81,7 @@ class SettingsManager : public QObject
     bool mKeepOnTop = false;

     explicit SettingsManager(QObject *parent = nullptr);
+    ~SettingsManager() { printf("DESTROY SETTINGSMANAGER\n"); }
 public:
     static SettingsManager *getInstance();

diff --git a/src/model/vodmanager.cpp b/src/model/vodmanager.cpp
index af6913e..876e23a 100644
--- a/src/model/vodmanager.cpp
+++ b/src/model/vodmanager.cpp
@@ -21,6 +21,7 @@ VodManager::VodManager(QObject *parent) :
     QObject(parent),
     netman(NetworkManager::getInstance())
 {
+    printf("CREATE VODMANAGER\n");
     qmlRegisterInterface<VodListModel>("VodListModel");
     _model = new VodListModel(this);

@@ -50,6 +51,7 @@ VodManager *VodManager::getInstance() {

 VodManager::~VodManager()
 {
+    printf("DESTROY VODMANAGER\n");
     saveSettings();
     delete _model;
 }