rusticl/event: convert queue to Weak reference

This works around applications leaking cl_event references, which
prevented queues to be destroyed and GPU context resources to be freed.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35672>
This commit is contained in:
Karol Herbst 2025-06-21 18:31:18 +02:00 committed by Marge Bot
parent 8291331c43
commit 162ab66c2e
3 changed files with 15 additions and 8 deletions

View file

@ -11,6 +11,7 @@ use rusticl_proc_macros::cl_info_entrypoint;
use std::ptr;
use std::sync::Arc;
use std::sync::Weak;
#[cl_info_entrypoint(clGetEventInfo)]
unsafe impl CLInfo<cl_event_info> for cl_event {
@ -26,7 +27,7 @@ unsafe impl CLInfo<cl_event_info> for cl_event {
CL_EVENT_COMMAND_QUEUE => {
let ptr = match event.queue.as_ref() {
// Note we use as_ptr here which doesn't increase the reference count.
Some(queue) => Arc::as_ptr(queue),
Some(queue) => Weak::as_ptr(queue),
None => ptr::null_mut(),
};
v.write::<cl_command_queue>(cl_command_queue::from_ptr(ptr))

View file

@ -15,6 +15,7 @@ use std::sync::Arc;
use std::sync::Condvar;
use std::sync::Mutex;
use std::sync::MutexGuard;
use std::sync::Weak;
use std::time::Duration;
// we assert that those are a continous range of numbers so we won't have to use HashMaps
@ -46,7 +47,7 @@ struct EventMutState {
pub struct Event {
pub base: CLObjectBase<CL_INVALID_EVENT>,
pub context: Arc<Context>,
pub queue: Option<Arc<Queue>>,
pub queue: Option<Weak<Queue>>,
pub cmd_type: cl_command_type,
pub deps: Vec<Arc<Event>>,
state: Mutex<EventMutState>,
@ -65,7 +66,7 @@ impl Event {
Arc::new(Self {
base: CLObjectBase::new(RusticlTypes::Event),
context: Arc::clone(&queue.context),
queue: Some(Arc::clone(queue)),
queue: Some(Arc::downgrade(queue)),
cmd_type: cmd_type,
deps: deps,
state: Mutex::new(EventMutState {
@ -290,7 +291,9 @@ impl Event {
pub fn deep_unflushed_queues(events: &[Arc<Event>]) -> HashSet<Arc<Queue>> {
Event::deep_unflushed_deps(events)
.iter()
.filter_map(|e| e.queue.clone())
.filter_map(|e| e.queue.as_ref())
// We don't have to do anything for destroyed queues as they already flush on drop.
.filter_map(Weak::upgrade)
.collect()
}
}

View file

@ -229,8 +229,11 @@ impl QueueEvent {
unsafe { mem::transmute(self) }
}
fn queue(&self) -> &Option<Arc<Queue>> {
&self.0.queue
fn has_same_queue_as(&self, ev: &Event) -> bool {
match (&self.0.queue, &ev.queue) {
(Some(a), Some(b)) => Weak::ptr_eq(a, b),
_ => false,
}
}
fn set_user_status(self, status: cl_int) {
@ -349,7 +352,7 @@ impl Queue {
for e in new_events {
// If we hit any deps from another queue, flush so we don't risk a dead
// lock.
if e.deps().iter().any(|ev| &ev.queue != e.queue()) {
if e.deps().iter().any(|ev| !e.has_same_queue_as(&ev)) {
let dep_err = flush_events(&mut flushed, &ctx);
last_err = cmp::min(last_err, dep_err);
}
@ -357,7 +360,7 @@ impl Queue {
// check if any dependency has an error
for dep in e.deps() {
// We have to wait on user events or events from other queues.
let dep_err = if dep.is_user() || &dep.queue != e.queue() {
let dep_err = if dep.is_user() || !e.has_same_queue_as(&dep) {
dep.wait()
} else {
dep.status()