neon-bindings / neon

Rust bindings for writing safe and fast native Node.js modules.
https://www.neon-bindings.com/
Apache License 2.0
8k stars 283 forks source link

Trouble passing this to an async method in a neon class #986

Closed Daniel-Boll closed 1 year ago

Daniel-Boll commented 1 year ago

I'm trying to create a Neon class with an async method that requires access to the class instance (this). I'm facing difficulties in passing the this object to the async block within the method.

Here's my current implementation:

pub fn js_connection(mut cx: FunctionContext) -> JsResult<JsPromise> {
    let runtime = tokio_runtime(&mut cx)?;

    let cluster = cx
        .this()
        .downcast_or_throw::<JsBox<ScyllaCluster>, _>(&mut cx)?;

    let (deferred, promise) = cx.promise();
    let channel = cx.channel();

    // Parameters: optional keyspace
    let keyspace: Option<String> = cx
        .argument_opt(0)
        .and_then(|keyspace| keyspace.downcast_or_throw::<JsString, _>(&mut cx).ok())
        .map(|keyspace| keyspace.value(&mut cx));

    runtime.spawn(async move {
        let result = cluster.connect(keyspace, channel).await;

        // My logic...
    });

    Ok(promise)
}

As mentioned in one of the Neon's example, only Rust types that are Send can be moved into the async block. JavaScript values must first be converted to Rust types. However, the JsBox type is not Send by design.

I'm unsure if it's currently possible to achieve what I'm trying to do, or if my approach is correct. Can you provide guidance on how to pass the this object to an async block within a Neon class method?

Best Regards, Daniel Boll. 🎴

kjvalencik commented 1 year ago

You can only access JavaScript objects on the JavaScript main thread. The Root type can be used to save a reference to a JavaScript value and pass it across threads.

However, in order to get access to it again, you will need to be back on the main JavaScript thread. For example with a Channel.

In this case, it seems like you don't actually need a JavaScript value, you just need a reference to ScyllaCluster.

You may want to consider putting it in an Arc and passing a clone of that.

    let cluster = cx
        .this()
        .downcast_or_throw::<JsBox<Arc<ScyllaCluster>>, _>(&mut cx)?;

    // This is an `Arc<ScyllaCluster`, no box and it can be moved across threads.
    let cluster = Arc::clone(&cluster);
Daniel-Boll commented 1 year ago

Hello, @kjvalencik, thank you for your valuable insights.

Based on your feedback, I have made some improvements to my code. Since my logic depends on the mutable reference of self in one method, I realized that I needed to use a Mutex along with the Arc. Therefore, I have updated my code to use the Arc<Mutex<ScyllaCluster>> structure.

let cluster = cx
  .this()
  .downcast_or_throw::<JsBox<Arc<Mutex<ScyllaCluster>>>, _>(&mut cx)?;

Initially, I tried using both std and tokio sync mutexes, but I only managed to make tokio mutex work within the Async block correctly.

let cluster_clone = Arc::clone(&cluster);
runtime.spawn(async move {
  let cluster_lock = cluster_clone.lock();
  let result = cluster_lock.await.connect(keyspace, channel.clone()).await;
  // ...
});

Unfortunately, I encountered an issue where I couldn't create a JsBox of tokio Mutex.

pub fn js_new(mut cx: FunctionContext) -> JsResult<JsBox<Arc<tokio::sync::Mutex<ScyllaCluster>>>> { // This don't work
pub fn js_new(mut cx: FunctionContext) -> JsResult<JsBox<Arc<std::sync::Mutex<ScyllaCluster>>>> { // This work

As a result, I had to use std Mutex.

runtime.spawn(async move {
  let cluster_lock = cluster_clone.lock().unwrap();
  let result = cluster_lock.connect(keyspace, channel).await;

 // ...
});

Which in turn gives me this error:

future cannot be sent between threads safely
   within `[async block]`, the trait `std::marker::Send` is not implemented for `std::sync::MutexGuard<'_, scylla_wrapper::cluster::ScyllaCluster>`

Thank you once again for your feedback, and please let me know if you have any further suggestions.

Best Regards, Daniel Boll. 🎴

kjvalencik commented 1 year ago

I don't see any reason you shouldn't be able to use a tokio mutex.

The error is from trying to send a guard across threads. Make sure you are sending the actual Arc<Mutex<_>> across threads instead of the MutexGuard from unlocking.

If you don't mind, I'm going to close this issue since this is more about general Rust guidance than anything specific to Neon. Feel free to continue asking questions or re-open if you feel that is incorrect.

Thanks!