Fix issues with event lifetime (#18962)

When releasing executionEvent in the commandBuffer we need to also
retain the queue. Otherwise event->release() in commandBuffer's
destructor might attempt to release the event to an event pool that has
already been destroyed.

Also, make sure eventPool is destroyed after commandListManager to avoid
any issues with events owned by the commandListManager and move context
retain/release from commandListManager to queue/command_buffer.

The implementation of commandListManager marked move ctor and assignment
operator as defaulted which was wrong - the implementation would have to
account for context and device retain/release calls. To avoid this, move
the logic to queue/commandBuffer which can have move ctors/assignments
operators removed.

Author: igchor

source/adapters/level_zero/v2/command_buffer.cpp

@@ -66,15 +66,17 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
     ur_context_handle_t context, ur_device_handle_t device,
     v2::raii::command_list_unique_handle &&commandList,
     const ur_exp_command_buffer_desc_t *desc)
-    : isUpdatable(desc ? desc->isUpdatable : false),
+    : eventPool(context->getEventPoolCache(PoolCacheType::Regular)
+                    .borrow(device->Id.value(),
+                            isInOrder ? v2::EVENT_FLAGS_COUNTER : 0)),
+      context(context), device(device),
+      isUpdatable(desc ? desc->isUpdatable : false),
       isInOrder(desc ? desc->isInOrder : false),
       commandListManager(
           context, device,
-          std::forward<v2::raii::command_list_unique_handle>(commandList)),
-      context(context), device(device),
-      eventPool(context->getEventPoolCache(PoolCacheType::Regular)
-                    .borrow(device->Id.value(),
-                            isInOrder ? v2::EVENT_FLAGS_COUNTER : 0)) {}
+          std::forward<v2::raii::command_list_unique_handle>(commandList)) {
+  ur::level_zero::urContextRetain(context);
+}
 
 ur_exp_command_buffer_sync_point_t
 ur_exp_command_buffer_handle_t_::getSyncPoint(ur_event_handle_t event) {
@@ -173,6 +175,7 @@ ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() {
   for (auto &event : syncPoints) {
     event->release();
   }
+  ur::level_zero::urContextRelease(context);
 }
 
 ur_result_t ur_exp_command_buffer_handle_t_::applyUpdateCommands(

source/adapters/level_zero/v2/command_buffer.hpp

@@ -32,15 +32,6 @@ struct ur_exp_command_buffer_handle_t_ : public ur_object {
   ur_result_t
   registerExecutionEventUnlocked(ur_event_handle_t nextExecutionEvent);
 
-  // Indicates if command-buffer commands can be updated after it is closed.
-  const bool isUpdatable = false;
-  const bool isInOrder = true;
-
-  // Command-buffer profiling is enabled.
-  const bool isProfilingEnabled = false;
-
-  lockable<ur_command_list_manager> commandListManager;
-
   ur_result_t finalizeCommandBuffer();
 
   ur_result_t
@@ -63,6 +54,8 @@ struct ur_exp_command_buffer_handle_t_ : public ur_object {
   createEventIfRequested(ur_exp_command_buffer_sync_point_t *retSyncPoint);
 
 private:
+  v2::raii::cache_borrowed_event_pool eventPool;
+
   // Stores all sync points that are created by the command buffer.
   std::vector<ur_event_handle_t> syncPoints;
 
@@ -84,5 +77,13 @@ struct ur_exp_command_buffer_handle_t_ : public ur_object {
 
   ur_event_handle_t currentExecution = nullptr;
 
-  v2::raii::cache_borrowed_event_pool eventPool;
+public:
+  // Indicates if command-buffer commands can be updated after it is closed.
+  const bool isUpdatable = false;
+  const bool isInOrder = true;
+
+  // Command-buffer profiling is enabled.
+  const bool isProfilingEnabled = false;
+
+  lockable<ur_command_list_manager> commandListManager;
 };

source/adapters/level_zero/v2/command_list_manager.cpp

@@ -21,15 +21,7 @@ ur_command_list_manager::ur_command_list_manager(
     ur_context_handle_t context, ur_device_handle_t device,
     v2::raii::command_list_unique_handle &&commandList)
     : hContext(context), hDevice(device),
-      zeCommandList(std::move(commandList)) {
-  UR_CALL_THROWS(ur::level_zero::urContextRetain(context));
-  UR_CALL_THROWS(ur::level_zero::urDeviceRetain(device));
-}
-
-ur_command_list_manager::~ur_command_list_manager() {
-  ur::level_zero::urContextRelease(hContext);
-  ur::level_zero::urDeviceRelease(hDevice);
-}
+      zeCommandList(std::move(commandList)) {}
 
 ur_result_t ur_command_list_manager::appendGenericFillUnlocked(
     ur_mem_buffer_t *dst, size_t offset, size_t patternSize,

source/adapters/level_zero/v2/command_list_manager.hpp

@@ -47,7 +47,7 @@ struct ur_command_list_manager {
   operator=(const ur_command_list_manager &src) = delete;
   ur_command_list_manager &operator=(ur_command_list_manager &&src) = default;
 
-  ~ur_command_list_manager();
+  ~ur_command_list_manager() = default;
 
   ze_command_list_handle_t getZeCommandList();
 

source/adapters/level_zero/v2/queue_immediate_in_order.cpp

@@ -28,25 +28,29 @@ ur_queue_immediate_in_order_t::ur_queue_immediate_in_order_t(
     ze_command_queue_priority_t priority, std::optional<int32_t> index,
     event_flags_t eventFlags, ur_queue_flags_t flags)
     : hContext(hContext), hDevice(hDevice),
+      eventPool(hContext->getEventPoolCache(PoolCacheType::Immediate)
+                    .borrow(hDevice->Id.value(), eventFlags)),
       commandListManager(
           hContext, hDevice,
           hContext->getCommandListCache().getImmediateCommandList(
               hDevice->ZeDevice,
               {true, ordinal, true /* always enable copy offload */},
               ZE_COMMAND_QUEUE_MODE_ASYNCHRONOUS, priority, index)),
-      flags(flags),
-      eventPool(hContext->getEventPoolCache(PoolCacheType::Immediate)
-                    .borrow(hDevice->Id.value(), eventFlags)) {}
+      flags(flags) {
+  ur::level_zero::urContextRetain(hContext);
+}
 
 ur_queue_immediate_in_order_t::ur_queue_immediate_in_order_t(
     ur_context_handle_t hContext, ur_device_handle_t hDevice,
     raii::command_list_unique_handle commandListHandle,
     event_flags_t eventFlags, ur_queue_flags_t flags)
     : hContext(hContext), hDevice(hDevice),
-      commandListManager(hContext, hDevice, std::move(commandListHandle)),
-      flags(flags),
       eventPool(hContext->getEventPoolCache(PoolCacheType::Immediate)
-                    .borrow(hDevice->Id.value(), eventFlags)) {}
+                    .borrow(hDevice->Id.value(), eventFlags)),
+      commandListManager(hContext, hDevice, std::move(commandListHandle)),
+      flags(flags) {
+  ur::level_zero::urContextRetain(hContext);
+}
 
 ur_result_t
 ur_queue_immediate_in_order_t::queueGetInfo(ur_queue_info_t propName,
@@ -122,6 +126,7 @@ ur_result_t ur_queue_immediate_in_order_t::queueFlush() {
 ur_queue_immediate_in_order_t::~ur_queue_immediate_in_order_t() {
   try {
     UR_CALL_THROWS(queueFinish());
+    ur::level_zero::urContextRelease(hContext);
   } catch (...) {
     // Ignore errors during destruction
   }

source/adapters/level_zero/v2/queue_immediate_in_order.hpp

@@ -29,9 +29,11 @@ struct ur_queue_immediate_in_order_t : ur_object, ur_queue_t_ {
 private:
   ur_context_handle_t hContext;
   ur_device_handle_t hDevice;
+
+  v2::raii::cache_borrowed_event_pool eventPool;
+
   lockable<ur_command_list_manager> commandListManager;
   ur_queue_flags_t flags;
-  v2::raii::cache_borrowed_event_pool eventPool;
 
   // Only create an event when requested by the user.
   ur_event_handle_t createEventIfRequested(ur_event_handle_t *phEvent) {