Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

WebGPU: Attempting to use mapped buffer with GPU panics #10098

Closed
asos-craigmorten opened this issue Apr 9, 2021 · 2 comments · Fixed by #17534
Closed

WebGPU: Attempting to use mapped buffer with GPU panics #10098

asos-craigmorten opened this issue Apr 9, 2021 · 2 comments · Fixed by #17534
Labels
bug Something isn't working correctly webgpu WebGPU API

Comments

@asos-craigmorten
Copy link
Contributor

asos-craigmorten commented Apr 9, 2021

Some meta:

  • MacBook Pro (16-inch, 2019) macOS Big Sur
  • Intel UHD Graphics 630 1536 MB
$ deno --version
deno 1.8.3 (release, x86_64-apple-darwin)
v8 9.0.257.3
typescript 4.2.2

Seeing the following panic:

thread 'main' panicked at 'Buffer Valid((3, 1, Metal)) is still mapped', /Users/runner/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/wgpu-core-0.7.1/src/device/queue.rs:548:42
stack backtrace:
   0: _rust_begin_unwind
   1: std::panicking::begin_panic_fmt
   2: wgpu_core::device::queue::<impl wgpu_core::hub::Global<G>>::queue_submit
   3: deno_core::ops_json::json_op_sync::{{closure}}
   4: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
   5: deno_runtime::metrics::metrics_op::{{closure}}
   6: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
   7: <extern "C" fn(A0) .> R as rusty_v8::support::CFnFrom<F>>::mapping::c_fn
   8: __ZN2v88internal25FunctionCallbackArguments4CallENS0_15CallHandlerInfoE
   9: __ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb0EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEESA_NS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EENS0_16BuiltinArgumentsE
  10: __ZN2v88internalL26Builtin_Impl_HandleApiCallENS0_16BuiltinArgumentsEPNS0_7IsolateE
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5

After failing to unmap a storage copy source buffer and then attempting to use it's contents with the GPU. I would expect this to throw a "nice" error from the Deno WebGPU op crate or sim. For example, something similar to the error thrown when attempting to unmap a buffer twice:

OperationError: Failed to execute 'unmap' on 'GPUBuffer': buffer is not ready to be unmapped.
    at GPUBuffer.unmap (deno:op_crates/webgpu/01_webgpu.js:1772:15)
    ...

I will endeavour to add a small reproduceable example to the issue when it isn't quite so late!

@lucacasonato lucacasonato added bug Something isn't working correctly webgpu WebGPU API labels Apr 10, 2021
@crowlKats
Copy link
Member

@asos-craigmorten would you be able to provide a reproducible example?

@asos-craigmorten
Copy link
Contributor Author

asos-craigmorten commented Jan 9, 2023

Reading my message back I'm not sure I understand what I was trying to say haha!

Nevertheless I am able to repro on latest Deno using the Deno provided example "hello-compute" by meddling with some of the flow to make it incorrect. Namely swapping lines 116 and 118 such that the destination stagingBuffer is already mapped prior to submitting the command encoder for the computation and copy.

REF: https://github.com/denoland/webgpu-examples/blob/main/hello-compute/mod.ts#L116

I've distilled this case down to what I think is the minimal code:

// Whole load of setup

const adapter = await navigator.gpu.requestAdapter();
const device = await adapter?.requestDevice();

if (!device) {
  console.error("No suitable adapter found!");

  Deno.exit(1);
}

const stagingBuffer = device.createBuffer({
  size: 0,
  usage: GPUBufferUsage.MAP_READ | GPUBufferUsage.COPY_DST,
});

const storageBuffer = device.createBuffer({
  size: 0,
  usage: GPUBufferUsage.STORAGE | GPUBufferUsage.COPY_DST |
    GPUBufferUsage.COPY_SRC,
});

// Code somewhat of interest

const encoder = device.createCommandEncoder();

encoder.copyBufferToBuffer(
  storageBuffer,
  0,
  stagingBuffer,
  0,
  0,
);

// I've swapped the map and the submit around so the code is now incorrect.
// I'd expect a nice error letting me know I've messed up, but instead this
// causes a panic when the command encoder is submitted.

await stagingBuffer.mapAsync(1);

console.log("BOOM 💣");

try {
  device.queue.submit([encoder.finish()]); // Panics
} catch (e) {
  console.error(e);
}

console.log("Already panicked");

Which when executed yields:

$ deno --version

deno 1.29.2 (release, x86_64-apple-darwin)
v8 10.9.194.5
typescript 4.9.4

$ RUST_BACKTRACE=1 deno run -A --no-check --unstable test.ts

BOOM 💣

============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: macos x86_64
Version: 1.29.2
Args: ["deno", "run", "-A", "--no-check", "--unstable", "test.ts"]

thread 'main' panicked at 'Buffer Valid((0, 1, Metal)) is still mapped', /Users/runner/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/wgpu-core-0.13.2/src/device/queue.rs:827:42
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: wgpu_core::device::queue::<impl wgpu_core::hub::Global<G>>::queue_submit
   3: <extern "C" fn(A0) .> R as v8::support::CFnFrom<F>>::mapping::c_fn
   4: __ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb0EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEENS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EEPmi
   5: __ZN2v88internal21Builtin_HandleApiCallEiPmPNS0_7IsolateE
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Which going by the panic, guessing is around https://github.com/denoland/deno/blob/main/ext/webgpu/src/queue.rs#L38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly webgpu WebGPU API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants