call the notify_cmdq_finish callback after event callback & release

... in pocl_update_event_finished. Also adds Queue
Retain/Release into clFinish() call.

This has the consequence that user thread (via clFinish) calls
clReleaseCmdQueue *after* the clReleaseEvent by the driver thread.
This ensures both event & queue are properly released
before clFinish returns control.

Previously clReleaseEvent was only called after the user thread (clFinish)
was notified, leaving the clReleaseEvent to be executed by the driver
potentially after main thread already exited. That was working most
of the time, but occasionally lead to crashes at program exit.

lib/CL/clFinish.c

@@ -38,8 +38,18 @@ POname(clFinish)(cl_command_queue command_queue) CL_API_SUFFIX__VERSION_1_0
   if (err != CL_SUCCESS)
     return err;
 
+  POCL_LOCK_OBJ (command_queue);
+  ++command_queue->notification_waiting_threads;
+  POCL_RETAIN_OBJECT_UNLOCKED (command_queue);
+  POCL_UNLOCK_OBJ (command_queue);
+
   command_queue->device->ops->join(command_queue->device, command_queue);
 
+  POCL_LOCK_OBJ (command_queue);
+  --command_queue->notification_waiting_threads;
+  POCL_UNLOCK_OBJ (command_queue);
+  POname (clReleaseCommandQueue) (command_queue);
+
   return CL_SUCCESS;
 }
 POsym(clFinish)

lib/CL/pocl_cl.h

@@ -1307,6 +1307,10 @@ struct _cl_command_queue {
   cl_queue_properties queue_properties[10];
   unsigned num_queue_properties;
 
+  /* number of user threads (via clFinish) awaiting
+   * cmd-queue-finished notification (via ops->notify_cmdq_finished) */
+  unsigned notification_waiting_threads;
+
   /* device specific data */
   void *data;
 

lib/CL/pocl_util.c

@@ -2455,6 +2455,7 @@ pocl_update_event_finished (cl_int status, const char *func, unsigned line,
   assert (event != NULL);
   assert (event->queue != NULL);
   assert (event->status > CL_COMPLETE);
+  int notify_cmdq = CL_FALSE;
 
   cl_command_queue cq = event->queue;
   POCL_LOCK_OBJ (cq);
@@ -2483,6 +2484,10 @@ pocl_update_event_finished (cl_int status, const char *func, unsigned line,
     cq->last_event.event = NULL;
   DL_DELETE (cq->events, event);
 
+  if (ops->notify_cmdq_finished && (cq->command_count == 0) && cq->notification_waiting_threads) {
+    notify_cmdq = CL_TRUE;
+  }
+
   POCL_UNLOCK_OBJ (cq);
   /* note that we must unlock the CmqQ before calling pocl_event_updated,
    * because it calls event callbacks, which can have calls to
@@ -2538,16 +2543,17 @@ pocl_update_event_finished (cl_int status, const char *func, unsigned line,
   pocl_free_event_node (event);
   pocl_free_event_memobjs (event);
 
-  POCL_LOCK_OBJ (cq);
-  if (ops->notify_cmdq_finished && (cq->command_count == 0))
-    ops->notify_cmdq_finished (cq);
-  POCL_UNLOCK_OBJ (cq);
   POCL_LOCK_OBJ (event);
   if (ops->notify_event_finished)
     ops->notify_event_finished (event);
   POCL_UNLOCK_OBJ (event);
-
   POname (clReleaseEvent) (event);
+
+  if (notify_cmdq) {
+    POCL_LOCK_OBJ (cq);
+    ops->notify_cmdq_finished (cq);
+    POCL_UNLOCK_OBJ (cq);
+  }
 }
 
 

tests/runtime/test_command_buffer_images.c

@@ -210,6 +210,7 @@ main (int _argc, char **_argv)
       }
     CHECK_CL_ERROR (clEnqueueUnmapMemObject (command_queue, buffer, buf_map, 0,
                                              NULL, NULL));
+    CHECK_CL_ERROR (clFinish (command_queue));
 
     CHECK_CL_ERROR (clReleaseEvent (write_src_event));
     CHECK_CL_ERROR (clReleaseEvent (command_buf_event));