reapack

commit bbda91c48eed757c9531db6a2fa9658caf8f1b1e
parent 77b0d2fb40b714ba60e43b2605f91b9b6605fd4c
Author: cfillion <cfillion@users.noreply.github.com>
Date:   Thu, 14 Jan 2016 16:33:09 -0500

fix transaction cancelling when tasks are running

waiting for aborted-before-start downloads to finish was not a good idea

Diffstat:
M
src/download.cpp
 | 
12
++++++++----
M
src/download.hpp
 | 
6
++++--
M
src/task.cpp
 | 
18
+-----------------
M
src/task.hpp
 | 
3
---
M
src/transaction.cpp
 | 
6
+++---

5 files changed, 16 insertions(+), 29 deletions(-)
diff --git a/src/download.cpp b/src/download.cpp
@@ -44,7 +44,7 @@ Download::~Download()
 void Download::reset()
 {
   m_aborted = false;
-  m_finished = false;
+  m_running = false;
   m_status = 0;
   m_contents.clear();
 }
@@ -72,6 +72,7 @@ void Download::start()
 
   reset();
 
+  m_running = true;
   m_onStart();
 
   RegisterStart();
@@ -184,7 +185,7 @@ void Download::finish(const int status, const string &contents)
 
   WDL_MutexLock lock(&m_mutex);
 
-  m_finished = true;
+  m_running = false;
   m_status = status;
   m_contents = contents;
 
@@ -204,11 +205,11 @@ void Download::finishInMainThread()
   m_onDestroy();
 }
 
-bool Download::isFinished()
+bool Download::isRunning()
 {
   WDL_MutexLock lock(&m_mutex);
 
-  return m_finished;
+  return m_running;
 }
 
 bool Download::isAborted()
@@ -242,6 +243,9 @@ void DownloadQueue::push(Download *dl)
 
   dl->onDestroy([=] {
      delete dl;
+
+    if(idle())
+      m_onDone(nullptr);
   });
 
   m_queue.push(dl);
diff --git a/src/download.hpp b/src/download.hpp
@@ -41,7 +41,7 @@ public:
   int status() const { return m_status; }
   const std::string &contents() const { return m_contents; }
 
-  bool isFinished();
+  bool isRunning();
   bool isAborted();
 
   void onStart(const Callback &callback) { m_onStart.connect(callback); }
@@ -75,7 +75,7 @@ private:
 
   HANDLE m_threadHandle;
   bool m_aborted;
-  bool m_finished;
+  bool m_running;
   int m_status;
   std::string m_contents;
 
@@ -100,6 +100,7 @@ public:
   bool idle() const { return m_queue.empty() && m_running.empty(); }
 
   void onPush(const Callback &callback) { m_onPush.connect(callback); }
+  void onDone(const Callback &callback) { m_onDone.connect(callback); }
 
 private:
   void clear();
@@ -108,6 +109,7 @@ private:
   std::vector<Download *> m_running;
 
   Signal m_onPush;
+  Signal m_onDone;
 };
 
 #endif
diff --git a/src/task.cpp b/src/task.cpp
@@ -42,13 +42,8 @@ void Task::install(Version *ver)
     Download *dl = new Download(src->fullName(), src->url());
     dl->onFinish(bind(&Task::saveSource, this, dl, src));
 
-    m_remaining.push_back(dl);
     m_transaction->downloadQueue()->push(dl);
 
-    // executing finish after the download is deleted
-    // prevents the download queue from being deleted before the download is
-    dl->onFinish(bind(&Task::finish, this));
-
     // skip duplicate files
     do { it++; } while(it != sources.end() && path == it->first);
   }
@@ -56,8 +51,6 @@ void Task::install(Version *ver)
 
 void Task::saveSource(Download *dl, Source *src)
 {
-  m_remaining.erase(remove(m_remaining.begin(), m_remaining.end(), dl));
-
   if(m_isCancelled)
     return;
 
@@ -75,20 +68,11 @@ void Task::saveSource(Download *dl, Source *src)
   }
 }
 
-void Task::finish()
-{
-  if(!m_remaining.empty())
-    return;
-
-  m_onFinish();
-}
-
 void Task::cancel()
 {
   m_isCancelled = true;
 
-  for(Download *dl : m_remaining)
-    dl->abort();
+  // it's the transaction queue's job to abort the running downloads, not ours
 
   rollback();
 }
diff --git a/src/task.hpp b/src/task.hpp
@@ -35,7 +35,6 @@ public:
   Task(Transaction *parent);
 
   void onCommit(const Callback &callback) { m_onCommit.connect(callback); }
-  void onFinish(const Callback &callback) { m_onFinish.connect(callback); }
 
   void install(Version *ver);
   void commit();
@@ -54,11 +53,9 @@ private:
 
   Transaction *m_transaction;
   bool m_isCancelled;
-  std::vector<Download *> m_remaining;
   std::vector<PathPair> m_files;
 
   Signal m_onCommit;
-  Signal m_onFinish;
 };
 
 #endif
diff --git a/src/transaction.cpp b/src/transaction.cpp
@@ -32,6 +32,8 @@ Transaction::Transaction(Registry *reg, const Path &root)
 {
   m_dbPath = m_root + "ReaPack";
 
+  m_queue.onDone(bind(&Transaction::finish, this));
+
   RecursiveCreateDirectory(m_dbPath.join().c_str(), 0);
 }
 
@@ -129,7 +131,6 @@ void Transaction::run()
           );
         }
       });
-      task->onFinish(bind(&Transaction::finish, this));
 
       m_tasks.push_back(task);
     }
@@ -175,8 +176,7 @@ bool Transaction::saveFile(Download *dl, const Path &path)
 
 void Transaction::finish()
 {
-  if(!m_queue.idle())
-    return;
+  // called when the download queue is done, or if there is nothing to do
 
   if(!m_isCancelled) {
     for(Task *task : m_tasks)