Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Merged
merged 4 commits into from
May 10, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
957d111
fix(ext/webgpu): invalidate GPUAdapter when a device is created
crowlKats May 8, 2024
b6faa79
fmt
crowlKats May 9, 2024
d421828
Merge branch 'refs/heads/main' into webgpu-disable-adapter-on-device-…
crowlKats May 10, 2024
c5bd14c
fix tests
crowlKats May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ext/webgpu/01_webgpu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions ext/webgpu/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>();

Expand All @@ -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());
}
Expand Down Expand Up @@ -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(),
Expand Down
22 changes: 2 additions & 20 deletions tests/unit/webgpu_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand All @@ -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({
Expand All @@ -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() {
Expand Down Expand Up @@ -313,8 +301,6 @@ Deno.test({
);

device.destroy();
const resources = Object.keys(Deno.resources());
Deno.close(Number(resources[resources.length - 1]));
});

Deno.test({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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() {
Expand Down
Loading