Refactor L0 command-buffer cleanup.

Coverity has reported that there is the possibility of the
command-buffer destructor throwing an exception from its calls
to `CleanupCompletedEvent`. Where there is an underlying `throw`
when a `dynamic_cast` doesn't behave as expected.

This PR address this in two ways:

1. Change the `throw` error handling to an assert. Since uncaught
exceptions aren't a standard method of error reporting in the
handler change this to an assert. An assert was chosen rather than
providing a mechanism to return an error since it seems like this
case would arise due to programmer error rather than something a
user would do.

2. Move the calls to `CleanupCompletedEvent()` outside of the
destructor. This allows us us to report an error from command-buffer
release by doing cleanup prior to calling the destructor.

Author: Ewan Crawford

source/adapters/level_zero/command_buffer.cpp

@@ -449,6 +449,59 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
   urDeviceRetain(Device);
 }
 
+ur_result_t ur_exp_command_buffer_handle_t_::cleanupCommandBufferResources() {
+  ur_result_t RetResult = UR_RESULT_SUCCESS;
+  if (SignalEvent) {
+    ur_result_t Result = CleanupCompletedEvent(SignalEvent, false);
+    RetResult = (Result != UR_RESULT_SUCCESS) ? Result : RetResult;
+  }
+  if (WaitEvent) {
+    ur_result_t Result = CleanupCompletedEvent(WaitEvent, false);
+    RetResult = (Result != UR_RESULT_SUCCESS) ? Result : RetResult;
+  }
+  if (AllResetEvent) {
+    ur_result_t Result = CleanupCompletedEvent(AllResetEvent, false);
+    RetResult = (Result != UR_RESULT_SUCCESS) ? Result : RetResult;
+  }
+
+  // Release events added to the command_buffer
+  for (auto &Sync : SyncPoints) {
+    auto &Event = Sync.second;
+    ur_result_t Result = CleanupCompletedEvent(Event, false);
+    RetResult = (Result != UR_RESULT_SUCCESS) ? Result : RetResult;
+  }
+
+  auto ReleaseIndirectMem = [](ur_kernel_handle_t Kernel) {
+    if (IndirectAccessTrackingEnabled) {
+      // urKernelRelease is called by CleanupCompletedEvent(Event) as soon as
+      // kernel execution has finished. This is the place where we need to
+      // release memory allocations. If kernel is not in use (not submitted by
+      // some other thread) then release referenced memory allocations. As a
+      // result, memory can be deallocated and context can be removed from
+      // container in the platform. That's why we need to lock a mutex here.
+      ur_platform_handle_t Platform = Kernel->Program->Context->getPlatform();
+      std::scoped_lock<ur_shared_mutex> ContextsLock(Platform->ContextsMutex);
+
+      if (--Kernel->SubmissionsCount == 0) {
+        // Kernel is not submitted for execution, release referenced memory
+        // allocations.
+        for (auto &MemAlloc : Kernel->MemAllocs) {
+          // std::pair<void *const, MemAllocRecord> *, Hash
+          USMFreeHelper(MemAlloc->second.Context, MemAlloc->first,
+                        MemAlloc->second.OwnNativeHandle);
+        }
+        Kernel->MemAllocs.clear();
+      }
+    }
+  };
+
+  for (auto &AssociatedKernel : KernelsList) {
+    ReleaseIndirectMem(AssociatedKernel);
+  }
+
+  return RetResult;
+}
+
 // The ur_exp_command_buffer_handle_t_ destructor releases all the memory
 // objects allocated for command_buffer management.
 ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() {
@@ -475,22 +528,18 @@ ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() {
 
   // Release additional signal and wait events used by command_buffer
   if (SignalEvent) {
-    CleanupCompletedEvent(SignalEvent, false);
     urEventReleaseInternal(SignalEvent);
   }
   if (WaitEvent) {
-    CleanupCompletedEvent(WaitEvent, false);
     urEventReleaseInternal(WaitEvent);
   }
   if (AllResetEvent) {
-    CleanupCompletedEvent(AllResetEvent, false);
     urEventReleaseInternal(AllResetEvent);
   }
 
   // Release events added to the command_buffer
   for (auto &Sync : SyncPoints) {
     auto &Event = Sync.second;
-    CleanupCompletedEvent(Event, false);
     urEventReleaseInternal(Event);
   }
 
@@ -500,32 +549,7 @@ ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() {
     ZE_CALL_NOCHECK(zeFenceDestroy, (ZeFence));
   }
 
-  auto ReleaseIndirectMem = [](ur_kernel_handle_t Kernel) {
-    if (IndirectAccessTrackingEnabled) {
-      // urKernelRelease is called by CleanupCompletedEvent(Event) as soon as
-      // kernel execution has finished. This is the place where we need to
-      // release memory allocations. If kernel is not in use (not submitted by
-      // some other thread) then release referenced memory allocations. As a
-      // result, memory can be deallocated and context can be removed from
-      // container in the platform. That's why we need to lock a mutex here.
-      ur_platform_handle_t Platform = Kernel->Program->Context->getPlatform();
-      std::scoped_lock<ur_shared_mutex> ContextsLock(Platform->ContextsMutex);
-
-      if (--Kernel->SubmissionsCount == 0) {
-        // Kernel is not submitted for execution, release referenced memory
-        // allocations.
-        for (auto &MemAlloc : Kernel->MemAllocs) {
-          // std::pair<void *const, MemAllocRecord> *, Hash
-          USMFreeHelper(MemAlloc->second.Context, MemAlloc->first,
-                        MemAlloc->second.OwnNativeHandle);
-        }
-        Kernel->MemAllocs.clear();
-      }
-    }
-  };
-
   for (auto &AssociatedKernel : KernelsList) {
-    ReleaseIndirectMem(AssociatedKernel);
     urKernelRelease(AssociatedKernel);
   }
 }
@@ -727,8 +751,9 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t CommandBuffer) {
   if (!CommandBuffer->RefCount.decrementAndTest())
     return UR_RESULT_SUCCESS;
 
+  ur_result_t Result = CommandBuffer->cleanupCommandBufferResources();
   delete CommandBuffer;
-  return UR_RESULT_SUCCESS;
+  return Result;
 }
 
 UR_APIEXPORT ur_result_t UR_APICALL

source/adapters/level_zero/command_buffer.hpp

@@ -65,6 +65,13 @@ struct ur_exp_command_buffer_handle_t_ : public _ur_object {
    */
   ze_command_list_handle_t chooseCommandList(bool PreferCopyEngine);
 
+  /**
+   * Frees the resources associated with the command-buffer.
+   * @return UR_RESULT_SUCCESS or an error if a resources wasn't freed
+   * successfully.
+   */
+  ur_result_t cleanupCommandBufferResources();
+
   // UR context associated with this command-buffer
   ur_context_handle_t Context;
   // Device associated with this command buffer

source/adapters/level_zero/queue.hpp

@@ -886,9 +886,7 @@ template <typename QueueT> QueueT GetQueue(ur_queue_handle_t Queue) {
   if (!Queue)
     return nullptr;
   auto *Q = dynamic_cast<QueueT>(Queue);
-  if (!Q) {
-    throw UR_RESULT_ERROR_INVALID_QUEUE;
-  }
+  assert(Q && "Dynamic cast of queue handle failed");
   return Q;
 }