Redesign of the ArtworkModel in order to improve the handling of the implicitly shared data.

src/Model_Artwork.cpp

@@ -24,121 +24,152 @@
 #include "Global.h"
 
 #include <QFile>
+#include <QMutex>
 #include <QMutexLocker>
 
+////////////////////////////////////////////////////////////
+// Shared data class
 ////////////////////////////////////////////////////////////
 
-QMutex ArtworkModel::m_mutex;
+class ArtworkModel_SharedData
-QMap<QString, unsigned int> ArtworkModel::m_refCount;
+{
-QMap<QString, QFile*> ArtworkModel::m_fileHandle;
+	friend ArtworkModel;
+
+protected:
+	ArtworkModel_SharedData(const QString &filePath, const bool isOwner)
+	:
+		m_isOwner(isOwner),
+		m_filePath(filePath),
+		m_fileHandle(NULL)
+	{
+		m_referenceCounter = 1;
+
+		if(!m_filePath.isEmpty())
+		{
+			QFile *file = new QFile(m_filePath);
+			if(file->open(QIODevice::ReadOnly))
+			{
+				m_fileHandle = file;
+				return;
+			}
+			LAMEXP_DELETE(file);
+		}
+	}
+
+	~ArtworkModel_SharedData(void)
+	{
+		if(m_fileHandle)
+		{
+			if(m_isOwner)
+			{
+				m_fileHandle->remove();
+			}
+			m_fileHandle->close();
+			LAMEXP_DELETE(m_fileHandle);
+		}
+	}
+
+	static ArtworkModel_SharedData *attach(ArtworkModel_SharedData *ptr)
+	{
+		if(ptr)
+		{
+			QMutexLocker lock(&s_mutex);
+			ptr->m_referenceCounter = ptr->m_referenceCounter + 1;
+			return ptr;
+		}
+		return NULL;
+	}
+
+	static void detach(ArtworkModel_SharedData **ptr)
+	{
+		if(*ptr)
+		{
+			QMutexLocker lock(&s_mutex);
+			(*ptr)->m_referenceCounter = (*ptr)->m_referenceCounter - 1;
+			if((*ptr)->m_referenceCounter < 1)
+			{
+				delete (*ptr);
+			}
+			*ptr = NULL;
+		}
+	}
+
+	const QString m_filePath;
+	const bool m_isOwner;
+	QFile *m_fileHandle;
+	unsigned int m_referenceCounter;
+
+	static QMutex s_mutex;
+};
+
+QMutex ArtworkModel_SharedData::s_mutex;
 
 ////////////////////////////////////////////////////////////
 // Constructor & Destructor
 ////////////////////////////////////////////////////////////
 
 ArtworkModel::ArtworkModel(void)
+:
+	m_mutex(new QMutex)
 {
-	m_isOwner = false;
+	m_data = NULL;
 }
 
 ArtworkModel::ArtworkModel(const QString &fileName, bool isOwner)
+:
+	m_mutex(new QMutex)
 {
-	m_isOwner = false;
+	m_data = new ArtworkModel_SharedData(fileName, isOwner);
-	setFilePath(fileName, isOwner);
 }
 
 ArtworkModel::ArtworkModel(const ArtworkModel &model)
+:
+	m_mutex(new QMutex)
 {
-	m_isOwner = false;
+	m_data = ArtworkModel_SharedData::attach(model.m_data);
-	setFilePath(model.m_filePath, model.m_isOwner);
 }
 
 ArtworkModel &ArtworkModel::operator=(const ArtworkModel &model)
 {
-	setFilePath(model.m_filePath, model.m_isOwner);
+	QMutexLocker lock(m_mutex);
+	ArtworkModel_SharedData::detach(&m_data);
+	m_data = ArtworkModel_SharedData::attach(model.m_data);
 	return (*this);
 }
 
 ArtworkModel::~ArtworkModel(void)
 {
-	clear();
+	QMutexLocker lock(m_mutex);
+	ArtworkModel_SharedData::detach(&m_data);
+	lock.unlock();
+	LAMEXP_DELETE(m_mutex);
 }
 
-
 ////////////////////////////////////////////////////////////
 // Public Functions
 ////////////////////////////////////////////////////////////
 
 const QString &ArtworkModel::filePath(void) const
 {
-	return m_filePath;
+	QMutexLocker lock(m_mutex);
+	return (m_data) ? m_data->m_filePath : m_nullString;
 }
 
 bool ArtworkModel::isOwner(void) const
 {
-	return m_isOwner;
+	QMutexLocker lock(m_mutex);
+	return (m_data) ? m_data->m_isOwner : false;
 }
 
 void ArtworkModel::setFilePath(const QString &newPath, bool isOwner)
 {
-	if(newPath.isEmpty() || m_filePath.isEmpty() || QString::compare(m_filePath, newPath,Qt::CaseInsensitive))
+	QMutexLocker lock(m_mutex);
-	{
+	ArtworkModel_SharedData::detach(&m_data);
-		clear();
+	m_data = new ArtworkModel_SharedData(newPath, isOwner);
-	
-		if(!newPath.isEmpty())
-		{
-			QMutexLocker lock(&m_mutex);
-
-			if(!m_refCount.contains(newPath))
-			{
-				m_refCount.insert(newPath, 0);
-				m_fileHandle.insert(newPath, new QFile(newPath));
-				m_fileHandle[newPath]->open(QIODevice::ReadOnly);
-			}
-
-			m_refCount[newPath]++;
-		}
-
-		m_filePath = newPath;
-		m_isOwner = isOwner;
-	}
 }
 
 void ArtworkModel::clear(void)
 {
-	if(!m_filePath.isEmpty())
+	QMutexLocker lock(m_mutex);
-	{
+	ArtworkModel_SharedData::detach(&m_data);
-		QMutexLocker lock(&m_mutex);
-
-		if(m_refCount.contains(m_filePath))
-		{
-			if(--m_refCount[m_filePath] < 1)
-			{
-				m_refCount.remove(m_filePath);
-
-				if(m_fileHandle.contains(m_filePath))
-				{
-					if(QFile *fileHandle = m_fileHandle.take(m_filePath))
-					{
-						if(m_isOwner)
-						{
-							fileHandle->remove();
-						}
-						else
-						{
-							fileHandle->close();
-						}
-						LAMEXP_DELETE(fileHandle);
-					}
-				}
-
-				if(m_isOwner)
-				{
-					QFile::remove(m_filePath);
-				}
-			}
-		}
-
-		m_filePath.clear();
-	}
 }

src/Model_Artwork.h

@@ -22,10 +22,9 @@
 #pragma once
 
 #include <QString>
-#include <QMap>
-#include <QMutex>
 
-class QFile;
+class ArtworkModel_SharedData;
+class QMutex;
 
 class ArtworkModel
 {
@@ -42,10 +41,7 @@ public:
 	void clear(void);
 
 private:
-	QString m_filePath;
+	const QString m_nullString;
-	bool m_isOwner;
+	ArtworkModel_SharedData *m_data;
-
+	QMutex *m_mutex;
-	static QMutex m_mutex;
-	static QMap<QString, unsigned int> m_refCount;
-	static QMap<QString, QFile*> m_fileHandle;
 };

src/Model_CueSheet.cpp

@@ -30,6 +30,7 @@
 #include <QTime>
 #include <QTextCodec>
 #include <QTextStream>
+#include <QMutexLocker>
 
 #include <float.h>
 #include <limits>

src/Model_CueSheet.h

@@ -29,6 +29,7 @@ class CueSheetFile;
 class QApplication;
 class QDir;
 class QTextCodec;
+class QFile;
 
 class CueSheetModel : public QAbstractItemModel
 {