[Mesa-dev] [PATCH 1/4] gallium/u_queue: fix random crashes when the app calls exit()

[Nicolai Hähnle]

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

On 21.02.2017 22:05, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> This fixes:
>     vdpauinfo: ../lib/CodeGen/TargetPassConfig.cpp:579: virtual void
>     llvm::TargetPassConfig::addMachinePasses(): Assertion `TPI && IPI &&
>     "Pass ID not registered!"' failed.
>
> v2: use list_head, switch the call order in destroy
>
> Cc: 13.0 17.0 <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/auxiliary/util/u_queue.c | 76 +++++++++++++++++++++++++++++++++++-
>  src/gallium/auxiliary/util/u_queue.h |  4 ++
>  2 files changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_queue.c b/src/gallium/auxiliary/util/u_queue.c
> index 4da5d8e..52cfc0a 100644
> --- a/src/gallium/auxiliary/util/u_queue.c
> +++ b/src/gallium/auxiliary/util/u_queue.c
> @@ -22,20 +22,82 @@
>   * The above copyright notice and this permission notice (including the
>   * next paragraph) shall be included in all copies or substantial portions
>   * of the Software.
>   */
>
>  #include "u_queue.h"
>  #include "u_memory.h"
>  #include "u_string.h"
>  #include "os/os_time.h"
>
> +static void util_queue_killall_and_wait(struct util_queue *queue);
> +
> +/****************************************************************************
> + * Wait for all queues to assert idle when exit() is called.
> + *
> + * Otherwise, C++ static variable destructors can be called while threads
> + * are using the static variables.
> + */
> +
> +static once_flag atexit_once_flag = ONCE_FLAG_INIT;
> +static struct list_head queue_list;
> +pipe_static_mutex(exit_mutex);
> +
> +static void
> +atexit_handler(void)
> +{
> +   struct util_queue *iter;
> +
> +   pipe_mutex_lock(exit_mutex);
> +   /* Wait for all queues to assert idle. */
> +   LIST_FOR_EACH_ENTRY(iter, &queue_list, head) {
> +      util_queue_killall_and_wait(iter);
> +   }
> +   pipe_mutex_unlock(exit_mutex);
> +}
> +
> +static void
> +global_init(void)
> +{
> +   LIST_INITHEAD(&queue_list);
> +   atexit(atexit_handler);
> +}
> +
> +static void
> +add_to_atexit_list(struct util_queue *queue)
> +{
> +   call_once(&atexit_once_flag, global_init);
> +
> +   pipe_mutex_lock(exit_mutex);
> +   LIST_ADD(&queue->head, &queue_list);
> +   pipe_mutex_unlock(exit_mutex);
> +}
> +
> +static void
> +remove_from_atexit_list(struct util_queue *queue)
> +{
> +   struct util_queue *iter, *tmp;
> +
> +   pipe_mutex_lock(exit_mutex);
> +   LIST_FOR_EACH_ENTRY_SAFE(iter, tmp, &queue_list, head) {
> +      if (iter == queue) {
> +         LIST_DEL(&iter->head);
> +         break;
> +      }
> +   }
> +   pipe_mutex_unlock(exit_mutex);
> +}
> +
> +/****************************************************************************
> + * util_queue implementation
> + */
> +
>  static void
>  util_queue_fence_signal(struct util_queue_fence *fence)
>  {
>     pipe_mutex_lock(fence->mutex);
>     fence->signalled = true;
>     pipe_condvar_broadcast(fence->cond);
>     pipe_mutex_unlock(fence->mutex);
>  }
>
>  void
> @@ -97,20 +159,21 @@ static PIPE_THREAD_ROUTINE(util_queue_thread_func, input)
>     }
>
>     /* signal remaining jobs before terminating */
>     pipe_mutex_lock(queue->lock);
>     while (queue->jobs[queue->read_idx].job) {
>        util_queue_fence_signal(queue->jobs[queue->read_idx].fence);
>
>        queue->jobs[queue->read_idx].job = NULL;
>        queue->read_idx = (queue->read_idx + 1) % queue->max_jobs;
>     }
> +   queue->num_queued = 0; /* reset this when exiting the thread */
>     pipe_mutex_unlock(queue->lock);
>     return 0;
>  }
>
>  bool
>  util_queue_init(struct util_queue *queue,
>                  const char *name,
>                  unsigned max_jobs,
>                  unsigned num_threads)
>  {
> @@ -150,49 +213,58 @@ util_queue_init(struct util_queue *queue,
>           if (i == 0) {
>              /* no threads created, fail */
>              goto fail;
>           } else {
>              /* at least one thread created, so use it */
>              queue->num_threads = i+1;
>              break;
>           }
>        }
>     }
> +
> +   add_to_atexit_list(queue);
>     return true;
>
>  fail:
>     FREE(queue->threads);
>
>     if (queue->jobs) {
>        pipe_condvar_destroy(queue->has_space_cond);
>        pipe_condvar_destroy(queue->has_queued_cond);
>        pipe_mutex_destroy(queue->lock);
>        FREE(queue->jobs);
>     }
>     /* also util_queue_is_initialized can be used to check for success */
>     memset(queue, 0, sizeof(*queue));
>     return false;
>  }
>
> -void
> -util_queue_destroy(struct util_queue *queue)
> +static void
> +util_queue_killall_and_wait(struct util_queue *queue)
>  {
>     unsigned i;
>
>     /* Signal all threads to terminate. */
>     pipe_mutex_lock(queue->lock);
>     queue->kill_threads = 1;
>     pipe_condvar_broadcast(queue->has_queued_cond);
>     pipe_mutex_unlock(queue->lock);
>
>     for (i = 0; i < queue->num_threads; i++)
>        pipe_thread_wait(queue->threads[i]);
> +}
> +
> +void
> +util_queue_destroy(struct util_queue *queue)
> +{
> +   util_queue_killall_and_wait(queue);
> +   remove_from_atexit_list(queue);
>
>     pipe_condvar_destroy(queue->has_space_cond);
>     pipe_condvar_destroy(queue->has_queued_cond);
>     pipe_mutex_destroy(queue->lock);
>     FREE(queue->jobs);
>     FREE(queue->threads);
>  }
>
>  void
>  util_queue_fence_init(struct util_queue_fence *fence)
> diff --git a/src/gallium/auxiliary/util/u_queue.h b/src/gallium/auxiliary/util/u_queue.h
> index 351315c..335813f 100644
> --- a/src/gallium/auxiliary/util/u_queue.h
> +++ b/src/gallium/auxiliary/util/u_queue.h
> @@ -27,20 +27,21 @@
>  /* Job queue with execution in a separate thread.
>   *
>   * Jobs can be added from any thread. After that, the wait call can be used
>   * to wait for completion of the job.
>   */
>
>  #ifndef U_QUEUE_H
>  #define U_QUEUE_H
>
>  #include "os/os_thread.h"
> +#include "util/list.h"
>
>  /* Job completion fence.
>   * Put this into your job structure.
>   */
>  struct util_queue_fence {
>     pipe_mutex mutex;
>     pipe_condvar cond;
>     int signalled;
>  };
>
> @@ -59,20 +60,23 @@ struct util_queue {
>     pipe_mutex lock;
>     pipe_condvar has_queued_cond;
>     pipe_condvar has_space_cond;
>     pipe_thread *threads;
>     int num_queued;
>     unsigned num_threads;
>     int kill_threads;
>     int max_jobs;
>     int write_idx, read_idx; /* ring buffer pointers */
>     struct util_queue_job *jobs;
> +
> +   /* for cleanup at exit(), protected by exit_mutex */
> +   struct list_head head;
>  };
>
>  bool util_queue_init(struct util_queue *queue,
>                       const char *name,
>                       unsigned max_jobs,
>                       unsigned num_threads);
>  void util_queue_destroy(struct util_queue *queue);
>  void util_queue_fence_init(struct util_queue_fence *fence);
>  void util_queue_fence_destroy(struct util_queue_fence *fence);
>
>