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

Instance is Send even if it contains !Send host functions #1074

Closed
sfackler opened this issue Dec 17, 2019 · 2 comments · Fixed by #1161
Closed

Instance is Send even if it contains !Send host functions #1074

sfackler opened this issue Dec 17, 2019 · 2 comments · Fixed by #1161
Labels
bug Something isn't working 📦 lib-deprecated About the deprecated crates 🔈soundness Bugs causing an unsound API

Comments

@sfackler
Copy link

Describe the bug

In wasmer-runtime 0.11, Instance is always Send, but it can contain host functions which are not Send.

Steps to reproduce

Here's an example program that sends an instance containing a non-Send value (Rc) to another thread:

use wasmer_runtime::{imports, func};
use std::rc::Rc;
use std::thread;
use std::cell::RefCell;

fn main() {
    let module = &[];
    
    let non_send_state = Rc::new(RefCell::new(1i32));
    let imports = imports! {
        "env" => {
            "foobar" => func!({
                let non_send_state = non_send_state.clone();
                move || *non_send_state.borrow()
            }),
        },
    };

    let instance = wasmer_runtime::instantiate(module, &imports).unwrap();

    thread::spawn(move || drop(instance));
}

Expected behavior

It should not compile.

Actual behavior

It does compile.

Additional context

The implementations of ExternalFunction need to add a + Send to the Fn type parameter.

@sfackler sfackler added the bug Something isn't working label Dec 17, 2019
@MarkMcCaskey MarkMcCaskey added the 🔈soundness Bugs causing an unsound API label Dec 17, 2019
@MarkMcCaskey
Copy link
Contributor

Thanks for reporting this! I think it slipped through the cracks a bit due to timing, but I've just made a PR to fix it!

@Hywan
Copy link
Contributor

Hywan commented Jan 20, 2020

Thanks for the bug report, and sorry for the timing!

@Hywan Hywan added the 📦 lib-deprecated About the deprecated crates label Jan 20, 2020
@bors bors bot closed this as completed in b73f261 Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 📦 lib-deprecated About the deprecated crates 🔈soundness Bugs causing an unsound API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants