[L0 v2] Remove ur_shared_handle

as it's error prone to use - similarly to shared_ptr
we should not create multiple instances of shared_handle
from a single raw pointer.

Also, fix error in kernel.cpp: program handle was not
being retained, leading to segfaults.

Author: igchor

source/adapters/level_zero/v2/common.hpp

@@ -87,70 +87,6 @@ struct ze_handle_wrapper {
   bool ownZeHandle;
 };
 
-template <typename URHandle, ur_result_t (*retain)(URHandle),
-          ur_result_t (*release)(URHandle)>
-struct ur_shared_handle {
-  using handle_t = URHandle;
-
-  ur_shared_handle() : handle(nullptr) {}
-  explicit ur_shared_handle(handle_t handle) : handle(handle) {}
-  ~ur_shared_handle() {
-    try {
-      reset();
-    } catch (...) {
-    }
-  }
-
-  ur_shared_handle(const ur_shared_handle &other) : handle(other.handle) {
-    retain(handle);
-  }
-  ur_shared_handle(ur_shared_handle &&other) : handle(other.handle) {
-    other.handle = nullptr;
-  }
-  ur_shared_handle(std::nullptr_t) : handle(nullptr) {}
-
-  void reset() {
-    if (!handle) {
-      return;
-    }
-
-    UR_CALL_THROWS(release(handle));
-    handle = nullptr;
-  }
-
-  ur_shared_handle &operator=(const ur_shared_handle &other) {
-    if (handle) {
-      release(handle);
-    }
-    handle = other.handle;
-    retain(handle);
-    return *this;
-  }
-  ur_shared_handle &operator=(ur_shared_handle &&other) {
-    if (handle) {
-      release(handle);
-    }
-    handle = other.handle;
-    other.handle = nullptr;
-    return *this;
-  }
-  ur_shared_handle &operator=(std::nullptr_t) {
-    if (handle) {
-      release(handle);
-    }
-    new (this) ur_shared_handle(nullptr);
-    return *this;
-  }
-
-  handle_t *ptr() { return &handle; }
-  handle_t get() const { return handle; }
-  handle_t operator->() { return handle; }
-  operator handle_t() { return handle; }
-
-private:
-  handle_t handle;
-};
-
 using ze_kernel_handle_t =
     ze_handle_wrapper<::ze_kernel_handle_t, zeKernelDestroy>;
 
@@ -166,11 +102,5 @@ using ze_context_handle_t =
 using ze_command_list_handle_t =
     ze_handle_wrapper<::ze_command_list_handle_t, zeCommandListDestroy>;
 
-using ur_queue_shared_handle_t =
-    ur_shared_handle<ur_queue_handle_t, urQueueRetain, urQueueRelease>;
-
-using ur_kernel_shared_handle_t =
-    ur_shared_handle<ur_kernel_handle_t, urKernelRetain, urKernelRelease>;
-
 } // namespace raii
 } // namespace v2

source/adapters/level_zero/v2/kernel.cpp

@@ -32,10 +32,12 @@ ur_result_t ur_single_device_kernel_t::release() {
   return UR_RESULT_SUCCESS;
 }
 
-ur_kernel_handle_t_::ur_kernel_handle_t_(ur_program_shared_handle_t hProgram,
+ur_kernel_handle_t_::ur_kernel_handle_t_(ur_program_handle_t hProgram,
                                          const char *kernelName)
     : hProgram(hProgram),
       deviceKernels(hProgram->Context->getPlatform()->getNumDevices()) {
+  urProgramRetain(hProgram);
+
   for (auto [zeDevice, zeModule] : hProgram->ZeModuleMap) {
     ZeStruct<ze_kernel_desc_t> zeKernelDesc;
     zeKernelDesc.pKernelName = kernelName;
@@ -78,7 +80,8 @@ ur_result_t ur_kernel_handle_t_::release() {
       singleDeviceKernelOpt.value().hKernel.reset();
     }
   }
-  hProgram.reset();
+
+  UR_CALL_THROWS(urProgramRelease(hProgram));
 
   return UR_RESULT_SUCCESS;
 }
@@ -190,7 +193,7 @@ ur_result_t ur_kernel_handle_t_::setArgPointer(
 }
 
 ur_program_handle_t ur_kernel_handle_t_::getProgramHandle() const {
-  return hProgram.get();
+  return hProgram;
 }
 
 ur_result_t ur_kernel_handle_t_::setExecInfo(ur_kernel_exec_info_t propName,
@@ -238,8 +241,7 @@ ur_result_t ur_kernel_handle_t_::setExecInfo(ur_kernel_exec_info_t propName,
 UR_APIEXPORT ur_result_t UR_APICALL
 urKernelCreate(ur_program_handle_t hProgram, const char *pKernelName,
                ur_kernel_handle_t *phKernel) {
-  *phKernel = new ur_kernel_handle_t_(
-      ur_kernel_handle_t_::ur_program_shared_handle_t(hProgram), pKernelName);
+  *phKernel = new ur_kernel_handle_t_(hProgram, pKernelName);
   return UR_RESULT_SUCCESS;
 }
 

source/adapters/level_zero/v2/kernel.hpp

@@ -26,21 +26,8 @@ struct ur_single_device_kernel_t {
 
 struct ur_kernel_handle_t_ : _ur_object {
 private:
-  static inline ur_result_t
-  internalProgramRelease(ur_program_handle_t hProgram) {
-    // do a release on the program this kernel was part of without delete of the
-    // program handle.
-    hProgram->ur_release_program_resources(false);
-    return UR_RESULT_SUCCESS;
-  }
-
 public:
-  using ur_program_shared_handle_t =
-      v2::raii::ur_shared_handle<ur_program_handle_t, urProgramRetain,
-                                 internalProgramRelease>;
-
-  ur_kernel_handle_t_(ur_program_shared_handle_t hProgram,
-                      const char *kernelName);
+  ur_kernel_handle_t_(ur_program_handle_t hProgram, const char *kernelName);
 
   // From native handle
   ur_kernel_handle_t_(ur_native_handle_t hNativeKernel,
@@ -79,7 +66,7 @@ struct ur_kernel_handle_t_ : _ur_object {
 
 private:
   // Keep the program of the kernel.
-  ur_program_shared_handle_t hProgram;
+  ur_program_handle_t hProgram;
 
   // Vector of ur_single_device_kernel_t indexed by device->Id
   std::vector<std::optional<ur_single_device_kernel_t>> deviceKernels;

test/conformance/kernel/kernel_adapter_native_cpu.match

@@ -104,6 +104,7 @@ urKernelGetSubGroupInfoTest.InvalidEnumeration/SYCL_NATIVE_CPU___SYCL_Native_CPU
 urKernelGetSubGroupInfoTest.InvalidEnumeration/SYCL_NATIVE_CPU___SYCL_Native_CPU__{{.*}}__UR_KERNEL_SUB_GROUP_INFO_SUB_GROUP_SIZE_INTEL
 urKernelGetSubGroupInfoSingleTest.CompileNumSubgroupsIsZero/SYCL_NATIVE_CPU___SYCL_Native_CPU__{{.*}}
 urKernelReleaseTest.Success/SYCL_NATIVE_CPU___SYCL_Native_CPU__{{.*}}
+urKernelReleaseTest.KernelReleaseAfterProgramRelease/SYCL_NATIVE_CPU___SYCL_Native_CPU__{{.*}}
 urKernelReleaseTest.InvalidNullHandleKernel/SYCL_NATIVE_CPU___SYCL_Native_CPU__{{.*}}
 urKernelRetainTest.Success/SYCL_NATIVE_CPU___SYCL_Native_CPU__{{.*}}
 urKernelRetainTest.InvalidNullHandleKernel/SYCL_NATIVE_CPU___SYCL_Native_CPU__{{.*}}

test/conformance/kernel/urKernelRelease.cpp

@@ -13,6 +13,13 @@ TEST_P(urKernelReleaseTest, Success) {
     ASSERT_SUCCESS(urKernelRelease(kernel));
 }
 
+TEST_P(urKernelReleaseTest, KernelReleaseAfterProgramRelease) {
+    ASSERT_SUCCESS(urKernelRetain(kernel));
+    ASSERT_SUCCESS(urProgramRelease(program));
+    program = nullptr;
+    ASSERT_SUCCESS(urKernelRelease(kernel));
+}
+
 TEST_P(urKernelReleaseTest, InvalidNullHandleKernel) {
     ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_HANDLE,
                      urKernelRelease(nullptr));