2 files changed, 102 insertions, 26 deletions

diff --git a/intern/cycles/session/session.cpp b/intern/cycles/session/session.cpp
index d03063a7dda..f6e06f20aba 100644
--- a/intern/cycles/session/session.cpp
+++ b/intern/cycles/session/session.cpp
@@ -49,12 +49,9 @@ Session::Session(const SessionParams &params_, const SceneParams &scene_params)
 {
   TaskScheduler::init(params.threads);
 
-  session_thread_ = nullptr;
-
   delayed_reset_.do_reset = false;
 
   pause_ = false;
-  cancel_ = false;
   new_work_added_ = false;
 
   device = Device::create(params.device, stats, profiler);
@@ -73,48 +70,79 @@ Session::Session(const SessionParams &params_, const SceneParams &scene_params)
     }
     full_buffer_written_cb(filename);
   };
+
+  /* Create session thread. */
+  session_thread_ = new thread(function_bind(&Session::thread_run, this));
 }
 
 Session::~Session()
 {
+  /* Cancel any ongoing render operation. */
   cancel();
 
-  /* Make sure path tracer is destroyed before the device. This is needed because destruction might
-   * need to access device for device memory free. */
-  /* TODO(sergey): Convert device to be unique_ptr, and rely on C++ to destruct objects in the
+  /* Signal session thread to end. */
+  {
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    session_thread_state_ = SESSION_THREAD_END;
+  }
+  session_thread_cond_.notify_all();
+
+  /* Destroy session thread. */
+  session_thread_->join();
+  delete session_thread_;
+
+  /* Destroy path tracer, before the device. This is needed because destruction might need to
+   * access device for device memory free.
+   * TODO(sergey): Convert device to be unique_ptr, and rely on C++ to destruct objects in the
    * pre-defined order. */
   path_trace_.reset();
 
+  /* Destroy scene and device. */
   delete scene;
   delete device;
 
+  /* Stop task scheduler. */
   TaskScheduler::exit();
 }
 
 void Session::start()
 {
-  if (!session_thread_) {
-    session_thread_ = new thread(function_bind(&Session::run, this));
+  {
+    /* Signal session thread to start rendering. */
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    assert(session_thread_state_ == SESSION_THREAD_WAIT);
+    session_thread_state_ = SESSION_THREAD_RENDER;
   }
+
+  session_thread_cond_.notify_all();
 }
 
 void Session::cancel(bool quick)
 {
-  if (quick && path_trace_) {
-    path_trace_->cancel();
+  /* Check if session thread is rendering. */
+  bool rendering;
+  {
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    rendering = (session_thread_state_ == SESSION_THREAD_RENDER);
   }
 
-  if (session_thread_) {
-    /* wait for session thread to end */
+  if (rendering) {
+    /* Cancel path trace operations. */
+    if (quick && path_trace_) {
+      path_trace_->cancel();
+    }
+
+    /* Cancel other operations. */
     progress.set_cancel("Exiting");
 
+    /* Signal unpause in case the render was paused. */
     {
       thread_scoped_lock pause_lock(pause_mutex_);
       pause_ = false;
-      cancel_ = true;
     }
     pause_cond_.notify_all();
 
+    /* Wait for render thread to be cancelled or finished. */
     wait();
   }
 }
@@ -192,11 +220,46 @@ void Session::run_main_render_loop()
       break;
     }
   }
+}
+
+void Session::thread_run()
+{
+  while (true) {
+    {
+      thread_scoped_lock session_thread_lock(session_thread_mutex_);
+
+      if (session_thread_state_ == SESSION_THREAD_WAIT) {
+        /* Continue waiting for any signal from the main thread. */
+        session_thread_cond_.wait(session_thread_lock);
+        continue;
+      }
+      else if (session_thread_state_ == SESSION_THREAD_END) {
+        /* End thread immediately. */
+        break;
+      }
+    }
+
+    /* Execute a render. */
+    thread_render();
+
+    /* Go back from rendering to waiting. */
+    {
+      thread_scoped_lock session_thread_lock(session_thread_mutex_);
+      if (session_thread_state_ == SESSION_THREAD_RENDER) {
+        session_thread_state_ = SESSION_THREAD_WAIT;
+      }
+    }
+    session_thread_cond_.notify_all();
+  }
 
+  /* Flush any remaining operations and destroy display driver here. This ensure
+   * graphics API resources are created and destroyed all in the session thread,
+   * which can avoid problems contexts and multiple threads. */
   path_trace_->flush_display();
+  path_trace_->set_display_driver(nullptr);
 }
 
-void Session::run()
+void Session::thread_render()
 {
   if (params.use_profiling && (params.device.type == DEVICE_CPU)) {
     profiler.start();
@@ -338,9 +401,9 @@ bool Session::run_wait_for_work(const RenderWork &render_work)
   const bool no_work = !render_work;
   update_status_time(pause_, no_work);
 
-  /* Only leave the loop when rendering is not paused. But even if the current render is un-paused
-   * but there is nothing to render keep waiting until new work is added. */
-  while (!cancel_) {
+  /* Only leave the loop when rendering is not paused. But even if the current render is
+   * un-paused but there is nothing to render keep waiting until new work is added. */
+  while (!progress.get_cancel()) {
     scoped_timer pause_timer;
 
     if (!pause_ && (render_work || new_work_added_ || delayed_reset_.do_reset)) {
@@ -427,7 +490,8 @@ void Session::do_delayed_reset()
   tile_manager_.update(buffer_params_, scene);
 
   /* Update temp directory on reset.
-   * This potentially allows to finish the existing rendering with a previously configure temporary
+   * This potentially allows to finish the existing rendering with a previously configure
+   * temporary
    * directory in the host software and switch to a new temp directory when new render starts. */
   tile_manager_.set_temp_dir(params.temp_dir);
 
@@ -544,12 +608,14 @@ double Session::get_estimated_remaining_time() const
 
 void Session::wait()
 {
-  if (session_thread_) {
-    session_thread_->join();
-    delete session_thread_;
+  /* Wait until session thread either is waiting or ending. */
+  while (true) {
+    thread_scoped_lock session_thread_lock(session_thread_mutex_);
+    if (session_thread_state_ != SESSION_THREAD_RENDER) {
+      break;
+    }
+    session_thread_cond_.wait(session_thread_lock);
   }
-
-  session_thread_ = nullptr;
 }
 
 bool Session::update_scene(int width, int height)
diff --git a/intern/cycles/session/session.h b/intern/cycles/session/session.h
index adfd1346600..4017964d4aa 100644
--- a/intern/cycles/session/session.h
+++ b/intern/cycles/session/session.h
@@ -172,7 +172,8 @@ class Session {
     BufferParams buffer_params;
   } delayed_reset_;
 
-  void run();
+  void thread_run();
+  void thread_render();
 
   /* Update for the new iteration of the main loop in run implementation (run_cpu and run_gpu).
    *
@@ -205,10 +206,19 @@ class Session {
 
   int2 get_effective_tile_size() const;
 
-  thread *session_thread_;
+  /* Session thread that performs rendering tasks decoupled from the thread
+   * controlling the sessions. The thread is created and destroyed along with
+   * the session. */
+  thread *session_thread_ = nullptr;
+  thread_condition_variable session_thread_cond_;
+  thread_mutex session_thread_mutex_;
+  enum {
+    SESSION_THREAD_WAIT,
+    SESSION_THREAD_RENDER,
+    SESSION_THREAD_END,
+  } session_thread_state_ = SESSION_THREAD_WAIT;
 
   bool pause_ = false;
-  bool cancel_ = false;
   bool new_work_added_ = false;
 
   thread_condition_variable pause_cond_;