Make double finalizing fail

A command buffer should fail if finalize is called multiple times on the same command buffer. This matches openCL behaviour.
oneapi-src · Nov 14, 2024 · f685bb3 · f685bb3
1 parent 50ef8c9
commit f685bb3
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 0 deletions.
diff --git a/source/adapters/cuda/command_buffer.cpp b/source/adapters/cuda/command_buffer.cpp
@@ -401,6 +401,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
 
 UR_APIEXPORT ur_result_t UR_APICALL
 urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
+  UR_ASSERT(!hCommandBuffer->IsFinalized,
+            UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP);
   try {
     const unsigned long long flags = 0;
 #if CUDA_VERSION >= 12000
@@ -418,6 +420,7 @@ urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
   } catch (...) {
     return UR_RESULT_ERROR_UNKNOWN;
   }
+  hCommandBuffer->IsFinalized = true;
   return UR_RESULT_SUCCESS;
 }
 

diff --git a/source/adapters/cuda/command_buffer.hpp b/source/adapters/cuda/command_buffer.hpp
@@ -352,6 +352,8 @@ struct ur_exp_command_buffer_handle_t_ {
   ur_device_handle_t Device;
   // Whether commands in the command-buffer can be updated
   bool IsUpdatable;
+  // Keep track of whether command buffer is finalized
+  bool IsFinalized = false;
   // Cuda Graph handle
   CUgraph CudaGraph;
   // Cuda Graph Exec handle

diff --git a/source/adapters/hip/command_buffer.cpp b/source/adapters/hip/command_buffer.cpp
@@ -306,13 +306,16 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
 
 UR_APIEXPORT ur_result_t UR_APICALL
 urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
+  UR_ASSERT(!hCommandBuffer->IsFinalized,
+            UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP);
   try {
     const unsigned long long flags = 0;
     UR_CHECK_ERROR(hipGraphInstantiateWithFlags(
         &hCommandBuffer->HIPGraphExec, hCommandBuffer->HIPGraph, flags));
   } catch (...) {
     return UR_RESULT_ERROR_UNKNOWN;
   }
+  hCommandBuffer->IsFinalized = true;
   return UR_RESULT_SUCCESS;
 }
 

diff --git a/source/adapters/hip/command_buffer.hpp b/source/adapters/hip/command_buffer.hpp
@@ -172,6 +172,8 @@ struct ur_exp_command_buffer_handle_t_ {
   ur_device_handle_t Device;
   // Whether commands in the command-buffer can be updated
   bool IsUpdatable;
+  // Keep track of whether command buffer is finalized
+  bool IsFinalized = false;
   // HIP Graph handle
   hipGraph_t HIPGraph;
   // HIP Graph Exec handle

diff --git a/source/adapters/level_zero/command_buffer.cpp b/source/adapters/level_zero/command_buffer.cpp
@@ -595,6 +595,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t CommandBuffer) {
 ur_result_t
 urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t CommandBuffer) {
   UR_ASSERT(CommandBuffer, UR_RESULT_ERROR_INVALID_NULL_POINTER);
+  UR_ASSERT(!CommandBuffer->IsFinalized,
+            UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP);
   // It is not allowed to append to command list from multiple threads.
   std::scoped_lock<ur_shared_mutex> Guard(CommandBuffer->Mutex);
 

diff --git a/source/adapters/opencl/command_buffer.cpp b/source/adapters/opencl/command_buffer.cpp
@@ -124,6 +124,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
 
 UR_APIEXPORT ur_result_t UR_APICALL
 urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
+  UR_ASSERT(!hCommandBuffer->IsFinalized,
+            UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP);
   cl_context CLContext = cl_adapter::cast<cl_context>(hCommandBuffer->hContext);
   cl_ext::clFinalizeCommandBufferKHR_fn clFinalizeCommandBufferKHR = nullptr;
   UR_RETURN_ON_FAILURE(

diff --git a/test/conformance/exp_command_buffer/commands.cpp b/test/conformance/exp_command_buffer/commands.cpp
@@ -204,3 +204,15 @@ TEST_P(urCommandBufferAppendKernelLaunchExpTest, Basic) {
         ASSERT_EQ(result, ptrZ[i]);
     }
 }
+
+TEST_P(urCommandBufferAppendKernelLaunchExpTest, FinalizeTwice) {
+    ASSERT_SUCCESS(urCommandBufferAppendKernelLaunchExp(
+        cmd_buf_handle, kernel, n_dimensions, &global_offset, &global_size,
+        &local_size, 0, nullptr, 0, nullptr, 0, nullptr, nullptr, nullptr,
+        nullptr));
+
+    ASSERT_SUCCESS(urCommandBufferFinalizeExp(cmd_buf_handle));
+    EXPECT_EQ_RESULT(urCommandBufferFinalizeExp(cmd_buf_handle),
+                     UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP);
+
+}