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

Blob::new(&[u8]) fails when wasm memory is backed by SharedArrayBuffer #208

Open
Liamolucko opened this issue Apr 7, 2022 · 3 comments
Open
Labels
bug Something isn't working

Comments

@Liamolucko
Copy link
Contributor

Describe the Bug

I've just stumbled upon an issue with my changes in #152, which is that new Blob() doesn't allow passing Uint8Arrays backed by SharedArrayBuffer. This is problematic because SharedArrayBuffer is sometimes used to back wasm memory, which will then cause Blob::new(&[u8]) to fail.

This isn't an issue for Blob::new(ArrayBuffer), since SharedArrayBuffer isn't actually a subclass of ArrayBuffer.

Blob::new will have to fall back to copying the buffer when wasm memory is backed by a SharedArrayBuffer, or just do it unconditionally.

Steps to Reproduce

  1. Clone https://github.com/Liamolucko/gloo-sab-repro
  2. Build and bindgen it:
cargo build --release
wasm-bindgen target/wasm32-unknown-unknown/release/gloo-sab-repro.wasm --out-dir dist --target web
  1. Run the resulting wasm in a web browser or Deno

Expected Behavior

It should run successfully.

Actual Behavior

new Blob() throws an error, which causes a panic since it's not expected to fail.

Additional Context

This is actually the case (according to the spec, at least) for basically everything accepting a buffer source, because WebIDL doesn't consider SharedArrayBuffer a real buffer source unless the type has the [AllowShared] attribute. Based on a quick grep of web-sys's WebIDL definitions, only WebGPU is using that right now.

I found out about this from getrandom's similar issue: rust-random/getrandom#164.

@Liamolucko Liamolucko added the bug Something isn't working label Apr 7, 2022
@ranile
Copy link
Collaborator

ranile commented Apr 7, 2022

Do you know of a fix for this?

@Liamolucko
Copy link
Contributor Author

Yes; if the wasm memory is backed by a SharedArrayBuffer, the bytes will have to be copied into a regular ArrayBuffer before passing to new Blob() (probably via. Uint8Array::from). It's a bit annoying having to re-introduce an intermediate copy, but it's more important that it works.

@ranile
Copy link
Collaborator

ranile commented Apr 7, 2022

Can we expose a separate API for handling this? We document that Blob cannot be used with SharedArrayBuffer, there will be a new type introduced to handle that.

I would really like to avoid copying memory. If we have to do it, it must be explicitly done

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

No branches or pull requests

2 participants