Create functions from closures

[kjvalencik]

What

Allow Neon users to create functions from closures.

    cx.export_function("count_called", {
        let n = std::cell::RefCell::new(0);

        move |mut cx| {
            *n.borrow_mut() += 1;

            Ok(cx.number(*n.borrow()))
        }
    })?;

How

The closure is boxed and added as data. It works very similar to the current functions except the trampoline function is generic.

Dropping the Rust closure requires napi_add_finalizer which was added in Node-API 5. For this reason, when using older Node-API versions, only fn instead of Fn are accepted.

Future Expansion

It would be unsound to accept FnMut because a function may be reentrant. A function could trivially accept a reference to itself and then call that function. However, it is very cumbersome to use RefCell to get access to mutable state.

Since most functions will not have any possibility for reentrancy and since closure lifetime inference is very limited in Rust, it would be useful for Neon to provide a JsFunction::new_mut that accepts an FnMut and wraps it in a RefCell.

[dherman]

Would it be better to leave them the same and document the limitation that closures will be leaked?

We talked about this and decided it's better to conditionally compile it for now and avoid the leak; most users won't even notice the lack of functionality in older Node-API versions but it will avoid a hard-to-detect bug.

[dherman]

As appealing as this is, unfortunately allowing FnMut is unsound. You can make a function call itself recursively, which Rust doesn't allow for closures (since there can be a live active borrow of the closure data).

let mut data = /* ... */;

let f = JsFunction::new(move |mut cx| {
    let this: Handle<JsObject> = cx.this()?;
    let method: Handle<JsFunction> = this.get(&mut cx, "recursive")?;
    let n: Handle<JsNumber> = cx.argument(0)?;
    let n = n.value(&mut cx) as u32;
    println!("obj.recursive({})", n);

    do_something(&data); // borrow the mutable data in the closure

    if (n > 0) {
        let n = cx.number(n - 1);
        let args = vec![n.upcast()];
        method.call(&mut cx, this, args)?;
    }
});

obj.set("recursive", f)?;

[dherman]

This looks great. I just have a few suggestions for clarity and one question about a possible leak.

[dherman]

Thanks for the clarifying comments. Looks great!