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

Question: How to return something that borrows from the function context? #978

Closed stevenliebregt closed 1 year ago

stevenliebregt commented 1 year ago

Hi, I'm not sure how exactly to word this, so I hope the example below makes it clearer.

I'm trying to build a wrapper around a rust database client that uses postgres under the hood. But I'm having some lifetime troubles.

The specific case I'm struggling with is returning an async stream.

pub fn repo_query_many_stream(mut cx: FunctionContext) -> JsResult<JsFunction> {
    // This is the wrapper around my `Repo` which does the querying
    let repo_wrapper = &**cx.argument::<JsBox<RepoWrapper>>(0)?;
    // Borrow the actual repo
    let mut repo = repo_wrapper.0.borrow_mut();

    // Data is a `postgres::RowIter` which has a lifetime to the connection
    let data = repo.query_many_stream().or_else(|err| cx.throw_error(err.to_string()))?;

    // The intention was to return a function that you can call multiple times to get the next 
    // item from the `postgres::RowIter` which under the hood then fetches the next item from the
    // connection.
    JsFunction::new(&mut cx, move |mut fcx| -> JsResult<JsValue> {
        match data.next() {
            Ok(row) => match row {
                // We're done iterating, no more values, return `undefined`
                None => Ok(fcx.undefined().upcast()),
                // We have a next value, return it
                Some(row) => {
                    let object = fcx.empty_object();

                    for (index, column) in row.columns().iter().enumerate() {
                        match column.type_() {
                            &Type::TEXT => {
                                let value = fcx.string(row.get::<_, String>(index));
                                object.set(&mut fcx, column.name(), value)?;
                            }
                            &Type::INT4 => {
                                let value = fcx.number(row.get::<_, i32>(index));
                                object.set(&mut fcx, column.name(), value)?;
                            }
                            ty => unimplemented!("Postgres type conversion not implemented: {:#?}", ty),
                        }
                    }

                    Ok(object.upcast())
                }
            },
            Err(_) => Ok(fcx.undefined().upcast()),
        }
    })
}

The problem however is that the JsFunction is a Fn closure, so I can't call next on data. And the following lifetime problem:

error[E0716]: temporary value dropped while borrowed
   --> repo_ffi_node/src/lib.rs:107:27
    |
107 |     let repo_wrapper = &**cx.argument::<JsBox<RepoWrapper>>(0)?;
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
...
110 |     let data = repo.query_many_stream().or_else(|err| cx.throw_error(err.to_string()))?;
    |                ------------------------ argument requires that borrow lasts for `'static`
...
141 | }
    | - temporary value is freed at the end of this statement

error[E0521]: borrowed data escapes outside of function
   --> repo_ffi_node/src/lib.rs:110:16
    |
106 | pub fn repo_query_many_stream(mut cx: FunctionContext) -> JsResult<JsFunction> {
    |                               ------
    |                               |
    |                               `cx` is a reference that is only valid in the function body
    |                               has type `CallContext<'1, neon::prelude::JsObject>`
...
110 |     let data = repo.query_many_stream().or_else(|err| cx.throw_error(err.to_string()))?;
    |                ^^^^^^^^^^^^^^^^^^^^^^^^
    |                |
    |                `cx` escapes the function body here
    |                argument requires that `'1` must outlive `'static`

error[E0597]: `repo` does not live long enough
   --> repo_ffi_node/src/lib.rs:110:16
    |
110 |     let data = repo.query_many_stream().or_else(|err| cx.throw_error(err.to_string()))?;
    |                ^^^^^^^^^^^^^^^^^^^^^^^^
    |                |
    |                borrowed value does not live long enough
    |                argument requires that `repo` is borrowed for `'static`
...
141 | }
    | - `repo` dropped here while still borrowed

Is what I'm trying to do even possible?

kjvalencik commented 1 year ago

The issue is that repo is borrowing from repo_wrapper. When a value is put in a JsBox, ownership is transferred to the JavaScript runtime and you can only get references afterwards.

The typical pattern in Rust for this is to wrap the value in an Arc.

stevenliebregt commented 1 year ago

As far as I see I can't make this work with Neon, I tried looking at the gzip-stream example, but that uses Clone which I can't use as the underlying connection also isn't cloneable.

kjvalencik commented 1 year ago

That's what reference counting like an Arc does. It makes it cloneable (cloning increments the counter instead of copying the struct).

stevenliebregt commented 1 year ago

I feel like I'm not clear enough on what my problem exactly is, but I have a hard time explaining it.

The Repo has a postgres::Client and has a method fn query_many_stream(&mut self) -> Result<postgres::RowIter, Error> this RowIter has a lifetime 'a to the connection.

I have a hard time seeing what part exactly I should wrap in a Mutex, any help is much appreciated.

demurgos commented 1 year ago

After you return the value it is managed by the JS VM. The result must be 'static. If RowIter borrows from something, it is not compatible and you need to refactor your code to avoid this non-static lifetime.

You could maybe consider moving the connection with the stream using some of the crates enabling self-referential structs.