Skip to content

Commit 27b0151

Browse files
pbalcerbb-ur
authored andcommitted
fix event lifetime associated with async free (#21325)
A recent patch #21320, changed how events are allocated for async allocations. Instead of always allocating an event, we now allocate it optionally, depending on whether user specified it. But that changed did not update how event refcounts are updated, in the case that the event is actually allocated. This could lead to potential use-after-free. This patch moves the refcount management into command list manager, and correctly increments it when required.
1 parent 979a180 commit 27b0151

3 files changed

Lines changed: 24 additions & 7 deletions

File tree

source/adapters/level_zero/v2/command_list_manager.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,12 @@ ur_result_t ur_command_list_manager::appendUSMFreeExp(
966966
(getZeCommandList(), zeSignalEvent));
967967
}
968968

969+
// If event is specified, it's also going to be inserted
970+
// into the async pool, so it needs to be retained.
971+
if (phEvent) {
972+
phEvent->retain();
973+
}
974+
969975
// Insert must be done after the signal event is appended.
970976
usmPool->asyncPool.insert(pMem, size, phEvent, Queue);
971977

source/adapters/level_zero/v2/event_pool.hpp

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,31 @@ createEventIfRequested(event_pool *eventPool, ur_event_handle_t *phEvent,
7474
}
7575

7676
// Always creates an event (used in functions that need to store the event
77-
// internally). If event was requested by the user, also increase ref count of
78-
// that event to avoid pre-mature release.
79-
static inline ur_event_handle_t createEventAndRetain(event_pool *eventPool,
80-
ur_event_handle_t *phEvent,
81-
ur_queue_t_ *queue) {
77+
// internally).
78+
static inline ur_event_handle_t createEvent(event_pool *eventPool,
79+
ur_event_handle_t *phEvent,
80+
ur_queue_t_ *queue) {
8281
auto hEvent = eventPool->allocate();
8382
hEvent->setQueue(queue);
8483

8584
if (phEvent) {
8685
(*phEvent) = hEvent;
87-
hEvent->retain();
8886
}
8987

9088
return hEvent;
9189
}
9290

91+
// Always creates an event (used in functions that need to store the event
92+
// internally). If event was requested by the user, also increase ref count of
93+
// that event to avoid pre-mature release.
94+
static inline ur_event_handle_t createEventAndRetain(event_pool *eventPool,
95+
ur_event_handle_t *phEvent,
96+
ur_queue_t_ *queue) {
97+
auto *event = createEvent(eventPool, phEvent, queue);
98+
if (phEvent) {
99+
(*phEvent)->retain();
100+
}
101+
return event;
102+
}
103+
93104
} // namespace v2

source/adapters/level_zero/v2/queue_immediate_out_of_order.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ struct ur_queue_immediate_out_of_order_t : ur_object, ur_queue_t_ {
499499
auto commandListId = getNextCommandListId();
500500
return commandListManagers.lock()[commandListId].appendUSMFreeExp(
501501
this, pPool, pMem, waitListView,
502-
createEventAndRetain(eventPool.get(), phEvent, this));
502+
createEvent(eventPool.get(), phEvent, this));
503503
}
504504

505505
ur_result_t bindlessImagesImageCopyExp(

0 commit comments

Comments
 (0)