neon-bindings / rfcs

RFCs for changes to Neon
Apache License 2.0
14 stars 9 forks source link

JsBox RFC #33

Closed kjvalencik closed 4 years ago

kjvalencik commented 4 years ago

JsBox is a smart pointer to data created in Rust and managed by the V8 garbage collector. JsBox are a basic building block for higher level APIs like neon classes.

fn person_new(mut cx: FunctionContext) -> JsResult<JsBox<Person>> {
    let name = cx.argument::<JsString>(0)?.value(&mut cx);
    let person = Person::new(name);

    Ok(cx.boxed(person))
}

fn person_greet(mut cx: FunctionContext) -> JsResult<JsString> {
    let person = cx.argument::<JsRef<Person>>(0)?;
    let greeting = person.greet();

    Ok(cx.string(greeting))
}

Rendered RFC

Work in progress implementation

kjvalencik commented 4 years ago

This RFC has entered the final comment period.

kjvalencik commented 4 years ago

Future Expansion

Background

While exploring the Persistent RFC, an alternate design that requires manual drops was proposed. Manual drops are very simple for single use Persistent, but can become complicated when Persistent are cloned.

Possible Solution

JsBox could support an optional finalizer that is called on the main JavaScript thread prior to the Rust data being dropped. Since in most cases multi-use Persistent will be owned by a JsBox, it provides an opportunity to perform a manual drop.

API

pub trait Finalize {
    fn finalize(self, cx: FinalizeContext);
}

impl<T: Send + 'static> JsBox<T> {
    pub fn with_finalizer<'a, C>(cx: &mut C, value: T) -> Handle<'a, JsBox<T>>
    where
        C: Context<'a>,
        T: Finalize;
}

An additional JsBox constructor is added that accepts a T: Finalize and calls the finalize method prior to dropping.

Following idiomatic patterns establish elsewhere, Persistent would implement a Drop trait that panics to prevent user error.

impl<T> Drop for Persistent<T> {
    fn drop(&mut self) {
       panic!("Persistent must be manually dropped.");
    }
}

Alternative

An alternative design is to provide the finalize method as an argument to the constructor instead of as a trait.

impl<T: Send + 'static> JsBox<T> {
    pub fn with_finalizer<'a, C>(
        cx: &mut C,
        value: T,
        finalizer: fn(FinalizerContext, T),
    ) -> Handle<'a, JsBox<T>>
    where
        C: Context<'a>,
}

The advantage of the trait approach is that it co-locates the finalizer with the data. The disadvantage is that Neon would not prevent a user from defining a trait, but failing to use the with_finalizer constructor.

In my opinion the trait approach is preferred and the ergonomics could be improved in and when the Rust specialization RFC is implemented.

Example

struct MyServer {
    server: Server,
    // Wrapping the `Persistent` in a `ManuallyDrop` provides two advantages:
    // 1. Immediately signals that something in this struct needs special attention
    // 2. If a manual `drop` is missed, it causes a leak instead of a `panic`
    callback: ManuallyDrop<Persistent<JsFunction>>,
}

impl Finalize for MyServer {
    fn finalize(self, cx: FinalizeContext) {
        self.callback.drop(&mut cx);
    }
}

fn create_server(cx: FunctionContext) -> JsResult<JsBox<MyServer>> {
    let callback = cx.argument::<JsFunction>(0)?.persistent(&mut cx);
    let server = MyServer {
        server: Server::new(),
        callback: ManuallyDrop::new(callback),
    };

    Ok(JsBox::with_finalizer(server))
}
kjvalencik commented 4 years ago

Merged in https://github.com/neon-bindings/rfcs/commit/951e6257f1766576a5f20f93219ea8c40c9ff931