From 3f2906d4d114e763d72a86cfb2809a9819c1f898 Mon Sep 17 00:00:00 2001
From: Leo Franchi <lfranchi@kde.org>
Date: Sat, 16 Jun 2012 11:14:20 +0200
Subject: [PATCH] Safer locking of id() to prevent deadlocks

---
 src/libtomahawk/Album.cpp  | 19 ++++++++++++++-----
 src/libtomahawk/Artist.cpp | 18 ++++++++++++++----
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/src/libtomahawk/Album.cpp b/src/libtomahawk/Album.cpp
index fd92f2553..c719cf1da 100644
--- a/src/libtomahawk/Album.cpp
+++ b/src/libtomahawk/Album.cpp
@@ -29,6 +29,8 @@
 
 #include "utils/Logger.h"
 
+#include <QReadWriteLock>
+
 using namespace Tomahawk;
 
 QHash< QString, album_ptr > Album::s_albumsByName = QHash< QString, album_ptr >();
@@ -36,7 +38,7 @@ QHash< unsigned int, album_ptr > Album::s_albumsById = QHash< unsigned int, albu
 
 static QMutex s_nameCacheMutex;
 static QMutex s_idCacheMutex;
-static QMutex s_idMutex;
+static QReadWriteLock s_idMutex;
 
 Album::~Album()
 {
@@ -155,18 +157,25 @@ Album::loadId( bool autoCreate )
 unsigned int
 Album::id() const
 {
-    QMutexLocker l( &s_idMutex );
+    s_idMutex.lockForRead();
+    const bool waiting = m_waitingForId;
+    unsigned int finalId = m_id;
+    s_idMutex.unlock();
 
-    if ( m_waitingForId )
+    if ( waiting )
     {
-        m_id = m_idFuture.get();
+        finalId = m_idFuture.get();
+
+        s_idMutex.lockForWrite();
+        m_id = finalId;
         m_waitingForId = false;
 
         if ( m_id > 0 )
             s_albumsById[ m_id ] = m_ownRef.toStrongRef();
+        s_idMutex.unlock();
     }
 
-    return m_id;
+    return finalId;
 }
 
 
diff --git a/src/libtomahawk/Artist.cpp b/src/libtomahawk/Artist.cpp
index 5c08ee9c6..b58ce992f 100644
--- a/src/libtomahawk/Artist.cpp
+++ b/src/libtomahawk/Artist.cpp
@@ -30,6 +30,8 @@
 
 #include "utils/Logger.h"
 
+#include <QReadWriteLock>
+
 using namespace Tomahawk;
 
 QHash< QString, artist_ptr > Artist::s_artistsByName = QHash< QString, artist_ptr >();
@@ -37,7 +39,7 @@ QHash< unsigned int, artist_ptr > Artist::s_artistsById = QHash< unsigned int, a
 
 static QMutex s_nameCacheMutex;
 static QMutex s_idCacheMutex;
-static QMutex s_idMutex;
+static QReadWriteLock s_idMutex;
 
 Artist::~Artist()
 {
@@ -235,14 +237,22 @@ Artist::loadId( bool autoCreate )
 unsigned int
 Artist::id() const
 {
-    QMutexLocker l( &s_idMutex );
-    if ( m_waitingForFuture )
+    s_idMutex.lockForRead();
+    const bool waiting = m_waitingForFuture;
+    unsigned int finalid = m_id;
+    s_idMutex.unlock();
+
+    if ( waiting )
     {
-        m_id = m_idFuture.get();
+        finalid = m_idFuture.get();
+
+        s_idMutex.lockForWrite();
+        m_id = finalid;
         m_waitingForFuture = false;
 
         if ( m_id > 0 )
             s_artistsById[ m_id ] = m_ownRef.toStrongRef();
+        s_idMutex.unlock();
     }
 
     return m_id;