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

calling a JS function in the spawn function -- possible? #128

Closed
stevefan1999-personal opened this issue Apr 6, 2023 · 5 comments
Closed

Comments

@stevefan1999-personal
Copy link

I want to implement setInterval/setTimeout so I wrote something like this:

#[bind(object)]
#[quickjs(bare)]
pub mod timer {
    use rquickjs::{Ctx, Function};
    use std::time::Duration;
    use tokio::time;

    #[quickjs(rename = "setInterval")]
    pub fn set_interval<'js>(func: Function<'js>, delay: Option<usize>, ctx: Ctx<'js>) -> usize {
        let duration = Duration::from_millis(delay.unwrap_or(0) as u64);
        let mut interval = time::interval(duration);
        ctx.spawn(async move {
            loop {
                interval.tick().await;
                let _ = func.defer_call(());
            }
        });
        0
    }

    #[quickjs(rename = "clearInterval")]
    pub fn clear_interval(handle: usize) {}

    #[quickjs(rename = "setTimeout")]
    pub fn set_timeout(func: Function, delay: Option<usize>, ctx: Ctx) -> usize {
        let duration = Duration::from_millis(delay.unwrap_or(0) as u64);
        let mut interval = time::interval(duration);
        ctx.spawn(async move {
            interval.tick().await;
            let _ = func.defer_call(());
        });
        0
    }

    #[quickjs(rename = "clearTimeout")]
    pub fn clear_timeout(handle: usize) {}
}

However, it seems like it can't compile:

PS E:\rust\rquickjs-integration> cargo check
    Checking rquickjs-fun v0.1.0 (E:\rust\rquickjs-integration)
error: future cannot be sent between threads safely                                                                                                                                                                                                                                                                    
   --> src\js.rs:18:19
    |
18  |           ctx.spawn(async move {
    |  ___________________^
19  | |             loop {
20  | |                 interval.tick().await;
21  | |                 let _ = func.defer_call(());
22  | |             }
23  | |         });
    | |_________^ future created by async block is not `Send`
    |
    = help: within `[async block@src\js.rs:18:19: 23:10]`, the trait `Send` is not implemented for `NonNull<JSContext>`
note: captured value is not `Send`
   --> src\js.rs:21:25
    |
21  |                 let _ = func.defer_call(());
    |                         ^^^^ has type `rquickjs::Function<'js>` which is not `Send`
note: required by a bound in `Ctx::<'js>::spawn`
   --> C:\Users\steve\scoop\persist\rustup\.cargo\git\checkouts\rquickjs-c5d9b5f474075f2d\234cc91\core\src\context\ctx.rs:197:33
    |
197 |         F: Future<Output = T> + ParallelSend + 'static,
    |                                 ^^^^^^^^^^^^ required by this bound in `Ctx::<'js>::spawn`

error: future cannot be sent between threads safely
   --> src\js.rs:18:19
    |
18  |           ctx.spawn(async move {
    |  ___________________^
19  | |             loop {
20  | |                 interval.tick().await;
21  | |                 let _ = func.defer_call(());
22  | |             }
23  | |         });
    | |_________^ future created by async block is not `Send`
    |
    = help: within `[async block@src\js.rs:18:19: 23:10]`, the trait `Send` is not implemented for `*mut c_void`
note: captured value is not `Send`
   --> src\js.rs:21:25
    |
21  |                 let _ = func.defer_call(());
    |                         ^^^^ has type `rquickjs::Function<'js>` which is not `Send`
note: required by a bound in `Ctx::<'js>::spawn`
   --> C:\Users\steve\scoop\persist\rustup\.cargo\git\checkouts\rquickjs-c5d9b5f474075f2d\234cc91\core\src\context\ctx.rs:197:33
    |
197 |         F: Future<Output = T> + ParallelSend + 'static,
    |                                 ^^^^^^^^^^^^ required by this bound in `Ctx::<'js>::spawn`

error: future cannot be sent between threads safely
   --> src\js.rs:34:19
    |
34  |           ctx.spawn(async move {
    |  ___________________^
35  | |             interval.tick().await;
36  | |             let _ = func.defer_call(());
37  | |         });
    | |_________^ future created by async block is not `Send`
    |
    = help: within `[async block@src\js.rs:34:19: 37:10]`, the trait `Send` is not implemented for `NonNull<JSContext>`
note: captured value is not `Send`
   --> src\js.rs:36:21
    |
36  |             let _ = func.defer_call(());
    |                     ^^^^ has type `rquickjs::Function<'_>` which is not `Send`
note: required by a bound in `Ctx::<'js>::spawn`
   --> C:\Users\steve\scoop\persist\rustup\.cargo\git\checkouts\rquickjs-c5d9b5f474075f2d\234cc91\core\src\context\ctx.rs:197:33
    |
197 |         F: Future<Output = T> + ParallelSend + 'static,
    |                                 ^^^^^^^^^^^^ required by this bound in `Ctx::<'js>::spawn`

error: future cannot be sent between threads safely
   --> src\js.rs:34:19
    |
34  |           ctx.spawn(async move {
    |  ___________________^
35  | |             interval.tick().await;
36  | |             let _ = func.defer_call(());
37  | |         });
    | |_________^ future created by async block is not `Send`
    |
    = help: within `[async block@src\js.rs:34:19: 37:10]`, the trait `Send` is not implemented for `*mut c_void`
note: captured value is not `Send`
   --> src\js.rs:36:21
    |
36  |             let _ = func.defer_call(());
    |                     ^^^^ has type `rquickjs::Function<'_>` which is not `Send`
note: required by a bound in `Ctx::<'js>::spawn`
   --> C:\Users\steve\scoop\persist\rustup\.cargo\git\checkouts\rquickjs-c5d9b5f474075f2d\234cc91\core\src\context\ctx.rs:197:33
    |
197 |         F: Future<Output = T> + ParallelSend + 'static,
    |                                 ^^^^^^^^^^^^ required by this bound in `Ctx::<'js>::spawn`

error: could not compile `rquickjs-fun` (bin "rquickjs-fun") due to 4 previous errors

I have the parallel features on and maybe I can just ignore this for now, maybe I can workaround by using an async promise function and do the loop inside, but this would not be standard compliant

@stevefan1999-personal
Copy link
Author

stevefan1999-personal commented Apr 6, 2023

#[bind(object)]
#[quickjs(bare)]
pub mod timer {
    use rquickjs::{Ctx, Function};
    use std::time::Duration;
    use tokio::time;

    #[quickjs(rename = "setInterval")]
    pub async fn set_interval<'js>(func: Function<'js>, delay: Option<usize>) {
        let duration = Duration::from_millis(delay.unwrap_or(0) as u64);
        let mut interval = time::interval(duration);

        loop {
            interval.tick().await;
            let _ = func.defer_call(());
        }
    }

    #[quickjs(rename = "clearInterval")]
    pub fn clear_interval(handle: usize) {}

    #[quickjs(rename = "setTimeout")]
    pub async fn set_timeout<'js>(func: Function<'js>, delay: Option<usize>) {
        let duration = Duration::from_millis(delay.unwrap_or(0) as u64);
        let mut interval = time::interval(duration);
        interval.tick().await;
        let _ = func.defer_call(());
    }

    #[quickjs(rename = "clearTimeout")]
    pub fn clear_timeout(handle: usize) {}

Well this doesn't work either...

@DelSkayn
Copy link
Owner

DelSkayn commented Apr 6, 2023

You can't do that safely with a multi threaded runtime. Ctx::spawn is just a wrapper around the spawn function of whatever async runtime you have registered. With a multi threaded runtime futures can be run at any time in parallel but quickjs cannot be used in parallel. If you want to use rust futures which call other quickjs functions you need to make sure that the futures are run on a single thread.

@DelSkayn
Copy link
Owner

DelSkayn commented Apr 6, 2023

You can't do that safely with a multi threaded runtime. Ctx::spawn is just a wrapper around the spawn function of whatever async runtime you have registered. With a multi threaded runtime futures can be run at any time in parallel but quickjs cannot be used in parallel. If you want to use rust futures which call other quickjs functions you need to make sure that the futures are run on a single thread.

After looking to it somewhat more it seems that my previous comment is not entirely right. It might be safe but I am not entirely sure. We might be able to make rquickjs able to spawn futures which aren't Send safely but I don't have time at the moment to fully look into it. For right now it is not possible as the parallel feature indeed requires that futures are Send and quickjs values cannot be send across threads which can happen when a value is held across and await with a multithreaded runtime.

@stevefan1999-personal
Copy link
Author

I have managed to implement this way:

#[bind(object)]
#[quickjs(bare)]
pub mod timer {
    use rquickjs::{Context, Ctx, Function, Persistent};
    use std::time::Duration;
    use tokio::{select, time};
    use tokio_util::sync::CancellationToken;

    #[quickjs(cloneable)]
    #[derive(Clone)]
    pub struct CancellationTokenWrapper(CancellationToken);

    #[quickjs(rename = "setInterval")]
    pub fn set_interval(
        func: Persistent<Function<'static>>,
        delay: Option<usize>,
        ctx: Ctx,
    ) -> CancellationTokenWrapper {
        let delay = delay.unwrap_or(0) as u64;
        let duration = Duration::from_millis(delay);
        let mut interval = time::interval(duration);
        let token = CancellationToken::new();
        ctx.spawn({
            let token = token.clone();
            let context = Context::from_ctx(ctx).unwrap();
            async move {
                loop {
                    select! {
                        _ = token.cancelled() => break,
                        _ = interval.tick() => {
                            let _ = context.with(|ctx| func.clone().restore(ctx)?.defer_call(()));
                        }
                    }
                }
            }
        });

        CancellationTokenWrapper(token)
    }

    #[quickjs(rename = "clearInterval")]
    pub fn clear_interval(token: CancellationTokenWrapper) {
        token.0.cancel();
    }

    #[quickjs(rename = "setTimeout")]
    pub fn set_timeout(
        func: Persistent<Function<'static>>,
        delay: Option<usize>,
        ctx: Ctx,
    ) -> CancellationTokenWrapper {
        let delay = delay.unwrap_or(0) as u64;
        let duration = Duration::from_millis(delay);
        let token = CancellationToken::new();
        ctx.spawn({
            let token = token.clone();
            let context = Context::from_ctx(ctx).unwrap();
            async move {
                select! {
                    _ = token.cancelled() => { }
                    _ = time::sleep(duration) => {
                        let _ = context.with(|ctx| func.restore(ctx)?.defer_call(()));
                    }
                }
            }
        });
        CancellationTokenWrapper(token)
    }

    #[quickjs(rename = "clearTimeout")]
    pub fn clear_timeout(token: CancellationTokenWrapper) {
        token.0.cancel();
    }
}

If you think this is safe, let's close the issue for now.

@DelSkayn
Copy link
Owner

DelSkayn commented Apr 7, 2023

Yep that seems a way to do it safely. I'll close the issue.

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

No branches or pull requests

2 participants