neon-bindings / rfcs

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

Add a persistent handle type. #13

Closed abrodersen closed 4 years ago

abrodersen commented 6 years ago

Rendered

Create a persistent handle type that allows native Rust data structures to hold references to Node.js objects.

dherman commented 6 years ago

@abrodersen I should've thanked you right away when you submitted this, I just went into chewing-on-it mode right away and forgot to respond. This RFC is really intriguing!

Here are my initial thoughts and questions:

Thanks so much for this work! I'm very excited to see where it goes.

abrodersen commented 6 years ago

Hello @dherman, and thank you for the feedback!

The most concrete example I can think of is the one I'm using Neon for right now: Essentially, I have a Rust library that reads in data from a stream and performs some transforms on it. I want to integrate this library into an existing Node.js program. Because of our requirements, the Rust library must be able to pipe the data from a Node.js stream and do it asynchronously. After multiple iterations of prototyping, I finally found the (currently) only feasible design: run the Rust library code in a background task. When it needs input, it sends a signal to a dispatcher task. The dispatcher invokes a callback, passing it a continuation function (using the new closures 😉) that sends a response signal back to the library. The library then uses the data to perform its transformations and return the results. This design allows the callback implementation to asynchronously read all necessary data and send the results back to the library.

I have been working on this problem for a while, and as far as I can tell, it's the most workable solution. This design requires a persistent handle implementation, as the current Task API requires all referenced data to be stored in the implementing struct. Since this struct is moved when calling schedule, it cannot contain scoped Handle values. I need a reference the the callback function to be stored in the dispatcher task struct, because I want to reschedule the task after the callback has been invoked.

Yes, it appears the Drop trait should be effective in preventing memory leaks. 😃

There shouldn't be any atomic operations involved. The PersistentHandle type will simply hold a pointer to a heap-allocated native reference. The PersistentHandle can be moved around safely without any atomic operations. When we want to interact with the underlying value, the PersistentHandle must be moved (or cloned) to the main thread where we have access to a Scope reference. There it can be consumed into a local Handle reference and safely interacted with normally.

rebo commented 6 years ago

Hi,

I have created an example app showing how PersistentHandles can be used to reference Node.js objects in callbacks from a Neon Task.

A MainBackgroundTask signals a DispatchTask to return data from Node.js when it needs it.

https://github.com/rebo/async-neon-example

dherman commented 4 years ago

It looks like #25 really found the right abstraction level for being able to do background work in Rust threads—thanks in no small part to N-API demonstrating a good design. Thanks for this contribution and for helping us understand the design space!