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

Enforce isolation of instance data #843

Closed
dherman opened this issue Jan 15, 2022 · 0 comments
Closed

Enforce isolation of instance data #843

dherman opened this issue Jan 15, 2022 · 0 comments
Labels

Comments

@dherman
Copy link
Collaborator

dherman commented Jan 15, 2022

We don't have explicit logic for enforcing that instance data from one JS thread can't be shared with another JS thread. This test case shows how a Neon module can store a rooted handle in a global cell from one JS thread and then attempt to access it in another thread. In my tests this seems to hang sometimes, and other times panics with a confusing error:

Stack trace
FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope
 1: 0x1099e3d35 node::Abort() (.cold.1) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
 2: 0x1086d9239 node::Abort() [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
 3: 0x1086d93af node::OnFatalError(char const*, char const*) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
 4: 0x108857b8e v8::Utils::ReportApiFailure(char const*, char const*) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
 5: 0x1089c9ce2 v8::internal::HandleScope::Extend(v8::internal::Isolate*) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
 6: 0x108859ca5 v8::HandleScope::CreateHandle(v8::internal::Isolate*, unsigned long) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
 7: 0x108694621 napi_get_reference_value [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
 8: 0x10d317349 neon_runtime::napi::reference::get::h6858a65d09c3ab37 [/Users/dherman/Sources/instance-data-test/my-lib/index.node]
 9: 0x10d31398f my_lib::get::hef419b7ba393f378 [/Users/dherman/Sources/instance-data-test/my-lib/index.node]
10: 0x10d3120e4 neon::types::error::convert_panics::h4d928c6dbec5d1e5 [/Users/dherman/Sources/instance-data-test/my-lib/index.node]
11: 0x10d31294a neon::context::internal::Scope$LT$R$GT$::with::hb96cbf77f70198bc [/Users/dherman/Sources/instance-data-test/my-lib/index.node]
12: 0x10d313ce5 _$LT$neon..types..internal..FunctionCallback$LT$T$GT$$u20$as$u20$neon..types..internal..Callback$LT$$BP$mut$u20$neon_runtime..napi..bindings..types..Value__$GT$$GT$::invoke::h9a9a6a6c8245bd19 [/Users/dherman/Sources/instance-data-test/my-lib/index.node]
13: 0x108696a58 v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo const&) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
14: 0x1088be31e v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
15: 0x1088bda1a v8::internal::MaybeHandle v8::internal::(anonymous namespace)::HandleApiCallHelper(v8::internal::Isolate*, v8::internal::Handle, v8::internal::Handle, v8::internal::Handle, v8::internal::Handle, v8::internal::BuiltinArguments) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
16: 0x1088bd1bc v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]
17: 0x109122519 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/dherman/.volta/tools/image/node/16.8.0/bin/node]

I believe the fix is to tag Root structs with information about which thread they came from, and check on root.to_inner() that the context comes from the same thread, predictably panicking with a clearer error message. An alternative would be for to_inner() to produce a Result but that seems like a loss of ergonomics for a fairly rare and subtle failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant