Threadsafe Handles #32

kjvalencik · 2020-07-28T20:19:37Z

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.

text/0000-thread-safe-handles.md

+fn log(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let persistent_cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);
+
+    // `Persistent` are reference counted and clone-able.


text/0000-thread-safe-handles.md

+
+```rust
+fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);


text/0000-thread-safe-handles.md

+- ~A global `EventQueue` is necessary for dropping `Persistent`. Should this be exposed?~ No. This is no longer required for the design.
+- ~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?~ This is no longer required for the design.
+- 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?


text/0000-thread-safe-handles.md

+
+Two new `Send` and `Sync` features are introduced: `EventQueue: Send + Sync` and `Persistent: Send`. 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.


text/0000-thread-safe-handles.md

+Two new `Send` and `Sync` features are introduced: `EventQueue: Send + Sync` and `Persistent: Send`. 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, with inner contents only accessible on the main thread.


text/0000-thread-safe-handles.md

+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);
+
+    std::thread::spawn(move || {
+        // Panics when `cb` is dropped.


text/0000-thread-safe-handles.md

+}
+```
+
+While `Persistent<_>` may be shared across threads, the inner contents can only be accessed on the main thread.


text/0000-thread-safe-handles.md

+
+### `neon::handle::EventQueue`
+
+Once a value is wrapped in a `Persistent<_>` handle, it must be sent back to the main JavaScript thread to unwrap. `EventQueue` provides a mechanism for requesting work be peformed on the main thread.


text/0000-thread-safe-handles.md

+```rust
+fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);
+    let queue = EventQueue::new(&mut cx)?;


text/0000-thread-safe-handles.md

+}
+```
+
+`Persistent<_>` are clone-able and can be used many times. `EventQueue` are `Sync` and may be shared across threads.


text/0000-thread-safe-handles.md

+
+```rust
+fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {
+    let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);


text/0000-thread-safe-handles.md

+    let queue = Arc::new(EventQueue::new(&mut cx)?);
+
+    let handles = (1..=10).into_iter().map(|i| {
+        let queue = Arc::clone(queue);


text/0000-thread-safe-handles.md

+
+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.


text/0000-thread-safe-handles.md

+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.


text/0000-thread-safe-handles.md

+
+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.


dherman · 2020-09-26T22:08:31Z

Finished my review! This is going to be a great addition to the core primitives of Neon. I'm psyched.

text/0000-thread-safe-handles.md

+
+    /// Prevent the Node event loop from existing while this `EventQueue` exists. (Default)
+    /// _Idempotent_
+    pub fn reference(&mut self);


text/0000-thread-safe-handles.md

+
+Both `ref` and `unref` 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`.
+
+#### Future Expansion


text/0000-thread-safe-handles.md

feat: Implements Threadsafe Handles as specified by neon-bindings/rfcs#32 Requires the `event-queue-api` feature flag until the RFC is accepted.

dherman

There's just a couple things out of date now, but otherwise I think this is probably ready for Final Comment Period.

text/0000-thread-safe-handles.md

kjvalencik · 2021-09-03T15:13:43Z

@dherman This is close to stabilization. One change I would like to make prior to merging is the addition of a return value. Two APIs to take inspiration from:

In both of these APIs, a JoinHandle is returned. On a thread::spawn, a user can call join to block until the thread exits and get the return value. Similarly, in tokio, the handle can be awaited to get the resolved value.

I propose the following changes:

struct Channel
    pub fn send<T, F>(&self, f: F) -> JoinHandle<T>
    where
        T: Send + 'static,
        F: FnOnce(TaskContext) -> NeonResult<T>;

We don't need to initially do anything with the returned JoinHandle, but by adding it there, it allows extension without breaking changes to introduce JoinHandle::join and impl Future for JoinHandle.

Also, I'm still not feeling certain send is the right choice. Maybe it should be renamed spawn? If it is renamed spawn, Channel probably isn't the best name either.

dherman · 2021-09-03T17:04:39Z

This is a great idea @kjvalencik.

I think I do like send better than spawn, though, because we aren't spawning a thread (not even conceptually like a green thread), the user should be thinking of it as sending information back to an existing thread. As far as mixing traditions (channel / join) I think I'm comfortable with it: conceptually we're dealing with a channel because we're communicating between threads, and conceptually we're dealing with joining because we're blocking on another thread's computation.

Part of neon-bindings/rfcs#32

dherman

The JoinHandle changes look good!

Part of neon-bindings/rfcs#32

kjvalencik · 2022-03-07T21:53:10Z

Merged in c639681

kjvalencik force-pushed the kv/threadsafe-handles branch from 44da5af to fdfa972 Compare August 24, 2020 21:09

kjvalencik mentioned this pull request Sep 24, 2020

Having some trouble using rusty_v8 neon-bindings/neon#603

Closed

kjvalencik commented Sep 25, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

fn log(mut cx: FunctionContext) -> JsResult<JsUndefined> {

let persistent_cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);

// `Persistent` are reference counted and clone-able.

This comment was marked as resolved.

Sign in to view

kjvalencik commented Sep 25, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

```rust

fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {

let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);

This comment was marked as resolved.

Sign in to view

kjvalencik commented Sep 25, 2020

View reviewed changes

dherman reviewed Sep 26, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);

std::thread::spawn(move || {

// Panics when `cb` is dropped.

This comment was marked as resolved.

Sign in to view

dherman reviewed Sep 26, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

}

```

While `Persistent<_>` may be shared across threads, the inner contents can only be accessed on the main thread.

This comment was marked as resolved.

Sign in to view

dherman reviewed Sep 26, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

```rust

fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {

let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);

let queue = EventQueue::new(&mut cx)?;

This comment was marked as resolved.

Sign in to view

dherman reviewed Sep 26, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

}

```

`Persistent<_>` are clone-able and can be used many times. `EventQueue` are `Sync` and may be shared across threads.

This comment was marked as resolved.

Sign in to view

dherman reviewed Sep 26, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

```rust

fn thread_callback(mut cx: FunctionContext) -> JsResult<JsUndefined> {

let cb = cx.argument::<JsFunction>(0)?.persistent(&mut cx);

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

dherman reviewed Sep 26, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

let queue = Arc::new(EventQueue::new(&mut cx)?);

let handles = (1..=10).into_iter().map(|i| {

let queue = Arc::clone(queue);

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

dherman reviewed Sep 26, 2020

View reviewed changes

kjvalencik commented Oct 2, 2020

View reviewed changes

text/0000-thread-safe-handles.md Outdated

/// Prevent the Node event loop from existing while this `EventQueue` exists. (Default)

/// _Idempotent_

pub fn reference(&mut self);

This comment was marked as resolved.

Sign in to view

kjvalencik commented Oct 2, 2020

View reviewed changes

kjvalencik commented Oct 5, 2020

View reviewed changes

text/0000-thread-safe-handles.md Show resolved Hide resolved

goto-bus-stop changed the base branch from master to main October 8, 2020 13:46

kjvalencik force-pushed the kv/threadsafe-handles branch from be6c27a to 81f2bab Compare October 9, 2020 19:22

Threadsafe Handles

7fcc8af

kjvalencik force-pushed the kv/threadsafe-handles branch from 81f2bab to 7fcc8af Compare October 9, 2020 19:45

This was referenced Nov 5, 2020

Threadsafe Handles neon-bindings/neon#622

Merged

Promise docs examples do not run on background thread neon-bindings/neon#632

Closed

dherman mentioned this pull request Dec 4, 2020

RFC: TaskBuilder #30

Closed

Move EventQueue to task

aab766e

kjvalencik added a commit to neon-bindings/neon that referenced this pull request Jan 5, 2021

Threadsafe Handles (#622)

4730ab7

feat: Implements Threadsafe Handles as specified by neon-bindings/rfcs#32 Requires the `event-queue-api` feature flag until the RFC is accepted.

dherman requested changes May 1, 2021

View reviewed changes

text/0000-thread-safe-handles.md Outdated Show resolved Hide resolved

text/0000-thread-safe-handles.md Show resolved Hide resolved

Review feedback

830be2c

kjvalencik added the final comment period last opportunity to discuss the proposed conclusion label May 5, 2021

dherman mentioned this pull request May 18, 2021

Rename EventQueue to Channel neon-bindings/neon#734

Closed

kjvalencik removed the final comment period last opportunity to discuss the proposed conclusion label Jul 2, 2021

kjvalencik added a commit to neon-bindings/neon that referenced this pull request Sep 3, 2021

feat(neon): Add JoinHandle as a result of Channel::send

07632a1

Part of neon-bindings/rfcs#32

kjvalencik mentioned this pull request Sep 3, 2021

feat(neon): Add JoinHandle as a result of Channel::send neon-bindings/neon#787

Merged

kjvalencik added a commit to neon-bindings/neon that referenced this pull request Sep 3, 2021

feat(neon): Add JoinHandle as a result of Channel::send

7a90ca0

Part of neon-bindings/rfcs#32

kjvalencik added a commit to neon-bindings/neon that referenced this pull request Sep 3, 2021

feat(neon): Add JoinHandle as a result of Channel::send

16ee297

Part of neon-bindings/rfcs#32

kjvalencik added 2 commits September 3, 2021 17:17

Rename EventQueue to Channel

de05c05

Add information about TSFN re-use

0314940

kjvalencik mentioned this pull request Sep 3, 2021

Shared ThreadsafeFunction in EventQueue RFC #42

Closed

Add information about JoinHandle

a34dc1b

kjvalencik requested a review from dherman September 3, 2021 21:36

kjvalencik added the final comment period last opportunity to discuss the proposed conclusion label Sep 3, 2021

dherman approved these changes Sep 10, 2021

View reviewed changes

kjvalencik added a commit to neon-bindings/neon that referenced this pull request Sep 14, 2021

feat(neon): Add JoinHandle as a result of Channel::send

390877f

Part of neon-bindings/rfcs#32

This was referenced Sep 15, 2021

Ability to call Javascript methods from Rust threads neon-bindings/neon#197

Closed

Support uv_async_send for background task notifications neon-bindings/neon#228

Closed

kjvalencik closed this Mar 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Threadsafe Handles #32

Threadsafe Handles #32

kjvalencik commented Jul 28, 2020

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

dherman commented Sep 26, 2020

This comment was marked as resolved.

This comment was marked as off-topic.

dherman left a comment

kjvalencik commented Sep 3, 2021

dherman commented Sep 3, 2021

dherman left a comment

kjvalencik commented Mar 7, 2022


		Two new `Send` and `Sync` features are introduced: `EventQueue: Send + Sync` and `Persistent: Send`. 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.


		### `neon::handle::EventQueue`

		Once a value is wrapped in a `Persistent<_>` handle, it must be sent back to the main JavaScript thread to unwrap. `EventQueue` provides a mechanism for requesting work be peformed on the main thread.


		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.


		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.


		Both `ref` and `unref` 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`.

		#### Future Expansion

Threadsafe Handles #32

Threadsafe Handles #32

Conversation

kjvalencik commented Jul 28, 2020

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

dherman commented Sep 26, 2020

This comment was marked as resolved.

This comment was marked as off-topic.

dherman left a comment

Choose a reason for hiding this comment

kjvalencik commented Sep 3, 2021

dherman commented Sep 3, 2021

dherman left a comment

Choose a reason for hiding this comment

kjvalencik commented Mar 7, 2022