Merge pull request #1399 from GeorgeWeb/georgi/get-event-info-from-native-fix

[CUDA][HIP] Return error on silently failing urEventGetInfo queries for interop created events

Author: omarahmed1111

source/adapters/cuda/event.cpp

@@ -164,8 +164,18 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventGetInfo(ur_event_handle_t hEvent,
   UrReturnHelper ReturnValue(propValueSize, pPropValue, pPropValueSizeRet);
 
   switch (propName) {
-  case UR_EVENT_INFO_COMMAND_QUEUE:
+  case UR_EVENT_INFO_COMMAND_QUEUE: {
+    // If the runtime owns the native handle, we have reference to the queue.
+    // Otherwise, the event handle comes from an interop API with no RT refs.
+    if (!hEvent->getQueue()) {
+      setErrorMessage("Command queue info cannot be queried for the event. The "
+                      "event object was created from a native event and has no "
+                      "valid reference to a command queue.",
+                      UR_RESULT_ERROR_INVALID_VALUE);
+      return UR_RESULT_ERROR_ADAPTER_SPECIFIC;
+    }
     return ReturnValue(hEvent->getQueue());
+  }
   case UR_EVENT_INFO_COMMAND_TYPE:
     return ReturnValue(hEvent->getCommandType());
   case UR_EVENT_INFO_REFERENCE_COUNT:
@@ -280,17 +290,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventCreateWithNativeHandle(
 
   std::unique_ptr<ur_event_handle_t_> EventPtr{nullptr};
 
-  try {
-    EventPtr =
-        std::unique_ptr<ur_event_handle_t_>(ur_event_handle_t_::makeWithNative(
-            hContext, reinterpret_cast<CUevent>(hNativeEvent)));
-  } catch (const std::bad_alloc &) {
-    return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
-  } catch (...) {
-    return UR_RESULT_ERROR_UNKNOWN;
-  }
-
-  *phEvent = EventPtr.release();
+  *phEvent = ur_event_handle_t_::makeWithNative(
+      hContext, reinterpret_cast<CUevent>(hNativeEvent));
 
   return UR_RESULT_SUCCESS;
 }

source/adapters/hip/event.cpp

@@ -178,8 +178,18 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventGetInfo(ur_event_handle_t hEvent,
 
   UrReturnHelper ReturnValue(propValueSize, pPropValue, pPropValueSizeRet);
   switch (propName) {
-  case UR_EVENT_INFO_COMMAND_QUEUE:
+  case UR_EVENT_INFO_COMMAND_QUEUE: {
+    // If the runtime owns the native handle, we have reference to the queue.
+    // Otherwise, the event handle comes from an interop API with no RT refs.
+    if (!hEvent->getQueue()) {
+      setErrorMessage("Command queue info cannot be queried for the event. The "
+                      "event object was created from a native event and has no "
+                      "valid reference to a command queue.",
+                      UR_RESULT_ERROR_INVALID_VALUE);
+      return UR_RESULT_ERROR_ADAPTER_SPECIFIC;
+    }
     return ReturnValue(hEvent->getQueue());
+  }
   case UR_EVENT_INFO_COMMAND_TYPE:
     return ReturnValue(hEvent->getCommandType());
   case UR_EVENT_INFO_REFERENCE_COUNT:

test/adapters/cuda/CMakeLists.txt

@@ -16,6 +16,7 @@ add_adapter_test(cuda
         urQueueGetNativeHandle.cpp
         kernel_tests.cpp
         memory_tests.cpp
+        event_tests.cpp
         #FIXME: make this cleaner
         ${CMAKE_CURRENT_SOURCE_DIR}/../../../source/adapters/cuda/queue.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../../../source/adapters/cuda/common.cpp

test/adapters/cuda/event_tests.cpp

@@ -0,0 +1,36 @@
+// Copyright (C) 2022-2024 Intel Corporation
+// Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM Exceptions.
+// See LICENSE.TXT
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "device.hpp"
+#include "event.hpp"
+#include "fixtures.h"
+#include "raii.h"
+
+using cudaEventTest = uur::urContextTest;
+UUR_INSTANTIATE_DEVICE_TEST_SUITE_P(cudaEventTest);
+
+// Testing the urEventGetInfo behaviour for natively constructed (Cuda) events.
+// Backend interop APIs can lead to creating event objects that are not fully
+// initialized. In the Cuda adapter, an event can have nullptr command queue
+// because the interop API does not associate a UR-owned queue with the event.
+TEST_P(cudaEventTest, GetQueueFromEventCreatedWithNativeHandle) {
+    CUcontext cuda_ctx = device->getNativeContext();
+    EXPECT_NE(cuda_ctx, nullptr);
+    RAIICUevent cuda_event;
+    ASSERT_SUCCESS_CUDA(cuCtxSetCurrent(cuda_ctx));
+    ASSERT_SUCCESS_CUDA(cuEventCreate(cuda_event.ptr(), CU_EVENT_DEFAULT));
+
+    auto native_event = reinterpret_cast<ur_native_handle_t>(cuda_event.get());
+    uur::raii::Event event{nullptr};
+    ASSERT_SUCCESS(urEventCreateWithNativeHandle(native_event, context, nullptr,
+                                                 event.ptr()));
+    EXPECT_NE(event, nullptr);
+
+    size_t ret_size{};
+    ur_queue_handle_t q{};
+    ASSERT_EQ_RESULT(urEventGetInfo(event, UR_EVENT_INFO_COMMAND_QUEUE,
+                                    sizeof(ur_queue_handle_t), &q, &ret_size),
+                     UR_RESULT_ERROR_ADAPTER_SPECIFIC);
+}

test/adapters/cuda/raii.h

@@ -0,0 +1,25 @@
+// Copyright (C) 2022-2024 Intel Corporation
+// Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM Exceptions.
+// See LICENSE.TXT
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef UR_TEST_CONFORMANCE_ADAPTERS_CUDA_RAII_H_INCLUDED
+#define UR_TEST_CONFORMANCE_ADAPTERS_CUDA_RAII_H_INCLUDED
+
+#include "uur/raii.h"
+#include <cuda.h>
+
+struct RAIICUevent {
+    CUevent handle = nullptr;
+
+    ~RAIICUevent() {
+        if (handle) {
+            cuEventDestroy(handle);
+        }
+    }
+
+    CUevent *ptr() { return &handle; }
+    CUevent get() { return handle; }
+};
+
+#endif // UR_TEST_CONFORMANCE_ADAPTERS_CUDA_RAII_H_INCLUDED

test/adapters/cuda/urEventCreateWithNativeHandle.cpp

@@ -4,24 +4,11 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 #include "fixtures.h"
-#include "uur/raii.h"
+#include "raii.h"
 
 using urCudaEventCreateWithNativeHandleTest = uur::urQueueTest;
 UUR_INSTANTIATE_DEVICE_TEST_SUITE_P(urCudaEventCreateWithNativeHandleTest);
 
-struct RAIICUevent {
-    CUevent handle = nullptr;
-
-    ~RAIICUevent() {
-        if (handle) {
-            cuEventDestroy(handle);
-        }
-    }
-
-    CUevent *ptr() { return &handle; }
-    CUevent get() { return handle; }
-};
-
 TEST_P(urCudaEventCreateWithNativeHandleTest, Success) {
     RAIICUevent cuda_event;
     ASSERT_SUCCESS_CUDA(cuEventCreate(cuda_event.ptr(), CU_EVENT_DEFAULT));

test/adapters/hip/CMakeLists.txt

@@ -11,6 +11,7 @@ add_adapter_test(hip
         urDeviceGetNativeHandle.cpp
         urEventGetNativeHandle.cpp
         test_context.cpp
+        test_event.cpp
     ENVIRONMENT
         "UR_ADAPTERS_FORCE_LOAD=\"$<TARGET_FILE:ur_adapter_hip>\""
 )

test/adapters/hip/test_event.cpp

@@ -0,0 +1,49 @@
+// Copyright (C) 2022-2024 Intel Corporation
+// Part of the Unified-Runtime Project, under the Apache License v2.0 with LLVM Exceptions.
+// See LICENSE.TXT
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "event.hpp"
+#include "fixtures.h"
+#include "uur/raii.h"
+
+#include <hip/hip_runtime.h>
+#include <tuple>
+
+struct RAIIHipEvent {
+    hipEvent_t handle = nullptr;
+
+    ~RAIIHipEvent() {
+        if (handle) {
+            std::ignore = hipEventDestroy(handle);
+        }
+    }
+
+    hipEvent_t *ptr() { return &handle; }
+    hipEvent_t get() { return handle; }
+};
+
+using urHipEventTest = uur::urContextTest;
+UUR_INSTANTIATE_DEVICE_TEST_SUITE_P(urHipEventTest);
+
+// Testing the urEventGetInfo behaviour for natively constructed (HIP) events.
+// Backend interop APIs can lead to creating event objects that are not fully
+// initialized. In the Cuda adapter, an event can have nullptr command queue
+// because the interop API does not associate a UR-owned queue with the event.
+TEST_P(urHipEventTest, GetQueueFromEventCreatedWithNativeHandle) {
+    RAIIHipEvent hip_event;
+    ASSERT_SUCCESS_HIP(
+        hipEventCreateWithFlags(hip_event.ptr(), hipEventDefault));
+
+    auto native_event = reinterpret_cast<ur_native_handle_t>(hip_event.get());
+    uur::raii::Event event{nullptr};
+    ASSERT_SUCCESS(urEventCreateWithNativeHandle(native_event, context, nullptr,
+                                                 event.ptr()));
+    EXPECT_NE(event, nullptr);
+
+    size_t ret_size{};
+    ur_queue_handle_t q{};
+    ASSERT_EQ_RESULT(urEventGetInfo(event, UR_EVENT_INFO_COMMAND_QUEUE,
+                                    sizeof(ur_queue_handle_t), &q, &ret_size),
+                     UR_RESULT_ERROR_ADAPTER_SPECIFIC);
+}