Queue up spotify db ops if playlist is busy, otherwise we hit a race condition

src/accounts/spotify/SpotifyPlaylistUpdater.cpp

@@ -86,6 +86,7 @@ SpotifyPlaylistUpdater::init()
     connect( playlist().data(), SIGNAL( tracksInserted( QList<Tomahawk::plentry_ptr>, int ) ), this, SLOT( tomahawkTracksInserted( QList<Tomahawk::plentry_ptr>, int ) ) );
     connect( playlist().data(), SIGNAL( tracksRemoved( QList<Tomahawk::query_ptr> ) ), this, SLOT( tomahawkTracksRemoved( QList<Tomahawk::query_ptr> ) ) );
     connect( playlist().data(), SIGNAL( renamed( QString, QString ) ), this, SLOT( tomahawkPlaylistRenamed( QString, QString ) ) );
+    connect( playlist().data(), SIGNAL( revisionLoaded( Tomahawk::PlaylistRevision ) ), this, SLOT( playlistRevisionLoaded() ), Qt::QueuedConnection ); // Queued so that in playlist.cpp:443 we let the playlist clear its own queue first
     // TODO reorders in a playlist
 }
 
@@ -147,6 +148,19 @@ SpotifyPlaylistUpdater::checkDeleteDialog() const
 }
 
 
+void
+SpotifyPlaylistUpdater::playlistRevisionLoaded()
+{
+    if ( m_queuedOps.isEmpty() ) // nothing queued
+        return;
+
+    if ( playlist()->busy() ) // not ready yet, we'll get another revision loaded
+        return;
+
+    _detail::Closure* next = m_queuedOps.dequeue();
+    next->forceInvoke();
+}
+
 
 void
 SpotifyPlaylistUpdater::saveToSettings( const QString& group ) const
@@ -204,6 +218,14 @@ SpotifyPlaylistUpdater::sync() const
 void
 SpotifyPlaylistUpdater::spotifyTracksAdded( const QVariantList& tracks, const QString& startPosId, const QString& newRev, const QString& oldRev )
 {
+    if( playlist()->busy() )
+    {
+        // We might still be waiting for a add/remove tracks command to finish, so the entries we get here might be stale
+        // wait for any to be complete
+        m_queuedOps << NewClosure( 0, "", this, SLOT(spotifyTracksAdded(QVariantList, QString, QString, QString)), tracks, startPosId, newRev, oldRev );
+        return;
+    }
+
     const QList< query_ptr > queries = variantToQueries( tracks );
 
     qDebug() << Q_FUNC_INFO << "inserting tracks in middle of tomahawk playlist, from spotify command!" << tracks << startPosId << newRev << oldRev;
@@ -227,7 +249,7 @@ SpotifyPlaylistUpdater::spotifyTracksAdded( const QVariantList& tracks, const QS
     if ( pos == -1 || pos > entries.size() )
         pos = entries.size();
 
-    qDebug() << Q_FUNC_INFO << "inserting tracks at position:" << pos;
+    qDebug() << Q_FUNC_INFO << "inserting tracks at position:" << pos << "(playlist has current size:" << entries << ")";
 
     m_blockUpdatesForNextRevision = true;
     playlist()->insertEntries( queries, pos, playlist()->currentrevision() );
@@ -237,6 +259,14 @@ SpotifyPlaylistUpdater::spotifyTracksAdded( const QVariantList& tracks, const QS
 void
 SpotifyPlaylistUpdater::spotifyTracksRemoved( const QVariantList& trackIds, const QString& newRev, const QString& oldRev )
 {
+    if( playlist()->busy() )
+    {
+        // We might still be waiting for a add/remove tracks command to finish, so the entries we get here might be stale
+        // wait for any to be complete
+        m_queuedOps << NewClosure( 0, "", this, SLOT(spotifyTracksRemoved(QVariantList, QString, QString)), trackIds, newRev, oldRev );
+        return;
+    }
+
     qDebug() << Q_FUNC_INFO << "remove tracks in middle of tomahawk playlist, from spotify command!" << trackIds << newRev << oldRev;
     // Uh oh, dont' want to get out of sync!!
 //     Q_ASSERT( m_latestRev == oldRev );
@@ -285,6 +315,14 @@ SpotifyPlaylistUpdater::spotifyTracksRemoved( const QVariantList& trackIds, cons
 void
 SpotifyPlaylistUpdater::spotifyPlaylistRenamed( const QString& title, const QString& newRev, const QString& oldRev )
 {
+    if( playlist()->busy() )
+    {
+        // We might still be waiting for a add/remove tracks command to finish, so the entries we get here might be stale
+        // wait for any to be complete
+        m_queuedOps << NewClosure( 0, "", this, SLOT(spotifyPlaylistRenamed(QString, QString, QString)), title, newRev, oldRev );
+        return;
+    }
+
     Q_UNUSED( newRev );
     Q_UNUSED( oldRev );
     /// @note to self: should do some checking before trying to update
@@ -309,6 +347,13 @@ void
 SpotifyPlaylistUpdater::spotifyTracksMoved( const QVariantList& tracks, const QString& newRev, const QString& oldRev )
 {
     // TODO
+//     if( playlist()->busy() )
+//     {
+//         // We might still be waiting for a add/remove tracks command to finish, so the entries we get here might be stale
+//         // wait for any to be complete
+//         m_queuedOps << NewClosure( 0, "", this, "spotifyPlaylistRenamed", title, newRev, oldRev );
+//         return;
+//     }
 }
 
 

src/accounts/spotify/SpotifyPlaylistUpdater.h

@@ -20,7 +20,9 @@
 #define SPOTIFYPLAYLISTUPDATER_H
 
 #include "playlist/PlaylistUpdaterInterface.h"
+#include "utils/closure.h"
 
+#include <QQueue>
 #include <QVariant>
 
 namespace Tomahawk {
@@ -53,6 +55,7 @@ public:
 
     QString spotifyId() const { return m_spotifyId; }
 
+public slots:
     /// Spotify callbacks when we are directly instructed from the resolver
     void spotifyTracksAdded( const QVariantList& tracks, const QString& startPosId, const QString& newRev, const QString& oldRev );
     void spotifyTracksRemoved( const QVariantList& tracks, const QString& newRev, const QString& oldRev );
@@ -72,6 +75,8 @@ private slots:
     void onTracksRemovedReturn( const QString& msgType, const QVariantMap& msg );
 
     void checkDeleteDialog() const;
+
+    void playlistRevisionLoaded();
 private:
     void init();
 
@@ -86,6 +91,7 @@ private:
     bool m_blockUpdatesForNextRevision;
     bool m_sync;
 
+    QQueue<_detail::Closure*> m_queuedOps;
 #ifndef ENABLE_HEADLESS
     static QPixmap* s_typePixmap;
 #endif

src/libtomahawk/resolvers/scriptresolver.cpp

@@ -308,7 +308,7 @@ ScriptResolver::cmdExited( int code, QProcess::ExitStatus status )
         return;
     }
 
-    if ( m_num_restarts < 10 )
+    if ( m_num_restarts < 0 )
     {
         m_num_restarts++;
         tLog() << "*** Restart num" << m_num_restarts;

src/libtomahawk/utils/closure.cpp

@@ -54,7 +54,17 @@ Closure::Closure(QObject* sender,
 Closure::~Closure() {
 }
 
+
+void Closure::forceInvoke()
+{
+    Invoked();
+}
+
+
 void Closure::Connect(QObject* sender, const char* signal) {
+  if (!sender)
+      return;
+
   bool success = connect(sender, signal, SLOT(Invoked()));
   Q_ASSERT(success);
   success = connect(sender, SIGNAL(destroyed()), SLOT(Cleanup()));

src/libtomahawk/utils/closure.h

@@ -18,6 +18,8 @@
 #ifndef CLOSURE_H
 #define CLOSURE_H
 
+#include "dllmacro.h"
+
 #include <tr1/functional>
 
 #include <QMetaMethod>
@@ -29,7 +31,7 @@
 
 namespace _detail {
 
-class ClosureArgumentWrapper {
+class DLLEXPORT ClosureArgumentWrapper {
  public:
   virtual ~ClosureArgumentWrapper() {}
 
@@ -49,7 +51,7 @@ class ClosureArgument : public ClosureArgumentWrapper {
   T data_;
 };
 
-class Closure : public QObject, boost::noncopyable {
+class DLLEXPORT Closure : public QObject, boost::noncopyable {
   Q_OBJECT
 
  public:
@@ -67,6 +69,15 @@ class Closure : public QObject, boost::noncopyable {
 
   virtual ~Closure();
 
+  /**
+   * If you don't this Closure to act on a signal, but just act like
+   *  a closure in that it saves some args and delivers them on demand later
+   *
+   * Only call this is you passed a null QObject* as a sender! Otherwise you
+   *  might delete your object twice :)
+   */
+  void forceInvoke();
+
  private slots:
   void Invoked();
   void Cleanup();
@@ -84,7 +95,7 @@ class Closure : public QObject, boost::noncopyable {
   boost::scoped_ptr<const ClosureArgumentWrapper> val3_;
 };
 
-class SharedPointerWrapper {
+class DLLEXPORT SharedPointerWrapper {
  public:
   virtual ~SharedPointerWrapper() {}
   virtual QObject* data() const = 0;