Skip to content

Commit a3d84be

Browse files
committed
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`. This is from an underlying `throw` in [GetQueue](https://github.com/oneapi-src/unified-runtime/blob/main/source/adapters/level_zero/queue.hpp#L890) when a `dynamic_cast` doesn't behave as expected. I initially tried changing this to an assert but the V2 Level Zero adapter is triggering this case while in it's developmental state, so an assert brings down the whole test suite rather than just being reported as a fail (the UR loader catches exceptions and returns them as error codes). Therefore this PR addresses the issue by moving the calls to `CleanupCompletedEvent()` outside of the command-buffer destructor. This allows us us to report an error from command-buffer release by doing cleanup prior to calling the destructor, without the risk of throwing an exception.
1 parent d2b086a commit a3d84be

File tree

2 files changed

+6
-5
lines changed

2 files changed

+6
-5
lines changed

source/adapters/level_zero/command_buffer.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,9 +424,7 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
424424
urDeviceRetain(Device);
425425
}
426426

427-
// The ur_exp_command_buffer_handle_t_ destructor releases all the memory
428-
// objects allocated for command_buffer management.
429-
ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() {
427+
void ur_exp_command_buffer_handle_t_::cleanupCommandBufferResources() {
430428
// Release the memory allocated to the Context stored in the command_buffer
431429
urContextRelease(Context);
432430

@@ -703,6 +701,7 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t CommandBuffer) {
703701
if (!CommandBuffer->RefCount.decrementAndTest())
704702
return UR_RESULT_SUCCESS;
705703

704+
CommandBuffer->cleanupCommandBufferResources();
706705
delete CommandBuffer;
707706
return UR_RESULT_SUCCESS;
708707
}

source/adapters/level_zero/command_buffer.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ struct ur_exp_command_buffer_handle_t_ : public _ur_object {
3434
ur_event_handle_t WaitEvent, ur_event_handle_t AllResetEvent,
3535
const ur_exp_command_buffer_desc_t *Desc, const bool IsInOrderCmdList);
3636

37-
~ur_exp_command_buffer_handle_t_();
38-
3937
void registerSyncPoint(ur_exp_command_buffer_sync_point_t SyncPoint,
4038
ur_event_handle_t Event);
4139

@@ -65,6 +63,10 @@ struct ur_exp_command_buffer_handle_t_ : public _ur_object {
6563
*/
6664
ze_command_list_handle_t chooseCommandList(bool PreferCopyEngine);
6765

66+
// Releases the resources associated with the command-buffer before the
67+
// command-buffer object is destroyed.
68+
void cleanupCommandBufferResources();
69+
6870
// UR context associated with this command-buffer
6971
ur_context_handle_t Context;
7072
// Device associated with this command buffer

0 commit comments

Comments
 (0)