render: improved threading stability

app/threading/threadpool.cpp

@@ -54,6 +54,17 @@ ThreadPool::~ThreadPool()
   }
 }
 
+bool ThreadPool::RemoveTicket(RenderTicketPtr ticket)
+{
+  auto it = std::find(ticket_queue_.begin(), ticket_queue_.end(), ticket);
+  if (it == ticket_queue_.end()) {
+    return false;
+  }
+
+  ticket_queue_.erase(it);
+  return true;
+}
+
 void ThreadPool::AddTicket(RenderTicketPtr ticket, bool prioritize)
 {
   if (prioritize) {
@@ -72,16 +83,15 @@ void ThreadPool::RunNext()
     RenderTicketPtr ticket = ticket_queue_.front();
     ticket_queue_.pop_front();
 
-    if (!ticket->WasCancelled()) {
-      ThreadPoolThread* thread = available_threads_.front();
-      available_threads_.pop_front();
+    ThreadPoolThread* thread = available_threads_.front();
+    available_threads_.pop_front();
 
-      // Move ticket to other thread so event processing can occur there
-      ticket->moveToThread(thread);
+    // Move ticket to other thread so event processing can occur there
+    ticket->Start();
+    ticket->moveToThread(thread);
 
-      // Run the ticket in the thread, which actually just calls our virtual function RunTicket
-      thread->RunTicket(ticket);
-    }
+    // Run the ticket in the thread, which actually just calls our virtual function RunTicket
+    thread->RunTicket(ticket);
   }
 }
 

app/threading/threadpool.h

@@ -42,6 +42,8 @@ public:
 
   virtual void RunTicket(RenderTicketPtr ticket) const = 0;
 
+  bool RemoveTicket(RenderTicketPtr ticket);
+
 public slots:
   void AddTicket(olive::RenderTicketPtr ticket, bool prioritize = false);
 

app/threading/threadticket.cpp

@@ -23,20 +23,37 @@
 namespace olive {
 
 RenderTicket::RenderTicket() :
-  started_(false),
-  finished_(false),
-  cancelled_(false)
+  is_running_(false),
+  has_result_(false),
+  finish_count_(0)
 {
   SetJobTime();
 }
 
-void RenderTicket::WaitForFinished()
+void RenderTicket::WaitForFinished(QMutex *mutex)
+{
+  if (is_running_) {
+    wait_.wait(mutex);
+  }
+}
+
+void RenderTicket::Start()
 {
   QMutexLocker locker(&lock_);
 
-  if (!finished_) {
-    wait_.wait(&lock_);
-  }
+  is_running_ = true;
+  has_result_ = false;
+  result_.clear();
+}
+
+void RenderTicket::Finish()
+{
+  FinishInternal(false, QVariant());
+}
+
+void RenderTicket::Finish(QVariant result)
+{
+  FinishInternal(true, result);
 }
 
 QVariant RenderTicket::Get()
@@ -48,57 +65,61 @@ QVariant RenderTicket::Get()
   return result_;
 }
 
-bool RenderTicket::HasStarted()
+void RenderTicket::WaitForFinished()
 {
   QMutexLocker locker(&lock_);
-  return started_;
+
+  WaitForFinished(&lock_);
 }
 
-bool RenderTicket::IsFinished(bool lock)
+bool RenderTicket::IsRunning(bool lock)
 {
   if (lock) {
     lock_.lock();
   }
 
-  bool finished = finished_;
+  bool running = is_running_;
 
   if (lock) {
     lock_.unlock();
   }
 
-  return finished;
+  return running;
 }
 
-bool RenderTicket::WasCancelled()
+int RenderTicket::GetFinishCount(bool lock)
 {
-  QMutexLocker locker(&lock_);
-
-  return cancelled_;
-}
-
-void RenderTicket::Start()
-{
-  QMutexLocker locker(&lock_);
-
-  if (!started_ && !finished_) {
-    started_ = true;
+  if (lock) {
+    lock_.lock();
   }
+
+  int count = finish_count_;
+
+  if (lock) {
+    lock_.unlock();
+  }
+
+  return count;
 }
 
-void RenderTicket::Finish(QVariant result, bool cancelled)
+bool RenderTicket::HasResult()
 {
   QMutexLocker locker(&lock_);
 
-  if (!started_) {
-    qWarning() << "Tried to finish a ticket that hadn't started";
-  } else if (finished_) {
-    // Do nothing
-    return;
-  } else {
-    finished_ = true;
-    cancelled_ = cancelled;
+  return has_result_;
+}
 
+void RenderTicket::FinishInternal(bool has_result, QVariant result)
+{
+  QMutexLocker locker(&lock_);
+
+  if (!is_running_) {
+    qWarning() << "Tried to finish ticket that wasn't running";
+  } else {
+    is_running_ = false;
+    has_result_ = has_result;
     result_ = result;
+    finish_count_++;
 
     wait_.wakeAll();
 
@@ -108,23 +129,4 @@ void RenderTicket::Finish(QVariant result, bool cancelled)
   }
 }
 
-void RenderTicket::Cancel()
-{
-  QMutexLocker locker(&lock_);
-
-  if (!finished_) {
-    cancelled_ = true;
-
-    if (!started_) {
-      finished_ = true;
-
-      wait_.wakeAll();
-
-      locker.unlock();
-
-      emit Finished();
-    }
-  }
-}
-
 }

app/threading/threadticket.h

@@ -48,39 +48,91 @@ public:
     job_time_ = QDateTime::currentMSecsSinceEpoch();
   }
 
-  void WaitForFinished();
+  /**
+   * @brief Get the ticket's current state
+   *
+   * This function is thread safe, unless `lock` is set to false. Then the caller has responsibility
+   * of locking the mutex before and unlocking after this function is called.
+   */
+  bool IsRunning(bool lock = true);
 
+  /**
+   * @brief Determine how many times ticket has been finished
+   *
+   * This function is thread safe, unless `lock` is set to false. Then the caller has responsibility
+   * of locking the mutex before and unlocking after this function is called.
+   */
+  int GetFinishCount(bool lock = true);
+
+  /**
+   * @brief Check if this ticket has a result
+   *
+   * If this ticket is running, this will always return false.
+   */
+  bool HasResult();
+
+  /**
+   * @brief Get value, if any
+   */
   QVariant Get();
 
-  bool HasStarted();
-
-  bool IsFinished(bool lock = true);
-
-  bool WasCancelled();
+  /**
+   * @brief Wait for ticket to be finished
+   *
+   * If this ticket is not running, this function returns immediately.
+   */
+  void WaitForFinished();
+  void WaitForFinished(QMutex* mutex);
 
+  /**
+   * @brief Access this ticket's mutex
+   *
+   * Use if you're doing several operations on a ticket and need to ensure thread safety while
+   * doing so. Most of the time this isn't necessary since all functions are thread safe by default.
+   */
   QMutex* lock()
   {
     return &lock_;
   }
 
+  /**
+   * @brief Signal to the ticket that it is running
+   *
+   * If any value is set, it is cleared.
+   */
   void Start();
 
-  void Finish(QVariant result, bool cancelled);
+  /**
+   * @brief Finish ticket with no value
+   *
+   * Sets ticket to no longer running and assume it has received no result.
+   */
+  void Finish();
 
-  void Cancel();
+  /**
+   * @brief Finish ticket with value
+   *
+   * Sets ticket to no longer running and provide a value generated by the operation requested.
+   */
+  void Finish(QVariant result);
 
 signals:
+  /**
+   * @brief Emitted when finish has been called by any means (either cancelled or with a result)
+   */
   void Finished();
 
 private:
-  bool started_;
+  void FinishInternal(bool has_result, QVariant result);
 
-  bool finished_;
-
-  bool cancelled_;
+  bool is_running_;
 
   QVariant result_;
 
+  bool has_result_;
+
+  int finish_count_;
+
   QMutex lock_;
 
   QWaitCondition wait_;

app/threading/threadticketwatcher.cpp

@@ -42,38 +42,22 @@ void RenderTicketWatcher::SetTicket(RenderTicketPtr ticket)
 
   ticket_ = ticket;
 
+  // Lock ticket so we can query if it's already finished by the time this code runs
   QMutexLocker locker(ticket->lock());
 
-  if (ticket_->IsFinished(false)) {
+  connect(ticket_.get(), &RenderTicket::Finished, this, &RenderTicketWatcher::TicketFinished);
+
+  if (!ticket_->IsRunning(false) && ticket_->GetFinishCount(false) > 0) {
+    // Ticket has already finished before, so we emit a signal
     locker.unlock();
-    emit Finished(this);
-  } else {
-    connect(ticket_.get(), &RenderTicket::Finished, this, &RenderTicketWatcher::TicketFinished);
+    TicketFinished();
   }
 }
 
-bool RenderTicketWatcher::WasCancelled()
+bool RenderTicketWatcher::IsRunning()
 {
   if (ticket_) {
-    return ticket_->WasCancelled();
-  } else {
-    return false;
-  }
-}
-
-bool RenderTicketWatcher::IsFinished()
-{
-  if (ticket_) {
-    return ticket_->IsFinished();
-  } else {
-    return false;
-  }
-}
-
-bool RenderTicketWatcher::HasStarted()
-{
-  if (ticket_) {
-    return ticket_->HasStarted();
+    return ticket_->IsRunning();
   } else {
     return false;
   }
@@ -95,10 +79,12 @@ QVariant RenderTicketWatcher::Get()
   }
 }
 
-void RenderTicketWatcher::Cancel()
+bool RenderTicketWatcher::HasResult()
 {
   if (ticket_) {
-    ticket_->Cancel();
+    return ticket_->HasResult();
+  } else {
+    return false;
   }
 }
 

app/threading/threadticketwatcher.h

@@ -38,26 +38,23 @@ public:
 
   void SetTicket(RenderTicketPtr ticket);
 
-  void Cancel();
-
-  bool WasCancelled();
-
-  bool IsFinished();
-
-  bool HasStarted();
+  bool IsRunning();
 
   void WaitForFinished();
 
   QVariant Get();
 
+  bool HasResult();
+
 signals:
   void Finished(RenderTicketWatcher* watcher);
 
 private:
-  void TicketFinished();
-
   RenderTicketPtr ticket_;
 
+private slots:
+  void TicketFinished();
+
 };
 
 }