diff options
author | Bastien Montagne <montagne29@wanadoo.fr> | 2015-11-02 18:35:53 +0300 |
---|---|---|
committer | Bastien Montagne <montagne29@wanadoo.fr> | 2015-11-02 18:42:39 +0300 |
commit | 952afbf9168c8601e61fa64ffb37e6d93f1d3d61 (patch) | |
tree | 495bfc7421935230963d87f52992e30b023e069e /source | |
parent | ba5d6f5b6b3251a56b924f3b1c448068110a72f2 (diff) |
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.
Diffstat (limited to 'source')
-rw-r--r-- | source/blender/blenlib/intern/task.c | 15 |
1 files changed, 13 insertions, 2 deletions
diff --git a/source/blender/blenlib/intern/task.c b/source/blender/blenlib/intern/task.c index a125bf7d8d0..7ce230e97f3 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) |