From f9738dba16ffb35b3f16b3ae6958b9c2367c929d Mon Sep 17 00:00:00 2001
From: "Uwe L. Korn" <uwelk@xhochy.com>
Date: Sun, 16 Jun 2013 11:17:05 +0200
Subject: [PATCH] If a ControlConnection is not anymore responsible for a
 source, it should not touch it.

---
 src/libtomahawk/Source.cpp                    |  3 +++
 src/libtomahawk/network/ControlConnection.cpp | 26 +++++++++++++++++--
 src/libtomahawk/network/ControlConnection.h   | 10 +++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/libtomahawk/Source.cpp b/src/libtomahawk/Source.cpp
index 278d817a3..3d817c6c0 100644
--- a/src/libtomahawk/Source.cpp
+++ b/src/libtomahawk/Source.cpp
@@ -100,6 +100,7 @@ bool
 Source::setControlConnection( ControlConnection* cc )
 {
     Q_D( Source );
+
     QMutexLocker locker( &d->setControlConnectionMutex );
     if ( !d->cc.isNull() && d->cc->isReady() && d->cc->isRunning() )
     {
@@ -107,6 +108,8 @@ Source::setControlConnection( ControlConnection* cc )
         peerInfoDebug( (*cc->peerInfos().begin()) ) << Q_FUNC_INFO << "Comparing" << cc->id() << "and" << nodeid << "to detect duplicate connection, outbound:" << cc->outbound();
         if ( cc->id() < nodeid && d->cc->outbound() )
         {
+            // Tell the ControlConnection it is not anymore responsible for us.
+            d->cc->unbindFromSource();
             // This ControlConnection is not needed anymore, get rid of it!
             d->cc->deleteLater();
             // Use new ControlConnection
diff --git a/src/libtomahawk/network/ControlConnection.cpp b/src/libtomahawk/network/ControlConnection.cpp
index 2a749d50d..b6e93bba5 100644
--- a/src/libtomahawk/network/ControlConnection.cpp
+++ b/src/libtomahawk/network/ControlConnection.cpp
@@ -2,6 +2,7 @@
  *
  *   Copyright 2010-2011, Christian Muehlhaeuser <muesli@tomahawk-player.org>
  *   Copyright 2010-2012, Jeff Mitchell <jeff@tomahawk-player.org>
+ *   Copyright 2013, Uwe L. Korn <uwelk@xhochy.com>
  *
  *   Tomahawk is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -55,10 +56,13 @@ ControlConnection::ControlConnection( Servent* parent )
 
 ControlConnection::~ControlConnection()
 {
+    QReadLocker locker( &m_sourceLock );
     tDebug( LOGVERBOSE ) << Q_FUNC_INFO << id() << name();
 
     if ( !m_source.isNull() )
+    {
         m_source->setOffline();
+    }
 
     delete m_pingtimer;
     m_servent->unregisterControlConnection( this );
@@ -70,9 +74,18 @@ ControlConnection::~ControlConnection()
 source_ptr
 ControlConnection::source() const
 {
+    // We return a copy of the shared pointer, no need for a longer lock
+    QReadLocker locker( &m_sourceLock );
     return m_source;
 }
 
+void
+ControlConnection::unbindFromSource()
+{
+    QWriteLocker locker( &m_sourceLock );
+    m_source.clear();
+}
+
 
 Connection*
 ControlConnection::clone()
@@ -88,6 +101,7 @@ void
 ControlConnection::setup()
 {
     qDebug() << Q_FUNC_INFO << id() << name();
+    QWriteLocker sourceLocker( &m_sourceLock );
 
     if ( !m_source.isNull() )
     {
@@ -132,9 +146,10 @@ ControlConnection::setup()
 void
 ControlConnection::registerSource()
 {
+    QReadLocker sourceLocker( &m_sourceLock );
     QSharedPointer<QMutexLocker> locker = m_source->acquireLock();
     // Only continue if we are still the ControlConnection associated with this source.
-    if ( m_source->controlConnection() == this )
+    if ( !m_source.isNull() &&  m_source->controlConnection() == this )
     {
         qDebug() << Q_FUNC_INFO << m_source->id();
         Source* source = (Source*) sender();
@@ -150,6 +165,13 @@ ControlConnection::registerSource()
 void
 ControlConnection::setupDbSyncConnection( bool ondemand )
 {
+    QReadLocker locker( &m_sourceLock );
+    if ( m_source.isNull() )
+    {
+        // We were unbind from the Source, nothing to do here, just waiting to be deleted.
+        return;
+    }
+
     qDebug() << Q_FUNC_INFO << ondemand << m_source->id() << m_dbconnkey << m_dbsyncconn << m_registered;
 
     if ( m_dbsyncconn || !m_registered )
@@ -206,7 +228,6 @@ ControlConnection::dbSyncConnFinished( QObject* c )
 DBSyncConnection*
 ControlConnection::dbSyncConnection()
 {
-    qDebug() << Q_FUNC_INFO << m_source->id();
     if ( !m_dbsyncconn )
     {
         setupDbSyncConnection( true );
@@ -292,6 +313,7 @@ ControlConnection::onPingTimer()
 {
     if ( m_pingtimer_mark.elapsed() >= TCP_TIMEOUT * 1000 )
     {
+        QReadLocker locker( &m_sourceLock );
         qDebug() << "Timeout reached! Shutting down connection to" << m_source->friendlyName();
         shutdown( true );
     }
diff --git a/src/libtomahawk/network/ControlConnection.h b/src/libtomahawk/network/ControlConnection.h
index bddc0f9f0..c6753eda2 100644
--- a/src/libtomahawk/network/ControlConnection.h
+++ b/src/libtomahawk/network/ControlConnection.h
@@ -32,6 +32,7 @@
 
 #include "DllMacro.h"
 
+#include <QReadWriteLock>
 #include <QTime>
 #include <QTimer>
 
@@ -51,6 +52,11 @@ public:
 
     Tomahawk::source_ptr source() const;
 
+    /**
+     * Tell this ControlConnection that is no longer controlling the source and should not do any actions on it.
+     */
+    void unbindFromSource();
+
     void addPeerInfo( const Tomahawk::peerinfo_ptr& peerInfo );
     void removePeerInfo( const Tomahawk::peerinfo_ptr& peerInfo );
     void setShutdownOnEmptyPeerInfos( bool shutdownOnEmptyPeerInfos );
@@ -72,6 +78,10 @@ private:
     void setupDbSyncConnection( bool ondemand = false );
 
     Tomahawk::source_ptr m_source;
+    /**
+     * Lock acces to the source member. A "write" access is only if we change the value of source, not if doing a non-const call.
+     */
+    mutable QReadWriteLock m_sourceLock;
     DBSyncConnection* m_dbsyncconn;
 
     QString m_dbconnkey;