[Beignet] [PATCH V2] Fix the bug of multi-thread crash

[Zhigang Gong]

LGTM, pushed, thanks!

On Thu, Nov 13, 2014 at 05:40:46PM +0800, junyan.he at inbox.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
> 
> The cl_thread has a potential problem.
> If the threads are created and destroyed very fast,
> while the queue remain avaible, the resource of
> destroyed thread will not be free correctly and will
> be wrongly reused by later created thread.
> 
> V2:
>    Use a easy way to handle this case. We do not clear
>    the resource and just keep it. The later thread will
>    not wrongly reuse it. The thread number will not be
>    a very huge, so it is reasonable to clear all the
>    resource when the command queue is destroyed.
> 
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
>  src/cl_thread.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/src/cl_thread.c b/src/cl_thread.c
> index d4de1b3..0d99574 100644
> --- a/src/cl_thread.c
> +++ b/src/cl_thread.c
> @@ -37,7 +37,6 @@ static int thread_array_num = 1;
>  static int *thread_slot_map = NULL;
>  static int thread_magic_num = 1;
>  static pthread_mutex_t thread_queue_map_lock = PTHREAD_MUTEX_INITIALIZER;
> -static pthread_key_t destroy_key;
>  
>  static __thread int thread_id = -1;
>  static __thread int thread_magic = -1;
> @@ -55,13 +54,6 @@ typedef struct _queue_thread_private {
>    pthread_mutex_t thread_data_lock;
>  } queue_thread_private;
>  
> -static void thread_data_destructor(void *dummy) {
> -  pthread_mutex_lock(&thread_queue_map_lock);
> -  thread_slot_map[thread_id] = 0;
> -  pthread_mutex_unlock(&thread_queue_map_lock);
> -  free(dummy);
> -}
> -
>  static thread_spec_data * __create_thread_spec_data(cl_command_queue queue, int create)
>  {
>    queue_thread_private *thread_private = ((queue_thread_private *)(queue->thread_data));
> @@ -69,7 +61,6 @@ static thread_spec_data * __create_thread_spec_data(cl_command_queue queue, int
>    int i = 0;
>  
>    if (thread_id == -1) {
> -    void * dummy = malloc(sizeof(int));
>  
>      pthread_mutex_lock(&thread_queue_map_lock);
>      for (i = 0; i < thread_array_num; i++) {
> @@ -90,8 +81,6 @@ static thread_spec_data * __create_thread_spec_data(cl_command_queue queue, int
>  
>      thread_magic = thread_magic_num++;
>      pthread_mutex_unlock(&thread_queue_map_lock);
> -
> -    pthread_setspecific(destroy_key, dummy);
>    }
>  
>    pthread_mutex_lock(&thread_private->thread_data_lock);
> @@ -129,7 +118,6 @@ void* cl_thread_data_create(void)
>      thread_slot_map = calloc(thread_array_num, sizeof(int));
>      pthread_mutex_unlock(&thread_queue_map_lock);
>  
> -    pthread_key_create(&destroy_key, thread_data_destructor);
>    }
>  
>    pthread_mutex_init(&thread_private->thread_data_lock, NULL);
> @@ -238,7 +226,6 @@ void cl_thread_data_destroy(cl_command_queue queue)
>    thread_spec_data** threads_data;
>  
>    pthread_mutex_lock(&thread_private->thread_data_lock);
> -  assert(thread_private->threads_data_num == thread_array_num);
>    threads_data_num = thread_private->threads_data_num;
>    threads_data = thread_private->threads_data;
>    thread_private->threads_data_num = 0;
> -- 
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet