Wasm-bindgen: Inconsistent AudioBuffer / TypedArray return value interface

[pyrrho]

Summary

The AudioBuffer interface generated by web-sys is at odds with the native JS interface. In brief, in native JS audio_buffer.getChannelData() returns a Float32Array that points back to -- and allows modification of -- data living in the audio_buffer variable. In web-sys audio_buffer.get_channel_data().unwrap() returns a Vec<f32> that is a _copy of_ data living in the audio_buffer variable.

I put together a rust-webpack-template website with both Rust and JS examples of the above interface, in part to clearly demo the inconsistency, and in part to help myself understand what was going on.

https://github.com/ludumipsum/audio-interface-kerfuffle

I guess my questions are,

1. Is this known behavior?
2. Is this intended behavior?
3. Can this behavior be changed? (Both in terms of "Would you be willing..." and "Is it possible...")

Additional Details

I'm not sure where to put this, so... Let me say, learning and playing with this set of tools has been a ton of fun. Thank you for all the work put in so far. This paragraph isn't really a salient Additional Detail, but I am _very_ excited to see how all of this evolves.

Anyway.

I chose to pose this as a question rather than a focused bug report because I'm concerned it has broad implications regarding runtime memory management and the process of interface generation in web-sys. I'm not terribly familiar with either of those processes yet, so this may be more or less complex than I'm currently envisioning.

The long and short of this issue is a question of semantics and references. The semantics of the Float32Array returned by getChannelData is definitely referential; you edit the array, and that causes side-effects in the buffer that provided the array. The same semantics can be achieved in Rust, quite easily, by returning a &mut to an owned resource. But who owns the memory being copied by web-sys' get_channel_data()? Would it it possible to return a &Vec<T> from web-sys? Would it be correct to provide that interface across the WASM-module boundary?

I'm not sure where to look for those answers, honestly. I'm going to keep reading and poking, so I may update this issue if I get far enough to start taking shots at a PR. Please let me know if there's any more infromation I can provide. Thanks!

One final note;
greping through the .webidl files in web-sys, I've only been able to find two methods that return typed arrays; AudioBuffer's getChannelData, and TextEncoder's encode, so this may be a pretty small corner-case.

[alexcrichton]

Thanks for the report, and especially the thorough question!

This is all I think a result of the fact of how we generate the web-sys crate. The entire crate is procedurally generated from the Firefox webidl, and unfortunately the webidl doesn't have a lot of annotations that Rust might otherwise use for things like ownership and lifetimes.

For that reason whenever and API returns a Uint8Array, for example, we unconditionally assume it's "returning a fresh new array not referenced by anything else". This means that for wasm to see it then it needs to be copied back into wasm's own memory.

In your use case though it looks like this assumption isn't true for a particular API, where it's returning a reference to an already passed in buffer. That's sort of tricky for us to handle, but we can try to handle it in one-off cases! We've already had previous issues about how all slices are mutable but not all of them need to be, so we're starting to build up a whitelist of slices that can become immutable and tweaking APIs over time.

Unfortuntely shared references can't currently be returned from imported JS functions for vairous reasons, so I think the best thing to do here would be to actually omit the return value entirely. That way no data needs to unnecessarily be copied back and forth, and callers of the API would know that the return value is actually just what was already passed in.

Now doing all this as-is in web-sys today would probably be a breaking change so it may be awhile before we can do it, but you can always bind the API locally with a custom extern block!

[pyrrho]

Thanks for the detailed response!

You've mostly confirmed what I expected to be the case, both in terms of the memory model and the state of the API. I had come across the immutable whitelist in code, and agree that it's (somewhat unfortunately) the best way to handle that kind of ambiguous case. If there are other situations like this -- where the semantics of a return value can't be inferred by the idl -- I can definitely see the value of maintaining a similar list for return values.

This interface in particular might have an additional hangup, though. The typed array that AudioBuffer.getChannelData() returns isn't acting as a view over a buffer that was previously passed into the given audio buffer or audio context. The constructor for AudioBuffer is specified to directly call CreateByteDataBlock(length * numberOfChannels) (and that's not even an ArrayBuffer; it's the kind of thing ArrayBuffers _refer into_). I'm not 100% sure of this, but I assume the created data block will be placed at some unspecified location in the JS heap.

Is it currently possible for any WASM to receive a reference to and edit memory that's been allocated on the heap like that? I'm curious if the host-bindings proposal has anything to say about this...

fwiw, it is really easy to set up a #[wasm_bindgen(module = ...)] extern'd JS function that handles copying data from an arbitrary buffer into a &mut web_sys::AudioBuffer. I'm still interested in this question academically and theoretically, but, practically speaking, working around it is pretty simple.

[alexcrichton]

Currently no, a wasm module can only directly manipulate its own instance of WebAssembly.Memory, which it's only allowed to have one of (although eventually it will be able to have multiple).

[alexcrichton]

I'm gonna close this for now since I think the original question was answered and there's not a ton much else we can do about this, but let us know if there's anything else!