Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(neon): API for thread-local data #902

Merged
merged 33 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
94111d4
Add `neon::instance::Global` API for storing instance-global data.
dherman May 24, 2022
f872b76
Address review comments:
dherman May 25, 2022
6f12663
Use `'cx` as the inner lifetime of contexts.
dherman May 25, 2022
18e60f1
Get rid of `[]` overloading for `GlobalTable` in favor of an inherent…
dherman May 25, 2022
b8b089b
Immutable version of API:
dherman May 25, 2022
cf0d32a
Explicitly name the types in the transmute
dherman May 25, 2022
f5cd0ba
Explicitly name the types in the transmute.
dherman May 25, 2022
aee77f4
Add `get_or_try_init` and `get_or_init_default`
dherman May 27, 2022
9075b76
Protect re-entrant cases with "dirty" state checking
dherman Jun 1, 2022
b28f439
Use `GlobalCellValue` shorthand in type definition of `GlobalCell`.
dherman Jun 1, 2022
bbc4f22
Prettier fixups
dherman Jun 1, 2022
ff75d2d
Add a test for storing rooted objects in instance globals
dherman Jun 1, 2022
d9b8251
Minor style cleanup for `TryInitTransaction::is_trying()`
dherman Jun 1, 2022
a66e511
Global::new() can use the derived Default::default()
dherman Jun 2, 2022
507332d
Undo previous commit, which failed to type-check.
dherman Jun 2, 2022
0696901
Test that inner closure isn't even executed on re-entrant initializat…
dherman Jun 2, 2022
574877b
Make `get_or_try_init` generic for any `Result<T, E>`
dherman Jun 2, 2022
b397e2d
Change "safety" to "unwrap safety" for the `.unwrap()` comment.
dherman Jun 2, 2022
bc7d09e
Use `get_or_try_init` for the global object test, to only root the ob…
dherman Jun 2, 2022
3a0c041
Rename `Global` to `Local` and add top-level API docs for the `neon::…
dherman Jun 3, 2022
3338877
Improvements to `neon::instance` top-level API docs
dherman Jun 8, 2022
42eec50
Rename `neon::instance` to `neon::thread` and `Local` to `LocalKey`
dherman Jun 8, 2022
85d99f5
Some more documentation text about the relationships between instance…
dherman Jun 8, 2022
c906fbc
Addresses some of @kjvalencik's review suggestions:
dherman Jun 9, 2022
0f57620
Clarify doc text
dherman Jun 9, 2022
14a2fe4
Idiomatic Rust variable name in doc example
dherman Jun 9, 2022
019c2d3
Link to `neon::main` docs in doc comment
dherman Jun 9, 2022
58f80b3
Clarifying doc text about cross-thread sharing
dherman Jun 9, 2022
f3cc0ab
s/fail/panic/ in doc text
dherman Jun 9, 2022
2005e61
More docs improvements:
dherman Jun 9, 2022
d50ca63
Link to `crate::main` in the docs instead of `neon::main`
dherman Jun 9, 2022
ece0c02
More copy editing in the docs
dherman Jun 9, 2022
310cb5b
A few last copy-editing nits
dherman Jun 9, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/neon/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ nodejs-sys = "0.13.0"
libloading = "0.7.3"
semver = "0.9.0"
smallvec = "1.4.2"
once_cell = "1.10.0"
neon-macros = { version = "=0.10.1", path = "../neon-macros" }

[features]
Expand Down
72 changes: 72 additions & 0 deletions crates/neon/src/instance/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
use std::any::Any;
use std::marker::PhantomData;
use std::sync::atomic::{AtomicUsize, Ordering};

use once_cell::sync::OnceCell;

use crate::context::Context;
use crate::lifecycle::InstanceData;

static COUNTER: AtomicUsize = AtomicUsize::new(0);

fn next_id() -> usize {
COUNTER.fetch_add(1, Ordering::SeqCst)
kjvalencik marked this conversation as resolved.
Show resolved Hide resolved
}

/// A cell that can be used to allocate data that is global to an instance
/// of a Neon addon.
pub struct Global<T> {
_type: PhantomData<T>,
id: OnceCell<usize>,
}

impl<T> Global<T> {
pub const fn new() -> Self {
Self {
_type: PhantomData,
id: OnceCell::new(),
}
dherman marked this conversation as resolved.
Show resolved Hide resolved
}

fn id(&self) -> usize {
*self.id.get_or_init(next_id)
}
}

impl<T: Any + Send> Global<T> {
pub fn borrow<'a, 'b, C>(&self, cx: &'b mut C) -> Option<&'b T>
dherman marked this conversation as resolved.
Show resolved Hide resolved
dherman marked this conversation as resolved.
Show resolved Hide resolved
where
C: Context<'a>,
{
InstanceData::globals(cx)[self.id()]
.as_ref()
.map(|boxed| boxed.downcast_ref().unwrap())
}

pub fn borrow_mut<'a, 'b, C>(&self, cx: &'b mut C) -> Option<&'b mut T>
dherman marked this conversation as resolved.
Show resolved Hide resolved
where
C: Context<'a>,
{
InstanceData::globals(cx)[self.id()]
.as_mut()
.map(|boxed| boxed.downcast_mut().unwrap())
}

pub fn set<'a, 'b, C>(&self, cx: &'b mut C, v: T)
where
C: Context<'a>,
{
InstanceData::globals(cx)[self.id()] = Some(Box::new(v));
}
}

impl<T: Any + Send + Clone> Global<T> {
pub fn get<'a, C>(&self, cx: &mut C) -> Option<T>
dherman marked this conversation as resolved.
Show resolved Hide resolved
where
C: Context<'a>,
{
InstanceData::globals(cx)[self.id()]
.as_ref()
.map(|boxed| boxed.downcast_ref::<T>().unwrap().clone())
}
}
2 changes: 2 additions & 0 deletions crates/neon/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@
pub mod context;
pub mod event;
pub mod handle;
#[cfg(feature = "napi-6")]
pub mod instance;
pub mod meta;
pub mod object;
pub mod prelude;
Expand Down
48 changes: 44 additions & 4 deletions crates/neon/src/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@
//!
//! [napi-docs]: https://nodejs.org/api/n-api.html#n_api_environment_life_cycle_apis

use std::sync::{
atomic::{AtomicU64, Ordering},
Arc,
use std::{
any::Any,
ops::{Index, IndexMut},
sync::{
atomic::{AtomicU64, Ordering},
Arc,
},
};

use crate::{
Expand Down Expand Up @@ -54,6 +58,36 @@ pub(crate) struct InstanceData {

/// Shared `Channel` that is cloned to be returned by the `cx.channel()` method
shared_channel: Channel,

/// Table of user-defined global cells.
globals: GlobalTable,
}

pub(crate) struct GlobalTable {
cells: Vec<Option<Box<dyn Any + Send>>>,
dherman marked this conversation as resolved.
Show resolved Hide resolved
}

impl GlobalTable {
fn new() -> Self {
dherman marked this conversation as resolved.
Show resolved Hide resolved
Self { cells: Vec::new() }
}
}

impl Index<usize> for GlobalTable {
type Output = Option<Box<dyn Any + Send>>;

fn index(&self, index: usize) -> &Self::Output {
&self.cells[index]
dherman marked this conversation as resolved.
Show resolved Hide resolved
}
}

impl IndexMut<usize> for GlobalTable {
fn index_mut(&mut self, index: usize) -> &mut Self::Output {
if index >= self.cells.len() {
self.cells.resize_with(index + 1, Default::default);
}
&mut self.cells[index]
}
}

/// Wrapper for raw Node-API values to be dropped on the main thread
Expand Down Expand Up @@ -83,7 +117,7 @@ impl InstanceData {
/// # Safety
/// No additional locking (e.g., `Mutex`) is necessary because holding a
/// `Context` reference ensures serialized access.
pub(crate) fn get<'a, C: Context<'a>>(cx: &mut C) -> &'a mut InstanceData {
pub(crate) fn get<'a, C: Context<'a>>(cx: &mut C) -> &mut InstanceData {
let env = cx.env().to_raw();
let data = unsafe { lifecycle::get_instance_data::<InstanceData>(env).as_mut() };

Expand All @@ -107,6 +141,7 @@ impl InstanceData {
id: InstanceId::next(),
drop_queue: Arc::new(drop_queue),
shared_channel,
globals: GlobalTable::new(),
};

unsafe { &mut *lifecycle::set_instance_data(env, data) }
Expand All @@ -129,4 +164,9 @@ impl InstanceData {
pub(crate) fn id<'a, C: Context<'a>>(cx: &mut C) -> InstanceId {
InstanceData::get(cx).id
}

/// Helper to return a reference to the `globals` field of `InstanceData`.
pub(crate) fn globals<'a, C: Context<'a>>(cx: &mut C) -> &mut GlobalTable {
&mut InstanceData::get(cx).globals
}
}
37 changes: 36 additions & 1 deletion test/napi/lib/workers.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
const assert = require("assert");
const { Worker, isMainThread, parentPort } = require("worker_threads");
const { Worker, isMainThread, parentPort, threadId } = require("worker_threads");

const addon = require("..");

// Receive a message, try that method and return the error message
if (!isMainThread) {
addon.set_thread_id(threadId);
parentPort.once("message", (message) => {
try {
switch (message) {
Expand All @@ -17,6 +18,12 @@ if (!isMainThread) {
case "get_or_init_clone":
addon.get_or_init_clone(() => ({}));
break;
case "get_thread_id":
{
let id = addon.get_thread_id();
parentPort.postMessage(id);
}
break;
default:
throw new Error(`Unexpected message: ${message}`);
}
Expand All @@ -30,6 +37,11 @@ if (!isMainThread) {
return;
}

// From here on, we're in the main thread.

// Set the `THREAD_ID` Global value in the main thread cell.
addon.set_thread_id(threadId);

describe("Worker / Root Tagging Tests", () => {
describe("Single Threaded", () => {
it("should be able to stash a global with `get_and_replace`", () => {
Expand Down Expand Up @@ -106,3 +118,26 @@ describe("Worker / Root Tagging Tests", () => {
});
});
});

describe("Globals", () => {
it("should be able to read an instance global from the main thread", () => {
let lookedUpId = addon.get_thread_id();
assert.strictEqual(lookedUpId, threadId);
});

it("should allocate separate globals for each addon instance", (cb) => {
let mainThreadId = addon.get_thread_id();

const worker = new Worker(__filename);

worker.once("message", (message) => {
assert.strictEqual(typeof message, 'number');
assert.notStrictEqual(message, mainThreadId);
let mainThreadIdAgain = addon.get_thread_id();
assert.strictEqual(mainThreadIdAgain, mainThreadId);
cb();
});

worker.postMessage("get_thread_id");
});
});
17 changes: 17 additions & 0 deletions test/napi/src/js/workers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::sync::Mutex;

use once_cell::sync::{Lazy, OnceCell};

use neon::instance::Global;
use neon::prelude::*;

pub fn get_and_replace(mut cx: FunctionContext) -> JsResult<JsValue> {
Expand Down Expand Up @@ -44,3 +45,19 @@ pub fn get_or_init_clone(mut cx: FunctionContext) -> JsResult<JsObject> {
// test the `clone` method.
Ok(o.clone(&mut cx).into_inner(&mut cx))
}

static THREAD_ID: Global<u32> = Global::new();

pub fn set_thread_id(mut cx: FunctionContext) -> JsResult<JsUndefined> {
let id = cx.argument::<JsNumber>(0)?.value(&mut cx) as u32;
THREAD_ID.set(&mut cx, id);
Ok(cx.undefined())
}

pub fn get_thread_id(mut cx: FunctionContext) -> JsResult<JsValue> {
let id = THREAD_ID.get(&mut cx);
Ok(match id {
Some(id) => cx.number(id).upcast(),
None => cx.undefined().upcast(),
})
}
2 changes: 2 additions & 0 deletions test/napi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
cx.export_function("get_and_replace", js::workers::get_and_replace)?;
cx.export_function("get_or_init", js::workers::get_or_init)?;
cx.export_function("get_or_init_clone", js::workers::get_or_init_clone)?;
cx.export_function("get_thread_id", js::workers::get_thread_id)?;
cx.export_function("set_thread_id", js::workers::set_thread_id)?;

Ok(())
}