fix(ext/webgpu): invalidate GPUAdapter when a device is created (#23752)

This removes the need for using `Deno.resources` to close the gpuadapter resource, while being more spec compliant.
denoland · May 10, 2024 · 6066e06 · 6066e06
1 parent 19c0633
commit 6066e06
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 23 deletions.
diff --git a/ext/webgpu/01_webgpu.js b/ext/webgpu/01_webgpu.js
@@ -417,9 417,12 @@ function createGPUAdapter(inner) {
  return adapter;
 }
 
 const _invalid = Symbol("[[invalid]]");
 class GPUAdapter {
  /** @type {InnerGPUAdapter} */
  [_adapter];
  /** @type {bool} */
  [_invalid];
 
  /** @returns {GPUSupportedFeatures} */
  get features() {
@@ -466,13 469,21 @@ class GPUAdapter {
  }
  }
 
  if (this[_invalid]) {
  throw new TypeError(
  "The adapter cannot be reused, as it has been invalidated by a device creation",
  );
  }
 
  const { rid, queueRid, features, limits } = op_webgpu_request_device(
  this[_adapter].rid,
  descriptor.label,
  requiredFeatures,
  descriptor.requiredLimits,
  );
 
  this[_invalid] = true;
 
  const inner = new InnerGPUDevice({
  rid,
  adapter: this,
@@ -496,6 507,12 @@ class GPUAdapter {
  requestAdapterInfo() {
  webidl.assertBranded(this, GPUAdapterPrototype);
 
  if (this[_invalid]) {
  throw new TypeError(
  "The adapter cannot be reused, as it has been invalidated by a device creation",
  );
  }
 
  const {
  vendor,
  architecture,

diff --git a/ext/webgpu/lib.rs b/ext/webgpu/lib.rs
@@ -673,7 673,7 @@ pub fn op_webgpu_request_device(
 ) -> Result<GpuDeviceRes, AnyError> {
  let mut state = state.borrow_mut();
  let adapter_resource =
- state.resource_table.get::<WebGpuAdapter>(adapter_rid)?;
  state.resource_table.take::<WebGpuAdapter>(adapter_rid)?;
  let adapter = adapter_resource.1;
  let instance = state.borrow::<Instance>();
 
@@ -690,6 690,7 @@ pub fn op_webgpu_request_device(
  None,
  None
  ));
  adapter_resource.close();
  if let Some(err) = maybe_err {
  return Err(DomExceptionOperationError::new(&err.to_string()).into());
  }
@@ -731,13 732,14 @@ pub fn op_webgpu_request_adapter_info(
  state: Rc<RefCell<OpState>>,
  #[smi] adapter_rid: ResourceId,
 ) -> Result<GPUAdapterInfo, AnyError> {
- let state = state.borrow_mut();
  let mut state = state.borrow_mut();
  let adapter_resource =
- state.resource_table.get::<WebGpuAdapter>(adapter_rid)?;
  state.resource_table.take::<WebGpuAdapter>(adapter_rid)?;
  let adapter = adapter_resource.1;
  let instance = state.borrow::<Instance>();
 
  let info = gfx_select!(adapter => instance.adapter_get_info(adapter))?;
  adapter_resource.close();
 
  Ok(GPUAdapterInfo {
  vendor: info.vendor.to_string(),

diff --git a/tests/unit/webgpu_test.ts b/tests/unit/webgpu_test.ts
@@ -100,11 100,6 @@ Deno.test({
  stagingBuffer.unmap();
 
  device.destroy();
-
- // TODO(lucacasonato): webgpu spec should add a explicit destroy method for
- // adapters.
- const resources = Object.keys(Deno.resources());
- Deno.close(Number(resources[resources.length - 1]));
 });
 
 Deno.test({
@@ -210,11 205,6 @@ Deno.test({
  outputBuffer.unmap();
 
  device.destroy();
-
- // TODO(lucacasonato): webgpu spec should add a explicit destroy method for
- // adapters.
- const resources = Object.keys(Deno.resources());
- Deno.close(Number(resources[resources.length - 1]));
 });
 
 Deno.test({
@@ -223,8 213,8 @@ Deno.test({
  const adapter = await navigator.gpu.requestAdapter();
  assert(adapter);
  assert(adapter.features);
- const resources = Object.keys(Deno.resources());
- Deno.close(Number(resources[resources.length - 1]));
  const device = await adapter.requestDevice();
  device.destroy();
 });
 
 Deno.test({
@@ -243,8 233,6 @@ Deno.test({
  );
 
  device.destroy();
- const resources = Object.keys(Deno.resources());
- Deno.close(Number(resources[resources.length - 1]));
 });
 
 Deno.test(function getPreferredCanvasFormat() {
@@ -313,8 301,6 @@ Deno.test({
  );
 
  device.destroy();
- const resources = Object.keys(Deno.resources());
- Deno.close(Number(resources[resources.length - 1]));
 });
 
 Deno.test({
@@ -409,8 395,6 @@ Deno.test({
  // NOTE: GPUQueue.copyExternalImageToTexture needs to be validated the argument of copySize property's length when its a sequence, but it is not implemented yet
 
  device.destroy();
- const resources = Object.keys(Deno.resources());
- Deno.close(Number(resources[resources.length - 1]));
 });
 
 Deno.test({
@@ -510,8 494,6 @@ Deno.test({
  // NOTE: GPUQueue.copyExternalImageToTexture needs to be validated the argument of destination.origin property's length when its a sequence, but it is not implemented yet
 
  device.destroy();
- const resources = Object.keys(Deno.resources());
- Deno.close(Number(resources[resources.length - 1]));
 });
 
 async function checkIsWsl() {