Skip to content

Commit

Permalink
Fixes #10926. Safely teardown MythPainters.
Browse files Browse the repository at this point in the history
The deadlock was actually the least of our problems.
Since MythPainter::DeleteFormatImagePriv() is a pure
virtual in MythPainter calling ExpireImages() in
the dtor is unsafe. This puts the teardown code
into a Teardown() method which is called from all
the child classes where it is safe to do the teardown.
  • Loading branch information
daniel-kristjansson committed Jul 21, 2012
1 parent 84aac9d commit e29f47d
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 31 deletions.
2 changes: 1 addition & 1 deletion mythtv/libs/libmythbase/mythversion.h
Expand Up @@ -12,7 +12,7 @@
/// Update this whenever the plug-in API changes.
/// Including changes in the libmythbase, libmyth, libmythtv, libmythav* and
/// libmythui class methods used by plug-ins.
#define MYTH_BINARY_VERSION "0.26.20120719-1"
#define MYTH_BINARY_VERSION "0.26.20120721-1"

/** \brief Increment this whenever the MythTV network protocol changes.
*
Expand Down
40 changes: 19 additions & 21 deletions mythtv/libs/libmythui/mythpainter.cpp
Expand Up @@ -21,20 +21,23 @@ MythPainter::MythPainter()
SetMaximumCacheSizes(96, 96);
}

MythPainter::~MythPainter(void)
void MythPainter::Teardown(void)
{
QMutexLocker locker(&m_allocationLock);

ExpireImages(0);

if (m_allocatedImages.isEmpty())
return;
QMutexLocker locker(&m_allocationLock);

LOG(VB_GENERAL, LOG_WARNING,
QString("MythPainter: %1 images not yet de-allocated.")
if (!m_allocatedImages.isEmpty())
{
LOG(VB_GENERAL, LOG_WARNING,
QString("MythPainter: %1 images not yet de-allocated.")
.arg(m_allocatedImages.size()));
while (!m_allocatedImages.isEmpty())
m_allocatedImages.takeLast()->SetParent(NULL);
}

QSet<MythImage*>::iterator it = m_allocatedImages.begin();
for (; it != m_allocatedImages.end(); ++it)
(*it)->SetParent(NULL);
m_allocatedImages.clear();
}

void MythPainter::SetClipRect(const QRect &)
Expand Down Expand Up @@ -506,34 +509,29 @@ MythImage* MythPainter::GetImageFromRect(const QRect &area, int radius,
return im;
}

MythImage *MythPainter::GetFormatImage()
MythImage *MythPainter::GetFormatImage(void)
{
m_allocationLock.lock();
QMutexLocker locker(&m_allocationLock);
MythImage *result = GetFormatImagePriv();
result->SetFileName("GetFormatImage");
m_allocatedImages.append(result);
m_allocationLock.unlock();
m_allocatedImages.insert(result);
return result;
}

void MythPainter::DeleteFormatImage(MythImage *im)
{
m_allocationLock.lock();
QMutexLocker locker(&m_allocationLock);
DeleteFormatImagePriv(im);

while (m_allocatedImages.contains(im))
m_allocatedImages.removeOne(im);
m_allocationLock.unlock();
m_allocatedImages.remove(im);
}

void MythPainter::CheckFormatImage(MythImage *im)
{
if (im && !im->GetParent())
{
m_allocationLock.lock();
m_allocatedImages.append(im);
QMutexLocker locker(&m_allocationLock);
m_allocatedImages.insert(im);
im->SetParent(this);
m_allocationLock.unlock();
}
}

Expand Down
18 changes: 15 additions & 3 deletions mythtv/libs/libmythui/mythpainter.h
Expand Up @@ -7,6 +7,7 @@
#include <QWidget>
#include <QPaintDevice>
#include <QMutex>
#include <QSet>

class QRect;
class QRegion;
Expand All @@ -29,7 +30,14 @@ class MUI_PUBLIC MythPainter
{
public:
MythPainter();
virtual ~MythPainter();
/** MythPainter destructor.
*
* The MythPainter destructor does not cleanup, it is unsafe
* to do cleanup in the MythPainter destructor because
* DeleteImagePriv() is a pure virtual in this class. Instead
* children should call MythPainter::Teardown() for cleanup.
*/
virtual ~MythPainter() {}

virtual QString GetName(void) = 0;
virtual bool SupportsAnimation(void) = 0;
Expand Down Expand Up @@ -108,6 +116,10 @@ class MUI_PUBLIC MythPainter
virtual void DeleteFormatImagePriv(MythImage *im) = 0;
void ExpireImages(int64_t max = 0);

// This needs to be called by classes inheriting from MythPainter
// in the destructor.
virtual void Teardown(void);

void CheckFormatImage(MythImage *im);

QPaintDevice *m_Parent;
Expand All @@ -118,8 +130,8 @@ class MUI_PUBLIC MythPainter
int64_t m_SoftwareCacheSize;
int64_t m_MaxSoftwareCacheSize;

QList<MythImage*> m_allocatedImages;
QMutex m_allocationLock;
QMutex m_allocationLock;
QSet<MythImage*> m_allocatedImages;

QMap<QString, MythImage *> m_StringToImageMap;
std::list<QString> m_StringExpireList;
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythui/mythpainter_d3d9.cpp
Expand Up @@ -46,7 +46,7 @@ bool MythD3D9Painter::InitD3D9(QPaintDevice *parent)

void MythD3D9Painter::Teardown(void)
{
ExpireImages();
MythPainter::Teardown();
ClearCache();
DeleteBitmaps();

Expand Down
3 changes: 2 additions & 1 deletion mythtv/libs/libmythui/mythpainter_d3d9.h
Expand Up @@ -34,8 +34,9 @@ class MUI_PUBLIC MythD3D9Painter : public MythPainter
protected:
virtual MythImage* GetFormatImagePriv(void) { return new MythImage(this); }
virtual void DeleteFormatImagePriv(MythImage *im);
virtual void Teardown(void);

bool InitD3D9(QPaintDevice *parent);
void Teardown(void);
void ClearCache(void);
void DeleteBitmaps(void);
D3D9Image* GetImageFromCache(MythImage *im);
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythui/mythpainter_ogl.cpp
Expand Up @@ -31,7 +31,7 @@ MythOpenGLPainter::MythOpenGLPainter(MythRenderOpenGL *render,

MythOpenGLPainter::~MythOpenGLPainter()
{
ExpireImages(0);
Teardown();
FreeResources();
}

Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythui/mythpainter_qimage.cpp
Expand Up @@ -17,7 +17,7 @@ MythQImagePainter::MythQImagePainter() :

MythQImagePainter::~MythQImagePainter()
{
ExpireImages();
Teardown();
}

void MythQImagePainter::Begin(QPaintDevice *parent)
Expand Down
1 change: 1 addition & 0 deletions mythtv/libs/libmythui/mythpainter_qt.cpp
Expand Up @@ -61,6 +61,7 @@ MythQtPainter::MythQtPainter() :

MythQtPainter::~MythQtPainter()
{
Teardown();
DeletePixmaps();
}

Expand Down
3 changes: 2 additions & 1 deletion mythtv/libs/libmythui/mythpainter_vdpau.cpp
Expand Up @@ -52,7 +52,8 @@ bool MythVDPAUPainter::InitVDPAU(QPaintDevice *parent)

void MythVDPAUPainter::Teardown(void)
{
ExpireImages();
MythPainter::Teardown();

FreeResources();

m_ImageBitmapMap.clear();
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythui/mythpainter_vdpau.h
Expand Up @@ -33,9 +33,9 @@ class MUI_PUBLIC MythVDPAUPainter : public MythPainter
protected:
virtual MythImage* GetFormatImagePriv(void) { return new MythImage(this); }
virtual void DeleteFormatImagePriv(MythImage *im);
virtual void Teardown(void);

bool InitVDPAU(QPaintDevice *parent);
void Teardown(void);
void ClearCache(void);
void DeleteBitmaps(void);
uint GetTextureFromCache(MythImage *im);
Expand Down
1 change: 1 addition & 0 deletions mythtv/libs/libmythui/mythpainter_yuva.cpp
Expand Up @@ -11,6 +11,7 @@ QColor inline rgb_to_yuv(const QColor &original);

MythYUVAPainter::~MythYUVAPainter()
{
Teardown();
foreach(MythFontProperties* font, m_convertedFonts)
delete font;
}
Expand Down

0 comments on commit e29f47d

Please sign in to comment.