Skip to content

Commit 1e2eede

Browse files
authored
[SYCL][UR][L0 v2] 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.
1 parent 1680dc6 commit 1e2eede

File tree

6 files changed

+36
-33
lines changed

6 files changed

+36
-33
lines changed

unified-runtime/source/adapters/level_zero/v2/command_buffer.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,17 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
6666
ur_context_handle_t context, ur_device_handle_t device,
6767
v2::raii::command_list_unique_handle &&commandList,
6868
const ur_exp_command_buffer_desc_t *desc)
69-
: isUpdatable(desc ? desc->isUpdatable : false),
69+
: eventPool(context->getEventPoolCache(PoolCacheType::Regular)
70+
.borrow(device->Id.value(),
71+
isInOrder ? v2::EVENT_FLAGS_COUNTER : 0)),
72+
context(context), device(device),
73+
isUpdatable(desc ? desc->isUpdatable : false),
7074
isInOrder(desc ? desc->isInOrder : false),
7175
commandListManager(
7276
context, device,
73-
std::forward<v2::raii::command_list_unique_handle>(commandList)),
74-
context(context), device(device),
75-
eventPool(context->getEventPoolCache(PoolCacheType::Regular)
76-
.borrow(device->Id.value(),
77-
isInOrder ? v2::EVENT_FLAGS_COUNTER : 0)) {}
77+
std::forward<v2::raii::command_list_unique_handle>(commandList)) {
78+
ur::level_zero::urContextRetain(context);
79+
}
7880

7981
ur_exp_command_buffer_sync_point_t
8082
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_() {
173175
for (auto &event : syncPoints) {
174176
event->release();
175177
}
178+
ur::level_zero::urContextRelease(context);
176179
}
177180

178181
ur_result_t ur_exp_command_buffer_handle_t_::applyUpdateCommands(

unified-runtime/source/adapters/level_zero/v2/command_buffer.hpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,6 @@ struct ur_exp_command_buffer_handle_t_ : public ur_object {
3232
ur_result_t
3333
registerExecutionEventUnlocked(ur_event_handle_t nextExecutionEvent);
3434

35-
// Indicates if command-buffer commands can be updated after it is closed.
36-
const bool isUpdatable = false;
37-
const bool isInOrder = true;
38-
39-
// Command-buffer profiling is enabled.
40-
const bool isProfilingEnabled = false;
41-
42-
lockable<ur_command_list_manager> commandListManager;
43-
4435
ur_result_t finalizeCommandBuffer();
4536

4637
ur_result_t
@@ -63,6 +54,8 @@ struct ur_exp_command_buffer_handle_t_ : public ur_object {
6354
createEventIfRequested(ur_exp_command_buffer_sync_point_t *retSyncPoint);
6455

6556
private:
57+
v2::raii::cache_borrowed_event_pool eventPool;
58+
6659
// Stores all sync points that are created by the command buffer.
6760
std::vector<ur_event_handle_t> syncPoints;
6861

@@ -84,5 +77,13 @@ struct ur_exp_command_buffer_handle_t_ : public ur_object {
8477

8578
ur_event_handle_t currentExecution = nullptr;
8679

87-
v2::raii::cache_borrowed_event_pool eventPool;
80+
public:
81+
// Indicates if command-buffer commands can be updated after it is closed.
82+
const bool isUpdatable = false;
83+
const bool isInOrder = true;
84+
85+
// Command-buffer profiling is enabled.
86+
const bool isProfilingEnabled = false;
87+
88+
lockable<ur_command_list_manager> commandListManager;
8889
};

unified-runtime/source/adapters/level_zero/v2/command_list_manager.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,7 @@ ur_command_list_manager::ur_command_list_manager(
2121
ur_context_handle_t context, ur_device_handle_t device,
2222
v2::raii::command_list_unique_handle &&commandList)
2323
: hContext(context), hDevice(device),
24-
zeCommandList(std::move(commandList)) {
25-
UR_CALL_THROWS(ur::level_zero::urContextRetain(context));
26-
UR_CALL_THROWS(ur::level_zero::urDeviceRetain(device));
27-
}
28-
29-
ur_command_list_manager::~ur_command_list_manager() {
30-
ur::level_zero::urContextRelease(hContext);
31-
ur::level_zero::urDeviceRelease(hDevice);
32-
}
24+
zeCommandList(std::move(commandList)) {}
3325

3426
ur_result_t ur_command_list_manager::appendGenericFillUnlocked(
3527
ur_mem_buffer_t *dst, size_t offset, size_t patternSize,

unified-runtime/source/adapters/level_zero/v2/command_list_manager.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ struct ur_command_list_manager {
4747
operator=(const ur_command_list_manager &src) = delete;
4848
ur_command_list_manager &operator=(ur_command_list_manager &&src) = default;
4949

50-
~ur_command_list_manager();
50+
~ur_command_list_manager() = default;
5151

5252
ze_command_list_handle_t getZeCommandList();
5353

unified-runtime/source/adapters/level_zero/v2/queue_immediate_in_order.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,29 @@ ur_queue_immediate_in_order_t::ur_queue_immediate_in_order_t(
2828
ze_command_queue_priority_t priority, std::optional<int32_t> index,
2929
event_flags_t eventFlags, ur_queue_flags_t flags)
3030
: hContext(hContext), hDevice(hDevice),
31+
eventPool(hContext->getEventPoolCache(PoolCacheType::Immediate)
32+
.borrow(hDevice->Id.value(), eventFlags)),
3133
commandListManager(
3234
hContext, hDevice,
3335
hContext->getCommandListCache().getImmediateCommandList(
3436
hDevice->ZeDevice,
3537
{true, ordinal, true /* always enable copy offload */},
3638
ZE_COMMAND_QUEUE_MODE_ASYNCHRONOUS, priority, index)),
37-
flags(flags),
38-
eventPool(hContext->getEventPoolCache(PoolCacheType::Immediate)
39-
.borrow(hDevice->Id.value(), eventFlags)) {}
39+
flags(flags) {
40+
ur::level_zero::urContextRetain(hContext);
41+
}
4042

4143
ur_queue_immediate_in_order_t::ur_queue_immediate_in_order_t(
4244
ur_context_handle_t hContext, ur_device_handle_t hDevice,
4345
raii::command_list_unique_handle commandListHandle,
4446
event_flags_t eventFlags, ur_queue_flags_t flags)
4547
: hContext(hContext), hDevice(hDevice),
46-
commandListManager(hContext, hDevice, std::move(commandListHandle)),
47-
flags(flags),
4848
eventPool(hContext->getEventPoolCache(PoolCacheType::Immediate)
49-
.borrow(hDevice->Id.value(), eventFlags)) {}
49+
.borrow(hDevice->Id.value(), eventFlags)),
50+
commandListManager(hContext, hDevice, std::move(commandListHandle)),
51+
flags(flags) {
52+
ur::level_zero::urContextRetain(hContext);
53+
}
5054

5155
ur_result_t
5256
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() {
122126
ur_queue_immediate_in_order_t::~ur_queue_immediate_in_order_t() {
123127
try {
124128
UR_CALL_THROWS(queueFinish());
129+
ur::level_zero::urContextRelease(hContext);
125130
} catch (...) {
126131
// Ignore errors during destruction
127132
}

unified-runtime/source/adapters/level_zero/v2/queue_immediate_in_order.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@ struct ur_queue_immediate_in_order_t : ur_object, ur_queue_t_ {
2929
private:
3030
ur_context_handle_t hContext;
3131
ur_device_handle_t hDevice;
32+
33+
v2::raii::cache_borrowed_event_pool eventPool;
34+
3235
lockable<ur_command_list_manager> commandListManager;
3336
ur_queue_flags_t flags;
34-
v2::raii::cache_borrowed_event_pool eventPool;
3537

3638
// Only create an event when requested by the user.
3739
ur_event_handle_t createEventIfRequested(ur_event_handle_t *phEvent) {

0 commit comments

Comments
 (0)