path: root/abs/core/mythtv/stable-29/mythtv/0287-MythUiImage-Don-t-block-UI-when-exiting-screens.patch

From 5ee658b482ad634706c21d20d4f8fd15828fc1d8 Mon Sep 17 00:00:00 2001
From: Roger Siddons <dizygotheca@ntlworld.com>
Date: Mon, 12 Oct 2015 14:54:36 +0100
Subject: [PATCH 11/13] MythUiImage: Don't block UI when exiting screens

When deleting MythUIImages the UI blocks until all image loading threads complete.

This patch removes the need for that by introducing an image proxy for the load threads that
prohibits access to a deleted image.

Note the ImageLoadEvent is unchanged but has been moved to resolve dependancies. It still
contains a (potentially dangerous) pointer to the deleted MythUIImage, which IMO is redundant and should be removed.

diff --git a/mythtv/libs/libmythui/mythuiimage.cpp b/mythtv/libs/libmythui/mythuiimage.cpp
index fc067f7..a937358 100644
--- a/mythtv/libs/libmythui/mythuiimage.cpp
+++ src/mythtv/libs/libmythui/mythuiimage.cpp
@@ -121,25 +121,104 @@ void ImageProperties::SetMaskImage(MythImage *image)
 }
 
 /*!
+ * \class ImageLoadEvent
+ */
+class ImageLoadEvent : public QEvent
+{
+  public:
+    ImageLoadEvent(const MythUIImage *parent, MythImage *image,
+                   const QString &basefile, const QString &filename,
+                   int number, bool aborted)
+        : QEvent(kEventType),
+          m_parent(parent), m_image(image), m_basefile(basefile),
+          m_filename(filename), m_number(number),
+          m_images(NULL), m_aborted(aborted) { }
+
+    ImageLoadEvent(const MythUIImage *parent, AnimationFrames *frames,
+                   const QString &basefile,
+                   const QString &filename, bool aborted)
+        : QEvent(kEventType),
+          m_parent(parent), m_image(NULL), m_basefile(basefile),
+          m_filename(filename), m_number(0),
+          m_images(frames), m_aborted(aborted) { }
+
+    const MythUIImage *GetParent() const        { return m_parent; }
+    MythImage *GetImage() const                 { return m_image; }
+    const QString GetBasefile() const           { return m_basefile; }
+    const QString GetFilename() const           { return m_filename; }
+    int GetNumber() const                       { return m_number; }
+    AnimationFrames *GetAnimationFrames() const { return m_images; }
+    bool GetAbortState() const                  { return m_aborted; }
+
+    static Type kEventType;
+
+  private:
+    const MythUIImage *m_parent;
+    MythImage         *m_image;
+    QString            m_basefile;
+    QString            m_filename;
+    int                m_number;
+
+    // Animated Images
+    AnimationFrames  *m_images;
+
+    // Image Load
+    bool             m_aborted;
+};
+
+QEvent::Type ImageLoadEvent::kEventType =
+    (QEvent::Type) QEvent::registerEventType();
+
+/*!
+   \brief Guards access to the image by Load threads. If the image is deleted
+   by the UI this proxy persists until all its load threads complete.
+ */
+class ImageProxy
+{
+public:
+    ImageProxy(MythUIImage *image) : m_mutex(), m_image(image) {}
+
+    MythUIImage *GetImage() { QMutexLocker lock(&m_mutex); return m_image; }
+    void         Orphan()   { QMutexLocker lock(&m_mutex); m_image = NULL; }
+    void         PostEvent(ImageLoadEvent *event)
+    {
+        QMutexLocker lock(&m_mutex);
+        if (m_image)
+            QCoreApplication::postEvent(m_image, event);
+        else
+            LOG(VB_GUI | VB_FILE, LOG_DEBUG,
+                QString("ImageLoadThread: Discarding load of %1")
+                .arg(event->GetFilename()));
+    }
+
+private:
+    QMutex       m_mutex;
+    MythUIImage *m_image;
+};
+
+//! The proxy is shared by the image and all its load threads.
+typedef QSharedPointer<ImageProxy> ProxyPtr;
+
+/*!
  * \class ImageLoader
  */
 class ImageLoader
 {
   public:
-    ImageLoader() { };
-   ~ImageLoader() { };
+    ImageLoader() { }
+   ~ImageLoader() { }
 
-    static QHash<QString, const MythUIImage *> m_loadingImages;
-    static QMutex                        m_loadingImagesLock;
-    static QWaitCondition                m_loadingImagesCond;
+    static QHash<QString, const ImageProxy*> m_loadingImages;
+    static QMutex                            m_loadingImagesLock;
+    static QWaitCondition                    m_loadingImagesCond;
 
-    static bool PreLoad(const QString &cacheKey, const MythUIImage *uitype)
+    static bool PreLoad(const QString &cacheKey, const ProxyPtr &proxy)
     {
         m_loadingImagesLock.lock();
 
         // Check to see if the image is being loaded by us in another thread
         if ((m_loadingImages.contains(cacheKey)) &&
-            (m_loadingImages[cacheKey] == uitype))
+            (m_loadingImages[cacheKey] == proxy.data()))
         {
             LOG(VB_GUI | VB_FILE, LOG_DEBUG,
                 QString("ImageLoader::PreLoad(%1), this "
@@ -153,7 +232,7 @@ class ImageLoader
         while (m_loadingImages.contains(cacheKey))
             m_loadingImagesCond.wait(&m_loadingImagesLock);
 
-        m_loadingImages[cacheKey] = uitype;
+        m_loadingImages[cacheKey] = proxy.data();
         m_loadingImagesLock.unlock();
 
         return true;
@@ -229,15 +308,12 @@ class ImageLoader
                                  // Must be a copy for thread safety
                                 ImageProperties imProps,
                                 ImageCacheMode cacheMode,
-                                 // Included only to check address, could be
-                                 // replaced by generating a unique value for
-                                 // each MythUIImage object?
-                                const MythUIImage *parent,
+                                const ProxyPtr &proxy,
                                 bool &aborted,
                                 MythImageReader *imageReader = NULL)
     {
         QString cacheKey = GenImageLabel(imProps);
-        if (!PreLoad(cacheKey, parent))
+        if (!PreLoad(cacheKey, proxy))
         {
             aborted = true;
             return NULL;
@@ -390,10 +466,7 @@ class ImageLoader
                                                // Must be a copy for thread safety
                                               ImageProperties imProps,
                                               ImageCacheMode cacheMode,
-                                               // Included only to check address, could be
-                                               // replaced by generating a unique value for
-                                               // each MythUIImage object?
-                                              const MythUIImage *parent,
+                                              const ProxyPtr &proxy,
                                               bool &aborted)
     {
         QString filename = QString("frame-%1-") + imProps.filename;
@@ -411,7 +484,7 @@ class ImageLoader
             ImageProperties frameProps = imProps;
             frameProps.filename = frameFilename;
 
-            MythImage *im = LoadImage(painter, frameProps, cacheMode, parent,
+            MythImage *im = LoadImage(painter, frameProps, cacheMode, proxy,
                                       aborted, imageReader);
 
             if (!im)
@@ -428,58 +501,9 @@ class ImageLoader
 
 };
 
-QHash<QString, const MythUIImage *> ImageLoader::m_loadingImages;
-QMutex                              ImageLoader::m_loadingImagesLock;
-QWaitCondition                      ImageLoader::m_loadingImagesCond;
-
-/*!
- * \class ImageLoadEvent
- */
-class ImageLoadEvent : public QEvent
-{
-  public:
-    ImageLoadEvent(const MythUIImage *parent, MythImage *image,
-                   const QString &basefile, const QString &filename,
-                   int number, bool aborted)
-        : QEvent(kEventType),
-          m_parent(parent), m_image(image), m_basefile(basefile),
-          m_filename(filename), m_number(number),
-          m_images(NULL), m_aborted(aborted) { }
-
-    ImageLoadEvent(const MythUIImage *parent, AnimationFrames *frames,
-                   const QString &basefile,
-                   const QString &filename, bool aborted)
-        : QEvent(kEventType),
-          m_parent(parent), m_image(NULL), m_basefile(basefile),
-          m_filename(filename), m_number(0),
-          m_images(frames), m_aborted(aborted) { }
-
-    const MythUIImage *GetParent() const    { return m_parent; }
-    MythImage *GetImage() const       { return m_image; }
-    const QString GetBasefile() const { return m_basefile; }
-    const QString GetFilename() const { return m_filename; }
-    int GetNumber() const             { return m_number; }
-    AnimationFrames *GetAnimationFrames() const { return m_images; }
-    bool GetAbortState() const        { return m_aborted; }
-
-    static Type kEventType;
-
-  private:
-    const MythUIImage     *m_parent;
-    MythImage       *m_image;
-    QString          m_basefile;
-    QString          m_filename;
-    int              m_number;
-
-    // Animated Images
-    AnimationFrames  *m_images;
-
-    // Image Load
-    bool             m_aborted;
-};
-
-QEvent::Type ImageLoadEvent::kEventType =
-    (QEvent::Type) QEvent::registerEventType();
+QHash<QString, const ImageProxy*> ImageLoader::m_loadingImages;
+QMutex                            ImageLoader::m_loadingImagesLock;
+QWaitCondition                    ImageLoader::m_loadingImagesCond;
 
 /*!
 * \class ImageLoadThread
@@ -487,10 +511,10 @@ QEvent::Type ImageLoadEvent::kEventType =
 class ImageLoadThread : public QRunnable
 {
   public:
-    ImageLoadThread(const MythUIImage *parent, MythPainter *painter,
+    ImageLoadThread(const ProxyPtr &proxy, MythPainter *painter,
                     const ImageProperties &imProps, const QString &basefile,
                     int number, ImageCacheMode mode) :
-        m_parent(parent), m_painter(painter), m_imageProperties(imProps),
+        m_proxy(proxy), m_painter(painter), m_imageProperties(imProps),
         m_basefile(basefile), m_number(number), m_cacheMode(mode)
     {
     }
@@ -500,6 +524,14 @@ class ImageLoadThread : public QRunnable
         bool aborted = false;
         QString filename =  m_imageProperties.filename;
 
+        // Don't even bother if parent image no longer exists
+        if (!m_proxy || !m_proxy->GetImage())
+        {
+            LOG(VB_GUI | VB_FILE, LOG_DEBUG,
+                QString("ImageLoadThread: Ignoring load of %1").arg(filename));
+            return;
+        }
+
         // NOTE Do NOT use MythImageReader::supportsAnimation here, it defeats
         // the point of caching remote images
         if (ImageLoader::SupportsAnimation(filename))
@@ -508,17 +540,16 @@ class ImageLoadThread : public QRunnable
 
              frames = ImageLoader::LoadAnimatedImage(m_painter,
                                                      m_imageProperties,
-                                                     m_cacheMode, m_parent,
+                                                     m_cacheMode, m_proxy,
                                                      aborted);
 
              if (frames && frames->count() > 1)
              {
-                ImageLoadEvent *le = new ImageLoadEvent(m_parent, frames,
+                ImageLoadEvent *le = new ImageLoadEvent(m_proxy->GetImage(), frames,
                                                         m_basefile,
                                                         m_imageProperties.filename,
                                                         aborted);
-                QCoreApplication::postEvent(const_cast<MythUIImage*>(m_parent), le);
-
+                m_proxy->PostEvent(le);
                 return;
              }
              delete frames;
@@ -526,18 +557,19 @@ class ImageLoadThread : public QRunnable
 
         MythImage *image = ImageLoader::LoadImage(m_painter,
                                                     m_imageProperties,
-                                                    m_cacheMode, m_parent,
+                                                    m_cacheMode, m_proxy,
                                                     aborted);
 
-        ImageLoadEvent *le = new ImageLoadEvent(m_parent, image, m_basefile,
+        ImageLoadEvent *le = new ImageLoadEvent(m_proxy->GetImage(), image,
+                                                m_basefile,
                                                 m_imageProperties.filename,
                                                 m_number, aborted);
-        QCoreApplication::postEvent(const_cast<MythUIImage*>(m_parent), le);
+        m_proxy->PostEvent(le);
     }
 
 private:
-    const MythUIImage    *m_parent;
-    MythPainter       *m_painter;
+    const ProxyPtr  m_proxy;
+    MythPainter    *m_painter;
     ImageProperties m_imageProperties;
     QString         m_basefile;
     int             m_number;
@@ -549,13 +581,14 @@ class MythUIImagePrivate
 {
 public:
     explicit MythUIImagePrivate(MythUIImage *p)
-        : m_parent(p),            m_UpdateLock(QReadWriteLock::Recursive)
-    { };
-    ~MythUIImagePrivate() {};
-
-    MythUIImage *m_parent;
+        : m_parent(p), m_UpdateLock(QReadWriteLock::Recursive),
+          m_proxy(ProxyPtr(new ImageProxy(p)))
+    { }
+    ~MythUIImagePrivate() {}
 
+    MythUIImage   *m_parent;
     QReadWriteLock m_UpdateLock;
+    ProxyPtr       m_proxy;
 };
 
 /////////////////////////////////////////////////////////////////
@@ -609,13 +642,8 @@ MythUIImage::MythUIImage(MythUIType *parent, const QString &name)
 
 MythUIImage::~MythUIImage()
 {
-    // Wait until all image loading threads are complete or bad things
-    // may happen if this MythUIImage disappears when a queued thread
-    // needs it.
-    if (m_runningThreads > 0)
-    {
-        GetMythUI()->GetImageThreadPool()->waitForDone();
-    }
+    // Prevent any updates from running loads
+    d->m_proxy->Orphan();
 
     Clear();
 
@@ -695,8 +723,6 @@ void MythUIImage::Init(void)
     m_animationReverse = false;
     m_animatedImage = false;
 
-    m_runningThreads = 0;
-
     m_showingRandomImage = false;
 }
 
@@ -1081,9 +1107,8 @@ bool MythUIImage::Load(bool allowLoadInBackground, bool forceStat)
             LOG(VB_GUI | VB_FILE, LOG_DEBUG, LOC +
                 QString("Load(), spawning thread to load '%1'").arg(filename));
 
-            m_runningThreads++;
             ImageLoadThread *bImgThread;
-            bImgThread = new ImageLoadThread(this, GetPainter(),
+            bImgThread = new ImageLoadThread(d->m_proxy, GetPainter(),
                                              imProps,
                                              bFilename, i,
                                              static_cast<ImageCacheMode>(cacheMode2));
@@ -1102,7 +1127,7 @@ bool MythUIImage::Load(bool allowLoadInBackground, bool forceStat)
 
                 myFrames = ImageLoader::LoadAnimatedImage(GetPainter(), imProps,
                                         static_cast<ImageCacheMode>(cacheMode2),
-                                        this, aborted);
+                                        d->m_proxy, aborted);
 
                 // TODO We might want to handle an abort here more gracefully
                 if (aborted)
@@ -1121,7 +1146,7 @@ bool MythUIImage::Load(bool allowLoadInBackground, bool forceStat)
                 image = ImageLoader::LoadImage(GetPainter(),
                                                imProps,
                                                static_cast<ImageCacheMode>(cacheMode2),
-                                               this, aborted);
+                                               d->m_proxy, aborted);
 
                 // TODO We might want to handle an abort here more gracefully
                 if (aborted)
@@ -1604,8 +1629,6 @@ void MythUIImage::customEvent(QEvent *event)
         animationFrames = le->GetAnimationFrames();
         aborted         = le->GetAbortState();
 
-        m_runningThreads--;
-
         d->m_UpdateLock.lockForRead();
         QString propFilename = m_imageProperties.filename;
         d->m_UpdateLock.unlock();
diff --git a/mythtv/libs/libmythui/mythuiimage.h b/mythtv/libs/libmythui/mythuiimage.h
index 7847c4a..e5390a1 100644
--- a/mythtv/libs/libmythui/mythuiimage.h
+++ src/mythtv/libs/libmythui/mythuiimage.h
@@ -174,8 +174,6 @@ class MUI_PUBLIC MythUIImage : public MythUIType
 
     ImageProperties m_imageProperties;
 
-    int m_runningThreads;
-
     bool m_showingRandomImage;
     QString m_imageDirectory;
 
-- 
2.1.4