Skip to content

Commit a140ef8

Browse files
committed
Retain queue when storing execution event in command buffer
Otherwise the event might be returned to the eventPool that has already been destroyed.
1 parent 2bcdd68 commit a140ef8

File tree

4 files changed

+75
-22
lines changed

4 files changed

+75
-22
lines changed

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

Lines changed: 46 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,45 @@
1515
#include "logger/ur_logger.hpp"
1616
#include "queue_handle.hpp"
1717

18+
ur_result_t ur_execution_event_handle_t::assign(ur_event_handle_t hNewEvent) {
19+
assert(hNewEvent);
20+
assert(hNewEvent->getURQueueHandle());
21+
22+
auto newQueue = hNewEvent->getURQueueHandle();
23+
auto currentQueue = hEvent ? hEvent->getURQueueHandle() : nullptr;
24+
25+
if (hEvent) {
26+
UR_CALL(hEvent->release());
27+
}
28+
29+
hEvent = hNewEvent;
30+
31+
if (newQueue != currentQueue) {
32+
if (currentQueue)
33+
UR_CALL(currentQueue->queueRelease());
34+
UR_CALL(newQueue->queueRetain());
35+
}
36+
37+
return UR_RESULT_SUCCESS;
38+
}
39+
40+
ur_event_handle_t ur_execution_event_handle_t::get() { return hEvent; }
41+
42+
ur_result_t ur_execution_event_handle_t::release() {
43+
if (hEvent) {
44+
assert(hEvent->getURQueueHandle());
45+
46+
auto hQueue = hEvent->getURQueueHandle();
47+
UR_CALL_NOCHECK(hEvent->release());
48+
UR_CALL_NOCHECK(hQueue->queueRelease());
49+
50+
hEvent = nullptr;
51+
}
52+
return UR_RESULT_SUCCESS;
53+
}
54+
55+
ur_execution_event_handle_t::~ur_execution_event_handle_t() { release(); }
56+
1857
namespace {
1958

2059
ur_result_t getZeKernelWrapped(ur_kernel_handle_t kernel,
@@ -69,13 +108,12 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
69108
: eventPool(context->getEventPoolCache(PoolCacheType::Regular)
70109
.borrow(device->Id.value(),
71110
isInOrder ? v2::EVENT_FLAGS_COUNTER : 0)),
72-
context(context), device(device),
111+
context(context), device(device), currentExecution(nullptr),
73112
isUpdatable(desc ? desc->isUpdatable : false),
74113
isInOrder(desc ? desc->isInOrder : false),
75114
commandListManager(
76115
context, device,
77-
std::forward<v2::raii::command_list_unique_handle>(commandList))
78-
{}
116+
std::forward<v2::raii::command_list_unique_handle>(commandList)) {}
79117

80118
ur_exp_command_buffer_sync_point_t
81119
ur_exp_command_buffer_handle_t_::getSyncPoint(ur_event_handle_t event) {
@@ -146,25 +184,16 @@ ur_result_t ur_exp_command_buffer_handle_t_::finalizeCommandBuffer() {
146184
return UR_RESULT_SUCCESS;
147185
}
148186
ur_event_handle_t ur_exp_command_buffer_handle_t_::getExecutionEventUnlocked() {
149-
return currentExecution;
187+
return currentExecution.get();
150188
}
151189

152190
ur_result_t ur_exp_command_buffer_handle_t_::registerExecutionEventUnlocked(
153191
ur_event_handle_t nextExecutionEvent) {
154-
if (currentExecution) {
155-
UR_CALL(currentExecution->release());
156-
currentExecution = nullptr;
157-
}
158-
if (nextExecutionEvent) {
159-
currentExecution = nextExecutionEvent;
160-
}
192+
UR_CALL(currentExecution.assign(nextExecutionEvent));
161193
return UR_RESULT_SUCCESS;
162194
}
163195

164196
ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() {
165-
if (currentExecution) {
166-
currentExecution->release();
167-
}
168197
for (auto &event : syncPoints) {
169198
event->release();
170199
}
@@ -181,14 +210,13 @@ ur_result_t ur_exp_command_buffer_handle_t_::applyUpdateCommands(
181210
this, device, context->getPlatform()->ZeDriverGlobalOffsetExtensionFound,
182211
numUpdateCommands, updateCommands));
183212

184-
if (currentExecution) {
213+
if (currentExecution.get()) {
185214
// TODO: Move synchronization to command buffer enqueue
186215
// it would require to remember the update commands and perform update
187216
// before appending to the queue
188217
ZE2UR_CALL(zeEventHostSynchronize,
189-
(currentExecution->getZeEvent(), UINT64_MAX));
190-
currentExecution->release();
191-
currentExecution = nullptr;
218+
(currentExecution.get()->getZeEvent(), UINT64_MAX));
219+
UR_CALL(currentExecution.release());
192220
}
193221

194222
device_ptr_storage_t zeHandles;

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,26 @@
1818
#include "queue_api.hpp"
1919
#include <unordered_set>
2020
#include <ze_api.h>
21+
2122
struct kernel_command_handle;
2223

24+
struct ur_execution_event_handle_t {
25+
ur_execution_event_handle_t(ur_event_handle_t event) : hEvent(event) {}
26+
27+
ur_execution_event_handle_t(const ur_execution_event_handle_t &) = delete;
28+
ur_execution_event_handle_t &
29+
operator=(const ur_execution_event_handle_t &) = delete;
30+
31+
ur_result_t assign(ur_event_handle_t hNewEvent);
32+
ur_event_handle_t get();
33+
ur_result_t release();
34+
35+
~ur_execution_event_handle_t();
36+
37+
private:
38+
ur_event_handle_t hEvent;
39+
};
40+
2341
struct ur_exp_command_buffer_handle_t_ : public ur_object {
2442
ur_exp_command_buffer_handle_t_(
2543
ur_context_handle_t context, ur_device_handle_t device,
@@ -72,7 +90,7 @@ struct ur_exp_command_buffer_handle_t_ : public ur_object {
7290
// Indicates if command-buffer was finalized.
7391
bool isFinalized = false;
7492

75-
ur_event_handle_t currentExecution = nullptr;
93+
ur_execution_event_handle_t currentExecution;
7694

7795
public:
7896
// Indicates if command-buffer commands can be updated after it is closed.

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,12 @@ ur_event_handle_t_::getEventEndTimestampAndHandle() {
192192

193193
ur_queue_t_ *ur_event_handle_t_::getQueue() const { return hQueue; }
194194

195+
ur_queue_handle_t ur_event_handle_t_::getURQueueHandle() const {
196+
auto urHandle = reinterpret_cast<uintptr_t>(getQueue()) -
197+
ur_queue_handle_t_::queue_offset;
198+
return reinterpret_cast<ur_queue_handle_t>(urHandle);
199+
}
200+
195201
ur_context_handle_t ur_event_handle_t_::getContext() const { return hContext; }
196202

197203
ur_command_t ur_event_handle_t_::getCommandType() const { return commandType; }
@@ -261,9 +267,7 @@ ur_result_t urEventGetInfo(ur_event_handle_t hEvent, ur_event_info_t propName,
261267
return returnValue(hEvent->RefCount.load());
262268
}
263269
case UR_EVENT_INFO_COMMAND_QUEUE: {
264-
auto urQueueHandle = reinterpret_cast<uintptr_t>(hEvent->getQueue()) -
265-
ur_queue_handle_t_::queue_offset;
266-
return returnValue(urQueueHandle);
270+
return returnValue(hEvent->getURQueueHandle());
267271
}
268272
case UR_EVENT_INFO_CONTEXT: {
269273
return returnValue(hEvent->getContext());

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ struct ur_event_handle_t_ : ur_object {
9191
// Queue associated with this event. Can be nullptr (for native events)
9292
ur_queue_t_ *getQueue() const;
9393

94+
// UR handle associated with this event's queue.
95+
ur_queue_handle_t getURQueueHandle() const;
96+
9497
// Context associated with this event
9598
ur_context_handle_t getContext() const;
9699

0 commit comments

Comments
 (0)