RFC: Generic function handles

[michiel-de-muynck]

Currently, if you have a closure and you want to call it from JavaScript, you can do that very easily by just using the closure in js! as if it's any other value. That's great. However, every time you do that you have to remember to call .drop() on JavaScript object that represents the closure, because if you don't, you have a memory leak.

Since JavaScript doesn't have destructors or any other way to run code when an object is garbage collected, it can sometimes be quite difficult to write your JavaScript code in such a way that the .drop() is called at the right time. It's also very easy to forget to call .drop(), as passing a rust closure to JavaScript looks the same as passing any other rust value (which don't need to have any .drop() called on them).

A solution that is used in many parts of the stdweb api (and one that I've also used in my own stdweb projects) is to make "handle" structs that automatically call .drop() when the handle gets dropped. Examples are EventListenerHandle, MutationObserverHandle, RequestAnimationFrameHandle, DoneHandle, etc. If you just keep these handles alive for as long as you expect to receive callbacks, everything works great and you won't get any memory leaks.

If you have to do extra work when dropping the handle (as is the case in the above examples: they also call removeEventListener(), disconnect(), cancelAnimationFrame(), etc.) then a custom handle struct for each situation is unavoidable. But if all you want to do is call .drop() on a closure, then you end up writing handles that differ in basically only their name. It might be good if stdweb offered a "generic" function handle that can be used for this purpose.

You would use it like this:

// just a regular rust closure
let f = || {
    println!("Hello world!");
};

// convert it into a handle (this serializes the closure to javascript)
let handle = f.into_js_handle();

js! {
    // This handle can now be used in js! without risk of memory leaks.
    // Not by value (because if the handle gets *moved* here it would immediately get dropped),
    // but by reference.
    @{&handle}();

    // no need to remember to call .drop() on anything
}

// when handle goes out of scope, f is dropped
// alternatively, they can be leaked, in which case .drop() is never called
handle.leak();

What do you think? Is this a good idea? Or is this something that's better suited for an independent crate?

There are also some "bikeshedding" considerations:

• What should the struct be called? FnHandle, GenericFunctionHandle, etc. Many possibilities. Should it have type parameters representing the arguments and return value? If so, we can implement a call() method for these handles that calls the closure (but it would be pretty inefficient because the handle only stores the JS reference, so despite the fact that you're calling a rust closure from rust, the arguments and return value need to be serialized and deserialized to JS). If the handle doesn't have type parameters, it may be slightly more ergonomic to store and move around.

• What should the syntax be for converting closures into handles? In the example above, I used let handle = f.into_js_handle();. The downside of this is that the trait (presumably IntoJsHandle) needs to be in scope before this can be used, and stdweb doesn't have a "prelude" like many other libraries do. We could also use a macro like let handle = get_fn_handle!(f); or something else. A normal function is unfortunately not possible I think.

• What should the syntax be for using a handle in js!? In my example, I just used a reference to the handle (@{&handle}()), but we could also give handles a function like get_reference() that returns the stored Reference or something.

• Is all of this even worth it?

[Pauan]

I've thought about this before, I like it. Even though it's only useful in certain situations, it's still useful.

If so, we can implement a call() method for these handles that calls the closure

I don't like this idea, as you said it's pretty inefficient.

stdweb doesn't have a "prelude" like many other libraries do.

It does actually, it's use stdweb::traits::*;

A normal function is unfortunately not possible I think.

I think it would probably have to be a macro. Something like this:

#[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
pub struct FnHandle( Reference );

impl FnHandle {
    #[inline]
    pub fn new( reference: Reference ) -> DiscardOnDrop< Self > {
        DiscardOnDrop::new( FnHandle( reference ) )
    }
}

impl Discard for FnHandle {
    #[inline]
    fn discard( self ) {
        js! { @(no_return)
            @{&self.0}.drop();
        }
    }
}

macro_rules! new_fn_handle {
    ( $f:expr ) => {
        // This can probably be implemented a lot more efficiently (without the js! macro)
        $crate::FnHandle::new( js!( return @{$f}; ).try_into().unwrap() )
    };
}

Unfortunately this doesn't have any static type checking (any JsSerialize expression is accepted).

What should the syntax be for using a handle in js!?

Since FnHandle is a ReferenceType, it should be possible to pass it directly to js! (as you said, using @{&handle})

[koute]

Hmm.... I would have to think about this a little harder to be able to give a conclusive answer, but it seems like a worthwhile idea!

Should it have type parameters representing the arguments and return value?

Hmm... I would probably prefer to have both - typed, and untyped, where the typed one would be the default but can be easily converted into the untyped. (I'm thinking of reworking our Promise bindings in such a way, e.g. to use in the WebMIDI APIs.)

If so, we can implement a call() method for these handles that calls the closure (but it would be pretty inefficient because the handle only stores the JS reference, so despite the fact that you're calling a rust closure from rust, the arguments and return value need to be serialized and deserialized to JS)

Not necessarily; we don't have to go through JS to call it - we'd just need to keep the pointer to the boxed function on Rust's side.

What should the syntax be for converting closures into handles?

Probably through a trait, or through a static method? (I think it might be possible to have a function which constructs such a handle, since it doesn't have to handle everything like the js! macro but only the closures. But don't quote me on this. I may be wrong.)

What should the syntax be for using a handle in js!?

Definitely passed by reference.

[Pauan]

I think it might be possible to have a function which constructs such a handle, since it doesn't have to handle everything like the js! macro but only the closures.

The problem is: how do you write a function which accepts closures where the number of arguments for the closure (and the type of each argument, and the type of the return value) varies? The same problem arises when trying to add typed generics toFnHandle.

[michiel-de-muynck]

Indeed, as @Pauan said it's not possible to have one static function that accepts closures with any number (and types) of arguments. However, I've found an alternative method that I like a lot: just use From and Into. If we impl From<F> for FnHandle where F: Fn(...) then converting closures to FnHandles is as simple as FnHandle::from(f) or even just f.into() (may need to help type inference with that one). The additional benefit of From/Into is that those traits are pretty much always in scope, as they're part of Rust's prelude.

I've started with an initial implementation (here's the commit). I've implemented it with both an untyped (GenericFnHandle) and typed versions (FnHandle<Args,Output>, FnMutHandle<Args,Output>, and FnHandle<Args,Output>). The latter can be converted into the former (also using From or Into). Unfortunately, rust doesn't seem to allow a conversion from closures directly to GenericFnHandle because of this "issue".

The one thing that I haven't implemented is any call() methods, if we want those. It's true what @koute says: we could implement these without serializing/deserializing the arguments through javascript by keeping a hold of the pointer to the Rust closure. But it won't be easy. The current serialization code doesn't really hand out its internal pointers. JsSerializeOwned goes from a value directly to a SerializedValue, so we'd need to either dig that pointer back out of the SerializedValue or just not use JsSerializeOwned at all when creating the FnHandle.

But there's another issue: while we can keep a hold of a pointer to the rust closure, we don't know if it's still valid or not (at least in my current implementation, @{&handle} is just like any other closure and can be .drop()ed). Whether that pointer is still valid or not is kept track of in JavaScript in a way that's pretty hard for Rust - or any outside code - to get at (it's valid if some function-scoped var pointer is not 0). This also means that you wouldn't be able to just call() the closure from the handle. It needs to be try_call().

The issues in the above 2 paragraphs can be solved by serializing and handling FnHandles completely differently from regular closures, but that doesn't seem worth it. Calling the closures directly would be a pretty niche use case of FnHandle, and FnHandle is a pretty niche to begin with.