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

RuntimeError: memory access out of bounds #1526

Closed
takkuumi opened this issue May 11, 2019 · 3 comments · Fixed by #1530
Closed

RuntimeError: memory access out of bounds #1526

takkuumi opened this issue May 11, 2019 · 3 comments · Fixed by #1530
Labels

Comments

@takkuumi
Copy link

takkuumi commented May 11, 2019

when i call clearInterval, there is a problem : "memory access out of bounds".

this is the rust code:

extern crate js_sys;

use self::js_sys::{Array, Function};

use wasm_bindgen::prelude::*;

use std::panic;


#[wasm_bindgen]
extern "C" {
  fn setInterval(closure: &Closure<FnMut()>, time: u32) -> i32;
  fn clearInterval(id: i32);
  #[wasm_bindgen(js_namespace = console)]
  fn log(s: &str);
}

#[wasm_bindgen]
pub struct Interval {
  interval_id: i32,
  _closure: Closure<FnMut()>,
}


// When the Interval is destroyed, cancel its `setInterval` timer
impl Drop for Interval {
  fn drop(&mut self) {
    clearInterval(self.interval_id);
  }
}


#[wasm_bindgen]
pub fn init_interval(func: Function, timer: u32) -> Interval {
  let exec = move || {
    panic::catch_unwind(|| {
      func.apply(&JsValue::null(), &Array::new()).unwrap();
    })
    .unwrap();
  };

  let js_callback = Box::new(exec) as Box<FnMut() + 'static>;

  // First up we use `Closure::wrap` to wrap up a Rust closure and create
  // a JS closure.
  let _closure = Closure::wrap(js_callback);

  // Next we pass this via reference to the `setInterval` function, and
  // `setInterval` gets a handle to the corresponding JS closure.
  let interval_id = setInterval(&_closure, timer);

  // If we were to drop `cb` here it would cause an exception to be raised
  // whenever the interval elapses. Instead we *return* our handle back to JS
  // so JS can decide when to cancel the interval and deallocate the closure.
  Interval {
    interval_id,
    _closure,
  }
}

this is my typescript code:

import { init_interval } from 'rust_assembly'


const s = init_interval(() => {
  console.log('Called by WebAssembly:', Date.now())
}, 1e3)


setTimeout(() => {
  s.free()
}, 5e3);

the log:

Called by WebAssembly: 1557605216313
Called by WebAssembly: 1557605217317
Called by WebAssembly: 1557605218320
Called by WebAssembly: 1557605219320
wasm-0001cfce:14



RuntimeError: memory access out of bounds
    at dlmalloc::dlmalloc::Dlmalloc::free::h8ea8901d99ed60ad (wasm-function[13]:15)
    at __rdl_dealloc (wasm-function[122]:8)
    at __rust_dealloc (wasm-function[109]:7)
    at <dyn +core::ops::function::FnMut<()>+Output = R as wasm_bindgen::closure::WasmClosure>::describe::destroy::hb2b20aef9624895d (wasm-function[83]:34)
    at Function.cb (/Users/ling/Codelab/tswasm/rust_assembly/pkg/rust_assembly.js:260:35)
    at listOnTimeout (internal/timers.js:531:17)
    at processTimers (internal/timers.js:475:7)
@takkuumi
Copy link
Author

Could anyone help me?

alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue May 13, 2019
This commit updates the drop glue generated for closures to simply
ignore null pointers. The drop glue can be called in erroneous
situations such as when a closure is invoked after it's been destroyed.
In these cases we don't want to segfault and/or corrupt memory but
instead let the normal error message from the invoke glue continue to
get propagated.

Closes rustwasm#1526
@alexcrichton
Copy link
Contributor

Thanks for the report! The bug here is that we're not providing a great error message sooner and we're allowing the runtime to continue in an invalid state. The cause of this is that the destroyed Closure is actually called despite the clearInterval, I think because the callback may have already been enqueued or something like that?

I've opened a #1530 so this'll have a clearer error message and the 'segfault' will be fixed.

@takkuumi
Copy link
Author

@alexcrichton thx, I will attention to this issue. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants