From 162ab66c2edec8cf7df0112c853668e987ef9652 Mon Sep 17 00:00:00 2001 From: Karol Herbst Date: Sat, 21 Jun 2025 18:31:18 +0200 Subject: [PATCH] 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 Part-of: --- src/gallium/frontends/rusticl/api/event.rs | 3 ++- src/gallium/frontends/rusticl/core/event.rs | 9 ++++++--- src/gallium/frontends/rusticl/core/queue.rs | 11 +++++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/gallium/frontends/rusticl/api/event.rs b/src/gallium/frontends/rusticl/api/event.rs index 808e554f2bd..72cacb131b4 100644 --- a/src/gallium/frontends/rusticl/api/event.rs +++ b/src/gallium/frontends/rusticl/api/event.rs @@ -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 for cl_event { @@ -26,7 +27,7 @@ unsafe impl CLInfo 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::from_ptr(ptr)) diff --git a/src/gallium/frontends/rusticl/core/event.rs b/src/gallium/frontends/rusticl/core/event.rs index 7aeef7359df..4df036b1490 100644 --- a/src/gallium/frontends/rusticl/core/event.rs +++ b/src/gallium/frontends/rusticl/core/event.rs @@ -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, pub context: Arc, - pub queue: Option>, + pub queue: Option>, pub cmd_type: cl_command_type, pub deps: Vec>, state: Mutex, @@ -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]) -> HashSet> { 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() } } diff --git a/src/gallium/frontends/rusticl/core/queue.rs b/src/gallium/frontends/rusticl/core/queue.rs index 398f29370af..d60c8423a8d 100644 --- a/src/gallium/frontends/rusticl/core/queue.rs +++ b/src/gallium/frontends/rusticl/core/queue.rs @@ -229,8 +229,11 @@ impl QueueEvent { unsafe { mem::transmute(self) } } - fn queue(&self) -> &Option> { - &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()