[deno] Rework error and device loss handling (#7693)

andyleiserson · web-flow · commit 26bab56fd5ad · 2025-05-17T19:38:22.000+02:00
diff --git a/deno_webgpu/adapter.rs b/deno_webgpu/adapter.rs
@@ -3,15 +3,14 @@
 #[allow(clippy::disallowed_types)]
 use std::collections::HashSet;
 use std::rc::Rc;
-use std::sync::Arc;
 
 use deno_core::cppgc::SameObject;
 use deno_core::op2;
 use deno_core::v8;
 use deno_core::GarbageCollected;
 use deno_core::OpState;
+use deno_core::V8TaskSpawner;
 use deno_core::WebIDL;
-use tokio::sync::Mutex;
 
 use super::device::GPUDevice;
 use crate::webidl::features_to_feature_names;
@@ -108,7 +107,6 @@ impl GPUAdapter {
     fn request_device(
         &self,
         state: &mut OpState,
-        isolate_ptr: *mut v8::Isolate,
         scope: &mut v8::HandleScope,
         #[webidl] descriptor: GPUDeviceDescriptor,
     ) -> Result<v8::Global<v8::Value>, CreateDeviceError> {
@@ -144,29 +142,27 @@ impl GPUAdapter {
             self.instance
                 .adapter_request_device(self.id, &wgpu_descriptor, None, None)?;
 
-        let (lost_sender, lost_receiver) = tokio::sync::oneshot::channel();
-        let (uncaptured_sender, mut uncaptured_receiver) = tokio::sync::mpsc::unbounded_channel();
-        let (uncaptured_sender_is_closed_sender, mut uncaptured_sender_is_closed_receiver) =
-            tokio::sync::oneshot::channel::<()>();
-
+        let spawner = state.borrow::<V8TaskSpawner>().clone();
+        let lost_resolver = v8::PromiseResolver::new(scope).unwrap();
+        let lost_promise = lost_resolver.get_promise(scope);
         let device = GPUDevice {
             instance: self.instance.clone(),
             id: device,
             queue,
             label: descriptor.label,
             queue_obj: SameObject::new(),
             adapter_info: self.info.clone(),
-            error_handler: Arc::new(super::error::DeviceErrorHandler::new(
-                lost_sender,
-                uncaptured_sender,
-                uncaptured_sender_is_closed_sender,
+            error_handler: Rc::new(super::error::DeviceErrorHandler::new(
+                v8::Global::new(scope, lost_resolver),
+                spawner,
             )),
             adapter: self.id,
-            lost_receiver: Mutex::new(Some(lost_receiver)),
+            lost_promise: v8::Global::new(scope, lost_promise),
             limits: SameObject::new(),
             features: SameObject::new(),
         };
         let device = deno_core::cppgc::make_cppgc_object(scope, device);
+        let weak_device = v8::Weak::new(scope, device);
         let event_target_setup = state.borrow::<crate::EventTargetSetup>();
         let webidl_brand = v8::Local::new(scope, event_target_setup.brand.clone());
         device.set(scope, webidl_brand, webidl_brand);
@@ -176,52 +172,15 @@ impl GPUAdapter {
         let null = v8::null(scope);
         set_event_target_data.call(scope, null.into(), &[device.into()]);
 
-        let key = v8::String::new(scope, "dispatchEvent").unwrap();
-        let val = device.get(scope, key.into()).unwrap();
-        let func = v8::Global::new(scope, val.try_cast::<v8::Function>().unwrap());
-        let device = v8::Global::new(scope, device.cast::<v8::Value>());
-        let error_event_class = state.borrow::<crate::ErrorEventClass>().0.clone();
-
-        let context = scope.get_current_context();
-        let context = v8::Global::new(scope, context);
-
-        let task_device = device.clone();
-        deno_unsync::spawn(async move {
-            loop {
-                // TODO(@crowlKats): check for uncaptured_receiver.is_closed instead once tokio is upgraded
-                if !matches!(
-                    uncaptured_sender_is_closed_receiver.try_recv(),
-                    Err(tokio::sync::oneshot::error::TryRecvError::Empty)
-                ) {
-                    break;
-                }
-                let Some(error) = uncaptured_receiver.recv().await else {
-                    break;
-                };
-
-                // SAFETY: eh, it's safe
-                let isolate: &mut v8::Isolate = unsafe { &mut *isolate_ptr };
-                let scope = &mut v8::HandleScope::with_context(isolate, &context);
-                let error = deno_core::error::to_v8_error(scope, &error);
-
-                let error_event_class = v8::Local::new(scope, error_event_class.clone());
-                let constructor = v8::Local::<v8::Function>::try_from(error_event_class).unwrap();
-                let kind = v8::String::new(scope, "uncapturederror").unwrap();
-
-                let obj = v8::Object::new(scope);
-                let key = v8::String::new(scope, "error").unwrap();
-                obj.set(scope, key.into(), error);
-
-                let event = constructor
-                    .new_instance(scope, &[kind.into(), obj.into()])
-                    .unwrap();
-
-                let recv = v8::Local::new(scope, task_device.clone());
-                func.open(scope).call(scope, recv, &[event.into()]);
-            }
-        });
+        // Now that the device is fully constructed, give the error handler a
+        // weak reference to it.
+        let device = device.cast::<v8::Value>();
+        deno_core::cppgc::try_unwrap_cppgc_object::<GPUDevice>(scope, device)
+            .unwrap()
+            .error_handler
+            .set_device(weak_device);
 
-        Ok(device)
+        Ok(v8::Global::new(scope, device))
     }
 }
 
diff --git a/deno_webgpu/device.rs b/deno_webgpu/device.rs
@@ -29,6 +29,7 @@ use crate::adapter::GPUAdapterInfo;
 use crate::adapter::GPUSupportedFeatures;
 use crate::adapter::GPUSupportedLimits;
 use crate::command_encoder::GPUCommandEncoder;
+use crate::error::GPUError;
 use crate::query_set::GPUQuerySet;
 use crate::render_bundle::GPURenderBundleEncoder;
 use crate::render_pipeline::GPURenderPipeline;
@@ -50,7 +51,7 @@ pub struct GPUDevice {
     pub queue_obj: SameObject<GPUQueue>,
 
     pub error_handler: super::error::ErrorHandler,
-    pub lost_receiver: tokio::sync::Mutex<Option<tokio::sync::oneshot::Receiver<()>>>,
+    pub lost_promise: v8::Global<v8::Promise>,
 }
 
 impl Drop for GPUDevice {
@@ -127,6 +128,8 @@ impl GPUDevice {
     #[fast]
     fn destroy(&self) {
         self.instance.device_destroy(self.id);
+        self.error_handler
+            .push_error(Some(GPUError::Lost(GPUDeviceLostReason::Destroyed)));
     }
 
     #[required(1)]
@@ -560,16 +563,10 @@ impl GPUDevice {
         }
     }
 
-    // TODO(@crowlKats): support returning same promise
-    #[async_method]
     #[getter]
-    #[cppgc]
-    async fn lost(&self) -> GPUDeviceLostInfo {
-        if let Some(lost_receiver) = self.lost_receiver.lock().await.take() {
-            let _ = lost_receiver.await;
-        }
-
-        GPUDeviceLostInfo
+    #[global]
+    fn lost(&self) -> v8::Global<v8::Promise> {
+        self.lost_promise.clone()
     }
 
     #[required(1)]
@@ -826,7 +823,17 @@ impl GPUDevice {
     }
 }
 
-pub struct GPUDeviceLostInfo;
+#[derive(Clone, Debug, Default, Hash, Eq, PartialEq)]
+pub enum GPUDeviceLostReason {
+    #[default]
+    Unknown,
+    Destroyed,
+}
+
+#[derive(Default)]
+pub struct GPUDeviceLostInfo {
+    pub reason: GPUDeviceLostReason,
+}
 
 impl GarbageCollected for GPUDeviceLostInfo {}
 
@@ -835,7 +842,11 @@ impl GPUDeviceLostInfo {
     #[getter]
     #[string]
     fn reason(&self) -> &'static str {
-        "unknown"
+        use GPUDeviceLostReason::*;
+        match self.reason {
+            Unknown => "unknown",
+            Destroyed => "destroyed",
+        }
     }
 
     #[getter]
diff --git a/deno_webgpu/error.rs b/deno_webgpu/error.rs
@@ -5,6 +5,11 @@ use std::fmt::Formatter;
 use std::sync::Mutex;
 use std::sync::OnceLock;
 
+use deno_core::cppgc::make_cppgc_object;
+use deno_core::v8;
+
+use deno_core::JsRuntime;
+use deno_core::V8TaskSpawner;
 use wgpu_core::binding_model::CreateBindGroupError;
 use wgpu_core::binding_model::CreateBindGroupLayoutError;
 use wgpu_core::binding_model::CreatePipelineLayoutError;
@@ -31,41 +36,38 @@ use wgpu_core::resource::CreateSamplerError;
 use wgpu_core::resource::CreateTextureError;
 use wgpu_core::resource::CreateTextureViewError;
 
-pub type ErrorHandler = std::sync::Arc<DeviceErrorHandler>;
+use crate::device::GPUDeviceLostInfo;
+use crate::device::GPUDeviceLostReason;
+
+pub type ErrorHandler = std::rc::Rc<DeviceErrorHandler>;
 
 pub struct DeviceErrorHandler {
     pub is_lost: OnceLock<()>,
-    lost_sender: Mutex<Option<tokio::sync::oneshot::Sender<()>>>,
-    uncaptured_sender_is_closed: Mutex<Option<tokio::sync::oneshot::Sender<()>>>,
-
-    pub uncaptured_sender: tokio::sync::mpsc::UnboundedSender<GPUError>,
-
     pub scopes: Mutex<Vec<(GPUErrorFilter, Vec<GPUError>)>>,
-}
+    lost_resolver: Mutex<Option<v8::Global<v8::PromiseResolver>>>,
+    spawner: V8TaskSpawner,
 
-impl Drop for DeviceErrorHandler {
-    fn drop(&mut self) {
-        if let Some(sender) = self.uncaptured_sender_is_closed.lock().unwrap().take() {
-            let _ = sender.send(());
-        }
-    }
+    // The error handler is constructed before the device. A weak
+    // reference to the device is placed here with `set_device`
+    // after the device is constructed.
+    device: OnceLock<v8::Weak<v8::Object>>,
 }
 
 impl DeviceErrorHandler {
-    pub fn new(
-        lost_sender: tokio::sync::oneshot::Sender<()>,
-        uncaptured_sender: tokio::sync::mpsc::UnboundedSender<GPUError>,
-        uncaptured_sender_is_closed: tokio::sync::oneshot::Sender<()>,
-    ) -> Self {
+    pub fn new(lost_resolver: v8::Global<v8::PromiseResolver>, spawner: V8TaskSpawner) -> Self {
         Self {
             is_lost: Default::default(),
-            lost_sender: Mutex::new(Some(lost_sender)),
-            uncaptured_sender,
-            uncaptured_sender_is_closed: Mutex::new(Some(uncaptured_sender_is_closed)),
             scopes: Mutex::new(vec![]),
+            lost_resolver: Mutex::new(Some(lost_resolver)),
+            device: OnceLock::new(),
+            spawner,
         }
     }
 
+    pub fn set_device(&self, device: v8::Weak<v8::Object>) {
+        self.device.set(device).unwrap()
+    }
+
     pub fn push_error<E: Into<GPUError>>(&self, err: Option<E>) {
         let Some(err) = err else {
             return;
@@ -77,17 +79,22 @@ impl DeviceErrorHandler {
 
         let err = err.into();
 
-        if matches!(err, GPUError::Lost) {
+        if let GPUError::Lost(reason) = err {
             let _ = self.is_lost.set(());
-
-            if let Some(sender) = self.lost_sender.lock().unwrap().take() {
-                let _ = sender.send(());
+            if let Some(resolver) = self.lost_resolver.lock().unwrap().take() {
+                self.spawner.spawn(move |scope| {
+                    let resolver = v8::Local::new(scope, resolver);
+                    let info = make_cppgc_object(scope, GPUDeviceLostInfo { reason });
+                    let info = v8::Local::new(scope, info);
+                    resolver.resolve(scope, info.into());
+                });
             }
+
             return;
         }
 
         let error_filter = match err {
-            GPUError::Lost => unreachable!(),
+            GPUError::Lost(_) => unreachable!(),
             GPUError::Validation(_) => GPUErrorFilter::Validation,
             GPUError::OutOfMemory => GPUErrorFilter::OutOfMemory,
             GPUError::Internal => GPUErrorFilter::Internal,
@@ -101,7 +108,41 @@ impl DeviceErrorHandler {
         if let Some(scope) = scope {
             scope.1.push(err);
         } else {
-            self.uncaptured_sender.send(err).unwrap();
+            let device = self
+                .device
+                .get()
+                .expect("set_device was not called")
+                .clone();
+            self.spawner.spawn(move |scope| {
+                let state = JsRuntime::op_state_from(&*scope);
+                let Some(device) = device.to_local(scope) else {
+                    // The device has already gone away, so we don't have
+                    // anywhere to report the error.
+                    return;
+                };
+                let key = v8::String::new(scope, "dispatchEvent").unwrap();
+                let val = device.get(scope, key.into()).unwrap();
+                let func = v8::Global::new(scope, val.try_cast::<v8::Function>().unwrap());
+                let device = v8::Global::new(scope, device.cast::<v8::Value>());
+                let error_event_class = state.borrow().borrow::<crate::ErrorEventClass>().0.clone();
+
+                let error = deno_core::error::to_v8_error(scope, &err);
+
+                let error_event_class = v8::Local::new(scope, error_event_class.clone());
+                let constructor = v8::Local::<v8::Function>::try_from(error_event_class).unwrap();
+                let kind = v8::String::new(scope, "uncapturederror").unwrap();
+
+                let obj = v8::Object::new(scope);
+                let key = v8::String::new(scope, "error").unwrap();
+                obj.set(scope, key.into(), error);
+
+                let event = constructor
+                    .new_instance(scope, &[kind.into(), obj.into()])
+                    .unwrap();
+
+                let recv = v8::Local::new(scope, device);
+                func.open(scope).call(scope, recv, &[event.into()]);
+            });
         }
     }
 }
@@ -118,7 +159,7 @@ pub enum GPUErrorFilter {
 pub enum GPUError {
     // TODO(@crowlKats): consider adding an unreachable value that uses unreachable!()
     #[class("UNREACHABLE")]
-    Lost,
+    Lost(GPUDeviceLostReason),
     #[class("GPUValidationError")]
     Validation(String),
     #[class("GPUOutOfMemoryError")]
@@ -131,7 +172,7 @@ pub enum GPUError {
 impl Display for GPUError {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         match self {
-            GPUError::Lost => Ok(()),
+            GPUError::Lost(_) => Ok(()),
             GPUError::Validation(s) => f.write_str(s),
             GPUError::OutOfMemory => f.write_str("not enough memory left"),
             GPUError::Internal => Ok(()),
@@ -170,7 +211,7 @@ impl From<CreateBufferError> for GPUError {
 impl From<DeviceError> for GPUError {
     fn from(err: DeviceError) -> Self {
         match err {
-            DeviceError::Lost => GPUError::Lost,
+            DeviceError::Lost => GPUError::Lost(GPUDeviceLostReason::Unknown),
             DeviceError::OutOfMemory => GPUError::OutOfMemory,
             _ => GPUError::Validation(fmt_err(&err)),
         }
