[Bf-blender-cvs] [952afbf] master: BLI_task: Fix/enhance logic of exiting worker threads.

[Bastien Montagne]

Commit: 952afbf9168c8601e61fa64ffb37e6d93f1d3d61
Author: Bastien Montagne
Date:   Mon Nov 2 16:35:53 2015 +0100
Branches: master
https://developer.blender.org/rB952afbf9168c8601e61fa64ffb37e6d93f1d3d61

BLI_task: Fix/enhance logic of exiting worker threads.

In previous code, worker would exit in case it gets awoken from a condition_wait() and
task queue is empty. However, there may be spurious wake up (either due to pthread itself,
or to some race condition between workers) that would lead to wrongly exiting a worker before
we actually exit the whole scheduler. See code for more details.

===================================================================

M	source/blender/blenlib/intern/task.c

===================================================================

diff --git a/source/blender/blenlib/intern/task.c b/source/blender/blenlib/intern/task.c
index a125bf7..7ce230e 100644
--- a/source/blender/blenlib/intern/task.c
+++ b/source/blender/blenlib/intern/task.c
@@ -117,11 +117,22 @@ static bool task_scheduler_thread_wait_pop(TaskScheduler *scheduler, Task **task
 
 	do {
 		Task *current_task;
-		if (!scheduler->queue.first) {
+
+		/* Assuming we can only have a void queue in 'exit' case here seems logical (we should only be here after
+		 * our worker thread has been woken up from a condition_wait(), which only happens after a new task was
+		 * added to the queue), but it is wrong.
+		 * Waiting on condition may wake up the thread even if condition is not signaled (spurious wakeups), and some
+		 * race condition may also empty the queue **after** condition has been signaled, but **before** awoken thread
+		 * reaches this point...
+		 * See http://stackoverflow.com/questions/8594591/why-does-pthread-cond-wait-have-spurious-wakeups
+		 *
+		 * So we only abort here if do_exit is set.
+		 */
+		if (scheduler->do_exit) {
 			BLI_mutex_unlock(&scheduler->queue_mutex);
-			BLI_assert(scheduler->do_exit);
 			return false;
 		}
+
 		for (current_task = scheduler->queue.first;
 		     current_task != NULL;
 		     current_task = current_task->next)