Threadsafe Handles

kjvalencik · kjvalencik · commit fdfa972ae083 · 2020-08-24T17:08:49.000-04:00
diff --git a/text/0000-thread-safe-handles.md b/text/0000-thread-safe-handles.md
@@ -0,0 +1,274 @@
+- Feature Name: Threadsafe Handles
+- Start Date: 2020-07-21
+- RFC PR: (leave this empty)
+- Neon Issue: (leave this empty)
+
+# Summary
+[summary]: #summary
+
+Two new `Send + Sync` features are introduced: `EventQueue` and `Persistent`. These features work together to allow multi-threaded modules to interact with the JavaScript main thread.
+
+* `EventQueue`: Threadsafe handle that allows sending closures to be executed on the main thread.
+* `Persistent<T>`: Opaque handle that can be sent across threads, but only dereferenced on the main thread.
+
+# Motivation
+[motivation]: #motivation
+
+High level event handlers have gone through [several](https://github.com/neon-bindings/rfcs/pull/25) [iterations](https://github.com/neon-bindings/rfcs/pull/28) without quite providing [ideal](https://github.com/neon-bindings/rfcs/issues/31) ergonomics or [safety](https://github.com/neon-bindings/neon/issues/551).
+
+The Threadsafe Handlers feature attempts to decompose the high level `EventHandler` API into lower level, more flexible primitives.
+
+These primitives can be used to build a higher level and _safe_ `EventHandler` API allowing experimentation with patterns outside of `neon`.
+
+# Guide-level explanation
+[guide-level-explanation]: #guide-level-explanation
+
+Neon provides a smart pointer type `Handle<'a, T: Value>` for referencing values on the JavaScript heap. These references are bound by the lifetime of `Context<'a>` to ensure they can only be used while the VM is locked in a synchronous neon function. These are neither `Send` or `Sync`.
+
+### `neon::handle::Persistent<T>`
+
+As a developer, I may want to retain a reference to a JavaScript value while returning control back to the VM.
+
+`Handle<'_, _>` may not outlive the context that created them.
+
+```rust
+// Does not compile because `cb` cannot be sent across threads
+fn thread_log(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?;
+
+    std::thread::spawn(move || {
+        println!("{:?}", cb);
+    });
+
+    cx.undefined()
+}
+```
+
+However, a persistent reference may be created from a `Handle<'_, _>` which can be sent across threads.
+
+```rust
+// Compiles
+fn thread_log(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx)?;
+
+    std::thread::spawn(move || {
+        println!("{:?}", cb);
+    });
+
+    cx.undefined()
+}
+```
+
+While `Persistent<_>` may be shared across threads, they can only be dereferenced on the main JavaScript thread. This is controlled by requiring a `Context<_>` to dereference.
+
+```rust
+fn log(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let persistent_cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx)?;
+    let cb = persistent_cb.deref(&cx);
+
+    println!("{:?}", cb);
+
+    cx.undefined()
+}
+```
+
+### `neon::handle::EventQueue`
+
+Once a value is wrapped in a `Persistent<_>` handle, it must be sent back to the main JavaScript thread to dereference. `EventQueue` provides a mechanism for requesting work be peformed on the main thread.
+
+To schedule work, send a closure to the event queue:
+
+```rust
+fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx)?;
+    let queue = EventQueue::new(&mut cx)?;
+
+    std::thread::spawn(move || {
+        queue.send(move |mut cx| {
+            let this = cx.undefined();
+            let msg = cx.string("Hello, World!");
+            let cb = cb.deref(&cx).unwrap();
+
+            cb.call(&mut cx, this, vec![msg]).unwrap();
+        });
+    });
+
+    cx.undefined()
+}
+```
+
+`Persistent<_>` and `EventQueue` are clone-able and can be used many times.
+
+```rust
+fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx)?;
+    let queue = EventQueue::new(&mut cx)?;
+
+    for i in 1..=10 {
+        let queue = queue.clone();
+        let cb = cb.clone();
+
+        std::thread::spawn(move || {
+            queue.send(move |mut cx| {
+                let this = cx.undefined();
+                let msg = cx.string(format!("Count: {}", i));
+                let cb = cb.deref(&cx).unwrap();
+
+                cb.call(&mut cx, this, vec![msg]).unwrap();
+            });
+        });
+    }
+
+    cx.undefined()
+}
+```
+
+Instances of `EventQueue` will keep the event loop running and prevent the process from exiting. The `EventQueue::unref` method is provided to change this behavior and allow the process to exit while an instance of `EventQueue` still exists.
+
+However, calls to `EventQueue::schedule` _might_ not execute before the process exits.
+
+```rust
+fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx)?;
+    let mut queue = EventQueue::new(&mut cx)?;
+
+    queue.unref();
+
+    std::thread::spawn(move || {
+        std::thread::sleep(std::time::Duration::from_secs(1));
+
+        // If the event queue is empty, the process may exit before this executes
+        queue.send(move |mut cx| {
+            let this = cx.undefined();
+            let msg = cx.string("Hello, World!");
+            let cb = cb.deref(&cx).unwrap();
+
+            cb.call(&mut cx, this, vec![msg]).unwrap();
+        });
+    });
+
+    cx.undefined()
+}
+```
+
+# Reference-level explanation
+[reference-level-explanation]: #reference-level-explanation
+
+### `neon::handle::Persistent<T: Object>`
+
+`Persistent` handles are references to objects on the v8 heap that prevent objects from being garbage collected. If objects are moved, the internal pointer will be updated.
+
+_`Persistent` handles may only be used for `Object` and `Function` types. This is a limitation of `n-api`._
+
+```rust
+/// `Send + Sync` Persistent handle to JavaScript objects
+impl Persistent<T: Object> {
+    pub fn new<'a, C: Context<'a>>(
+        cx: &mut C,
+        v: Handle<T>,
+    ) -> NeonResult<Self>;
+
+    pub fn deref<'a, C: Context<'a>>(
+        self,
+        cx: &mut C,
+    ) -> JsResult<'a, T>;
+}
+```
+
+#### Design Notes
+
+`Persistent` utilize an atomic reference count to track when they can be safely dropped. There are two circumstances where a `Persistent` may be dropped:
+
+* `Persistent` is no longer held. For example, if it is a member of a struct that is dropped.
+* `Persistent::deref` is called and it decrements the count to zero.
+
+`Persistent::deref` takes ownership of `self` to allow an optimization in the most common usage. In the case of a `Persistent` being dropped after a call to `deref`, neon may immediately mark the n-api reference for garbage collection while the main thread is still available.
+
+In the event that a `Persistent` is dropped when not executing on the main thread, the destruction must be scheduled by a global `EventQueue`.
+
+### `neon::handle::EventQueue`
+
+```rust
+impl EventQueue {
+    /// Creates an unbounded queue
+    pub fn new<'a, C: Context<'a>>(cx: &mut C) -> Self;
+
+    /// Creates a bounded queue
+    pub fn with_capacity<'a, C: Context<'a>>(cx: &mut C, size: usize) -> Self;
+
+    /// Decrements the strong count on the underlying threadsafe function.
+    /// When the count reaches zero, the event loop will not be kept running.
+    /// An unreferenced `EventQueue` may not execute submitted functions.
+    /// _Idempotent_
+    pub fn unref(&mut self);
+
+    /// Increments the strong count on the underlying function.
+    /// If the count is greater than zero, the event loop will be kept running.
+    /// _Idempotent_
+    pub fn ref(&mut self);
+
+    /// Schedules a closure to execute on the main JavaScript thread
+    /// Blocks if the event queue is full
+    pub fn send(
+        &self,
+        f: impl FnOnce(TaskContext) -> NeonResult<()>,
+    ) -> Result<(), EventQueueError>;
+
+    /// Schedules a closure to execute on the main JavaScript thread
+    /// Non-blocking
+    pub fn try_send(
+        &self,
+        f: impl FnOnce(TaskContext) -> NeonResult<()>,
+    ) -> Result<(), EventQueueError>;
+}
+```
+
+The native `napi_call_threadsafe_function` can fail in three ways:
+
+* `napi_queue_full` if the queue is full
+* `napi_invalid_arg` if the thread count is zero. This is due to misuse and statically enforced by ownership rules and the `Drop` trait.
+* `napi_generic_failure` if the call to `uv_async_send` fails
+
+These three failures may be reduced to two and are represented by the `EventQueueError<F>` enum.
+
+```rust
+enum EventQueueErrpub trait JsResultExt<'a, V: Value> {
+    fn or_throw<'b, C: Context<'b>>(self, cx: &mut C) -> JsResult<'a, V>;
+}a generic failure. Most likely on `uv_async_send(&async)`
+    Unknown,
+}
+```
+
+In the event of an `napi_invalid_arg` error, neon will `panic` as this indicates a bug in neon and not an expected error condition.
+
+#### Design Notes
+
+The backing `napi_threadsafe_function` is already `Send` and `Sync`, allowing the use of `send` with a shared reference.
+
+It is possible to implement `Clone` for `EventQueue` using `napi_acquire_threadsafe_function`; however, it was considered more idiomatic and flexible to leave this to users with Rust reference counting. E.g., `Arc<EventQueue>`.
+
+Both `deref` and `ref` mutate the behavior of the underlying threadsafe function and require exclusive references. These methods are infallible because Rust is capable of statically enforcing the invariants. We may want to optimize with `assert_debug!` on the `napi_status`.
+
+# Drawbacks
+[drawbacks]: #drawbacks
+
+![standards](https://imgs.xkcd.com/comics/standards.png)
+
+Neon already has `Task`, `EventHandler`, a [proposed](https://github.com/neon-bindings/rfcs/pull/30) `TaskBuilder` and an accepted, but currently unimplemented, [update](https://github.com/neon-bindings/rfcs/pull/28) to `EventHandler`. This is a large amount of API surface area without clear indication of what a user should use.
+
+This can be mitigated with documentation and soft deprecation of existing methods as we get a clearer picture of what a high-level, ergonomic API would look like.
+
+# Rationale and alternatives
+[alternatives]: #alternatives
+
+These are fairly thin RAII wrappers around N-API primitives. The most compelling alternatives are continuing to improve the existing high level APIs. No other designs were considered.
+
+# Unresolved questions
+[unresolved]: #unresolved-questions
+
+- ~Should this be implemented for the legacy runtime or only n-api?~ Only for `n-api`.
+- ~Should `EventQueue` be `Arc` wrapped internally to encourage users to share instances across threads?~ No. This can be covered in documentation and matches the abstraction of neon's proposed [`JsBox`](https://github.com/neon-bindings/rfcs/pull/33) and [`tokio::runtime::Runtime`](https://docs.rs/tokio/0.1.22/tokio/runtime/struct.Runtime.html).
+- ~A global `EventQueue` is necessary for dropping `Persistent`. Should this be exposed?~ No. This can be a follow-up RFC.
+- The global `EventQueue` requires instance data. Are we okay using an [experimental](https://nodejs.org/api/n-api.html#n_api_environment_life_cycle_apis) API? We can also use `thread_local` and an `unref` `EventQueue`.
+- Should `EventQueue::send` accept a closure that returns `()` instead of `NeonResult<()>`? In most cases, the user will want to allow `Throw` to become an `uncaughtException` instead of a `panic` and `NeonResult<()>` provides a small ergonomics improvement.
+- `persistent(&mut cx)` is a little difficult to type. Should it have a less complicated name?
