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

Add support for #[wasm_bindgen] on async fn #1754

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

alexcrichton
Copy link
Contributor

This commit adds support to attach #[wasm_bindgen] on an async fn
which will change the return value into a Promise in JS. This in
theory has the exact same semantics as an async function in JS where
you call it with all the arguments, nothing happens and you get a
promise back, and then later the promise actually resolves.

This commit also adds a helper trait, IntoJsResult, to allow async
functions with multiple kinds of return values instead of requiring
everything to be Result<JsValue, JsValue>.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! r=me with @Pauan's feedback addressed

This commit adds support to attach `#[wasm_bindgen]` on an `async fn`
which will change the return value into a `Promise` in JS. This in
theory has the exact same semantics as an `async` function in JS where
you call it with all the arguments, nothing happens and you get a
promise back, and then later the promise actually resolves.

This commit also adds a helper trait, `IntoJsResult`, to allow `async`
functions with multiple kinds of return values instead of requiring
everything to be `Result<JsValue, JsValue>`.
@alexcrichton alexcrichton merged commit 8861811 into rustwasm:master Sep 6, 2019
@alexcrichton alexcrichton deleted the async-fn branch September 6, 2019 18:47
cburgos added a commit to cburgos/wasm-bindgen that referenced this pull request Sep 6, 2019
Add support for `#[wasm_bindgen]` on `async fn` (rustwasm#1754)
@alexlapa
Copy link
Contributor

alexlapa commented Sep 27, 2019

@alexcrichton ,

Is this a known issue?

use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct Foo{

}
#[wasm_bindgen]
impl Foo {
    pub async fn bar(&self) -> Result<i32,JsValue> {
        Ok(42)
    }
}
error[E0597]: `me` does not live long enough
 --> src/lib.rs:8:1
  |
8 | #[wasm_bindgen]
  | ^^^^^^^^^^^^^^-
  | |             |
  | |             `me` dropped here while still borrowed
  | borrowed value does not live long enough
  | argument requires that `me` is borrowed for `'static`

error: aborting due to previous error
[dependencies]
wasm-bindgen = "0.2.51"
wasm-bindgen-futures = "0.4.1"

@alexcrichton
Copy link
Contributor Author

@alexlapa although not a great error message, that's a legitimate error message because futures shared with JS need to have the 'static lifetime unfortunately, which means &self can't work

@alexlapa
Copy link
Contributor

@alexcrichton ,

I see. Is it possible to make functions like this valid?

#[wasm_bindgen]
impl Foo {
    pub fn bar(&self) -> impl Future<Output=Result<Foo, JsValue>> + 'static {
        async {
            Ok(Foo {})
        }
    }
}

Or to add some kind of #[wasm_bindgen(async_static)] that would generate all necessary glue?

@alexcrichton
Copy link
Contributor Author

@alexlapa that should work yeah!

For that though you'll have to return -> js_sys::Promise and use wasm_bindgen_futures::future_to_promise directly.

@alexcrichton
Copy link
Contributor Author

As for automatically doing glue, I'm not entirely sure how it would work, but PRs and ideas are always welcome!

@Pauan
Copy link
Contributor

Pauan commented Sep 28, 2019

@alexlapa That isn't an issue with wasm-bindgen, it's an issue with how Rust Futures work in general:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=30a32014c0d66fb5e6cccb5d5bb5b541

Because Futures are heap allocated and run in the background, they cannot contain stack references. And &self is a stack reference.

Instead, you should take self by value:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=31f05428313900f131e4eaaa5fdf1daf

If you need to access self in multiple places, then Rc<Foo> is the way to do that. And if you need to mutate Foo, then Rc<RefCell<Foo>> is the way to do that.

Note: most of the time you can use stack references with async/await, it is only spawning that has the 'static restriction. But wasm-bindgen internally uses spawn in order to convert the Future into a JS Promise, so that's why it needs 'static.

@alexlapa
Copy link
Contributor

alexlapa commented Oct 8, 2019

@Pauan ,

Thanks for detailed explanation. So there is no way to #[wasm_bindgen] async fn foo(&self) -> _, and i have to use promises. Huge inconvenience is that promises erase concrete types which harms readability and testability. E.g.:

#[wasm_bindgen]
pub struct Foo;

#[wasm_bindgen]
impl Foo {
    pub fn new() -> Promise {
        future_to_promise(async {
            Ok(Foo.into())
        })
    }

    pub fn i_want_to_test_this_function(&self) -> bool {
        true
    }
}

#[wasm_bindgen_test]
async fn test() {
    let foo = JsFuture::from(Foo::new()).await.unwrap();

    assert!(foo.i_want_to_test_this_function()); // nope, not gonna happen
}

It would be awesome if impl Future<_> + 'static would be valid return type for exported functions, and future_to_promise conversion would happen in generated code in such case.

I can implement this feature, if it makes sense :)

@Pauan
Copy link
Contributor

Pauan commented Oct 28, 2019

@alexlapa Huge inconvenience is that promises erase concrete types which harms readability and testability.

That's fairly easy to workaround by simply having two methods:

impl Foo {
    pub async fn new_() -> Result<Foo, JsValue> {
        Ok(Foo)
    }
}

#[wasm_bindgen]
impl Foo {
    pub fn new() -> Promise {
        future_to_promise(Self::new_())
    }
}

Now you can test the new_ method in Rust (with proper typing), while still exposing a Promise to JS.

This is a common technique when using wasm-bindgen, because sometimes you need to do some type conversions when sending to JS, so that's what the above allows you to do.

It would be awesome if impl Future<_> + 'static would be valid return type for exported functions

That already is valid. The problem is that &self is not 'static, it's a shorthand for &'a self. So you can't have a &self method return a 'static, because that implies that the return value outlives self, which would cause use-after-free, so Rust doesn't allow that:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4bdb2d5abd8db1abb74cb0d885dac852

(Note: the above is a Rust error, it doesn't even have any wasm-bindgen code).

You can specify a method using &'static self...

impl Foo {
    pub async fn bar(&'static self) { ... }
}

...but now you can only put Foo inside of a static, you can't have a stack-allocated Foo anymore.

These restrictions aren't caused by wasm-bindgen, they're caused by lifetimes in Rust, which is necessary to prevent undefined behavior. So it can't be "fixed" by wasm-bindgen, because "fixing" it would cause undefined behavior.

Like I said, the solution is to not use &self, instead use self. All of these problems are caused by references, because references are stack-allocated (except for &'static), and Futures are heap allocated, so they can't contain stack references. This isn't an issue with only wasm-bindgen, other Future systems like tokio have it as well, it's just how Futures work in general.

But if you avoid references (i.e. you use self), then all the problems go away, because now there aren't any more references, so there's no more lifetime issue (lifetimes only care about references, not owned values).

But... if you can't use references, what if you need to access self in multiple places? In that case you can use self: Rc<Self>. And if you need to mutate self you can use self: Rc<RefCell<Self>>. It's very common to use Rc and RefCell when dealing with JS stuff.

@shijunti19
Copy link

@Pauan Is there a complete example of using self: RC < self > to call its own method

@Pauan
Copy link
Contributor

Pauan commented Nov 11, 2020

@shijunti19 I don't think there are any examples, but it's just standard Rust:

struct Inner {
    value: i32,
}

struct Foo {
    inner: RefCell<Inner>,
}

impl Foo {
    fn new(value: i32) -> Rc<Self> {
        Rc::new(Self {
            inner: RefCell::new(Inner { value }),
        })
    }

    async fn increment(self: Rc<Self>) {
        let mut borrow = self.inner.borrow_mut();
        borrow.value += 1;
    }
}
let x = Foo::new(5);

spawn_local(async move {
    x.increment().await;
});

Make sure that you never use .await while holding a borrow, or it will probably panic:

let mut borrow = self.inner.borrow_mut();

// Very bad, the borrow is still alive during the .await, don't do this
some_async().await;
{
    let mut borrow = self.inner.borrow_mut();
}

// This is okay, since it drops the borrow before the .await
some_async().await;

{
    // It can reborrow after the .await is finished
    let mut borrow = self.inner.borrow_mut();
}

@flosse
Copy link

flosse commented Nov 11, 2020

@Pauan thanks for your example.

I tried to combine this with wasm-bindgen but unfortunately that does not work.
Here is what I tried to do:

use std::{cell::RefCell, rc::Rc};
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct Wrapper {
    inner: RefCell<InnerCore>,
}

pub struct InnerCore {
    val: u32,
}

// TODO:
// Using `#[wasm_bindgen]` does not compile:
//
//     the trait `IntoWasmAbi` is not implemented for `std::rc::Rc<Wrapper>`
//
// #[wasm_bindgen]
impl Wrapper {
    //#[wasm_bindgen(constructor)]
    pub fn new() -> Rc<Wrapper> {
        Rc::new(Self {
            inner: RefCell::new(InnerCore { val: 33 }),
        })
    }

    pub async fn method_one(self: Rc<Wrapper>, _: u32) -> Result<JsValue, JsValue> {
        todo!()
    }

    pub async fn method_two(self: Rc<Wrapper>, _: String) -> Result<JsValue, JsValue> {
        todo!()
    }
}

Do you have any idea how to solve this?
Here is the code to play around with: https://github.com/flosse/wasm-bindgen-async-method-example

@Pauan
Copy link
Contributor

Pauan commented Nov 21, 2020

@flosse You have to move the Rc inside the struct:

#[wasm_bindgen]
#[derive(Clone)]
pub struct Wrapper {
    inner: Rc<RefCell<InnerCore>>,
}

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

Successfully merging this pull request may close these issues.

6 participants