neon-bindings / rfcs

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

Threadsafe Handles #32

Closed kjvalencik closed 2 years ago

kjvalencik commented 4 years ago

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.

dherman commented 4 years ago

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

kjvalencik commented 3 years ago

@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 commented 3 years ago

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.

kjvalencik commented 2 years ago

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