3

My first Rust-produced WASM is producing the following error, I have no idea how to go about it debugging.

wasm-000650c2-23:340 Uncaught RuntimeError: memory access out of bounds
    at dlmalloc::dlmalloc::Dlmalloc::free::h36961b6fbcc40c05 (wasm-function[23]:670)
    at __rdl_dealloc (wasm-function[367]:8)
    at __rust_dealloc (wasm-function[360]:7)
    at alloc::alloc::dealloc::h90df92e1f727e726 (wasm-function[146]:100)
    at <alloc::alloc::Global as core::alloc::Alloc>::dealloc::h7f22ab187c7f5835 (wasm-function[194]:84)
    at <alloc::raw_vec::RawVec<T, A>>::dealloc_buffer::hdce29184552be976 (wasm-function[82]:231)
    at <alloc::raw_vec::RawVec<T, A> as core::ops::drop::Drop>::drop::h3910dccc175e44e6 (wasm-function[269]:38)
    at core::ptr::real_drop_in_place::hd26be2408c00ce9d (wasm-function[267]:38)
    at core::ptr::real_drop_in_place::h6acb013dbd13c114 (wasm-function[241]:50)
    at core::ptr::real_drop_in_place::hb270ba635548ab74 (wasm-function[69]:192)

The context: latest Chrome, Rust wasm-bindgen code called from a TypeScript custom element, operating upon a canvas in the shadow DOM. Data rendered to the canvas comes from an HTML5 AudioBuffer. All rust variables are locally scoped.

The web component works perfectly if only one instance appears in the document, but if I further instances, a stack trace is dumped as above. The code runs without any other issue.

I know there are outstanding memory bugs in Chrome -- is this what they look like, or can an experienced rust/wasm developer tell me if this is unusual?

js-sys = "0.3.19"
wasm-bindgen = "0.2.42"
wee_alloc = { version = "0.4.2", optional = true }
[dependencies.web-sys]
version = "0.3.4"

The rust code is small, and just renders two channels of an AudioBuffer to a supplied HTMLCanvasElement:

#[wasm_bindgen]
pub fn render(
    canvas: web_sys::HtmlCanvasElement,
    audio_buffer: &web_sys::AudioBuffer,
    stroke_style: &JsValue,
    line_width: f64,
    step_size: usize,
) { 
  // ...
    let mut channel_data: [Vec<f32>; 2] = unsafe { std::mem::uninitialized() }; // !
    for channel_number in 0..1 {
        channel_data[channel_number] = audio_buffer
            .get_channel_data(channel_number as u32)
            .unwrap();
    }
  // ...

I've tried commenting out functionality, and if the code doesn't touch the canvas but does the above, I get the error. Making the below change results in a simple 'out of wam memory' error. The audio file is is 1,200 k.

    let channel_data: [Vec<f32>; 2] = [
        audio_buffer.get_channel_data(0).unwrap(),
        audio_buffer.get_channel_data(1).unwrap()
    ];

EDIT: The latter out of memory error, for the correct code above, really threw me, but it is actually a Chrome bug.

Lee Goddard
  • 10,680
  • 4
  • 46
  • 63
  • 1
    I hope you see, why an [mcve] is crucial ;) I'm glad I was able to solve your problem. I must admit, that we (the Rust tag "member") are sometimes very eager after MCVEs but it helps a lot! – hellow Apr 18 '19 at 10:02
  • Thanks for helping me work out what a minimal example is in this ridiculous case! I'm now only getting an out-of-memory error, but that's another question. – Lee Goddard Apr 18 '19 at 10:08
  • In fact, the 'out of memory' error was because of a Chrome bug I saw yesterday but forgot: https://stackoverflow.com/questions/55039923/why-does-chrome-eventually-throw-out-of-memory-wasm-memory-after-repeatedly-r – Lee Goddard Apr 18 '19 at 10:17

2 Answers2

6

Your problem is that you create a chunk of uninitialized memory and don't initialize it properly:

let mut channel_data: [Vec<f32>; 2] = unsafe { std::mem::uninitialized() };
for channel_number in 0..1 {
    channel_data[channel_number] = audio_buffer
        .get_channel_data(channel_number as u32) // no need for `as u32` here btw
        .unwrap();
}

Ranges (a.k.a. a..b) are exclusive in Rust. This means that your loop does not iterate twice as you would suppose, but instead only once and you have one uninitialized Vec<f32> which then will panic while dropping it. (Please see Matthieu M.'s answer for a proper explanation)

There are a few possibilities here.

  1. Use the proper range, e.g. 0..2
  2. Use an inclusive range 0..=1
  3. Don't use the unsafe construct, but instead
    let mut channel_data: [Vec<f32>; 2] = Default::default()
    
    This will properly initialize the two Vecs.

For a more complete overview on how to initialize an array, see What is the proper way to initialize a fixed length array?

As a sidenote: avoid using unsafe, especially if you're new to Rust.

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
hellow
  • 12,430
  • 7
  • 56
  • 79
  • 1
    I think recommending a *proper* iteration would be better. Don't use ranges, use iteration + `enumerate()`, and you're guaranteed to iterate over all. – Matthieu M. Apr 18 '19 at 11:54
  • @MatthieuM. feel free to add a fourth option. – hellow Apr 18 '19 at 12:13
  • Actually, there is an error in the diagnostic: the issue is not leaving one element uninitialized, it's that try to assign to the first element will cause a drop of uninitialized memory. I've settled for posting my own answer instead, rather than massively editing yours. – Matthieu M. Apr 18 '19 at 14:27
5

There are two issues here:

  1. You create an uninitialized chunk of memory, and treat it as if it were initialized.
  2. Your iteration is wrong, 0..1 iterates over [0] (it's exclusive).

Let's check them one at a time.


Don't use unsafe.

In general, you should strive to avoid unsafe. There are very few reasons to use it, and many ways to use it incorrectly (such as here).

The issue.

In this particular case:

let mut channel_data: [Vec<f32>; 2] = unsafe { std::mem::uninitialized() };
for channel_number in /*...*/ {
    channel_data[channel_number] = /*...*/;
}

There are two issues:

  1. The use of std::mem::uninitialized is deprecated because of safety reasons; it's a very bad idea to use it. Its replacement is MaybeUninitialized.
  2. Assigning to uninitialized memory is Undefined Behavior.

There is no assignment operator in Rust, in order to perform an assignment the language will:

  • Drop the previous instance.
  • Overwrite the now unused memory.

Dropping raw memory which thinks it's a Vec is Undefined Behavior; in this case the likely effect is that some random pointer value is read and freed. This may crash, this may free an unrelated pointer, leading to a latter crash or memory corruption, it's BAD.

The solution.

There is little reason to use unsafe here:

  • It is perfectly possible to safely perform the initialization of the array.
  • It is perfectly possible to directly initialize the array.
  • There is little performance benefit in NOT performing default initialization if you insist on two-steps initialization, as the Default implementation of Vec does not allocate memory.

In short:

auto create_channel = |channel_number: u32| {
    audio_buffer
        .get_channel_data(channel_number)
        .unwrap()
};

let mut channel_data = [create_channel(0), create_channel(1)];

is simple, safe, and most efficient.


Prefer iterators to indexing.

If you insist on two-step initialization, then use iterators rather than indexing to avoid off-by-one errors.

In your case:

let mut channel_data = [vec!(), vec!()];
for (channel_number, channel) = channel_data.iter_mut().enumerate() {
    *channel = audio_buffer
        .get_channel_data(channel_number as u32)
        .unwrap();
}

There are many utility functions on Iterator, in this particular case, enumerate will wrap the item returned by iter_mut() (a &mut Vec<f32>) into a tuple (usize, &mut Vec<32>):

  • You have direct access to the element, no computation required.
  • You also have the index of the element, without off-by-one errors.
Community
  • 1
  • 1
Matthieu M.
  • 287,565
  • 48
  • 449
  • 722