Exclusive and Shared Context References
Opened this issue · 2 comments
Exclusive and Shared Context References
Purpose
The purpose of this pre-RFC is to start gathering feedback from maintainers and users about the perception of mutability inside the VM via a Context<'_> in order to guide a future RFC to update Neon's usage of Context<'_>.
Summary
Neon provides the Context<'_> trait for abstracting interactions with the JavaScript engine. Most methods in Neon require a reference to a Context. Some of these references are shared (&Context<'_>) and others are exclusive (&mut Context<'_>). However, there are some inconsistencies in when one or the other is required. For example, most methods that create JavaScript objects require &mut while methods that tend to hold a reference to a Context<'_> (or an underlying Isolate or napi_env) tend to use a &.
Neon should provide a consistent, opinionated API that guides users towards correct implementations with the help of the borrow checker. While I can't speak for all contributors, I know that I have lazily chose a safer exclusive reference (&mut) without thinking critically if it was necessary.
Background
& and &mut in idiomatic Rust
Terminology: For the purpose of this document,
&will be referred to as a shared reference and&mutan exclusive reference.
In idiomatic Rust, a shared reference & signals that it is safe for multiple pointers to the same data to exist concurrently. In contrast, only a single exclusive reference &mut may exist. Typically, this aligns 1-to-1 with mutability. There can either be one mutable reference or many immutable reference.
However, Rust also provides a concept of interior mutability. This allows mutating a value from a shared & reference. Examples include, Cell, RefCell, and atomics. An AtomicU32 can be incremented with a shared & reference because it provides internal consistency guarantees.
In application code, it is generally best to only use internal mutability to abstract implementation details in implementations that appear immutable from the user. For example, memoization of a pure function.
JavaScript Engine Throwing State
Unlike Rust, JavaScript makes use of exceptions. When working with the JavaScript Engine, it can be in several states:
- Running the event loop. JavaScript is executing and native code cannot call into the engine.
- Locked. Synchronous native code is executing and JavaScript is not. Calls into the engine are safe.
- Throwing. A JavaScript exception has been thrown. Only some calls into the JavaScript engine are safe.
- Shutting down. Clean-up is executing and no calls may be made into the JavaScript engine.
Calls into JavaScript from a native extension may trigger an exception. For example, calling a function that executes throw or attempting to read a property from undefined. Neon modules must be sensitive to the state of the VM.
Current
Nan Backend
With the Nan backend, Neon does not prevent calls into the JavaScript engine when in a throwing exception. Making calls when in a throwing state is undefined behavior. The user must be careful to check for the Throw error type in Result<_, _> and avoid using the Context<'_>.
N-API Backend
All N-API methods verify state before executing code that accesses the engine. If the engine is not in a valid state for the call, a status enum other than napi_status::napi_ok will be returned.
Neon functions that use N-API validate the status with assert_eq!(status, napi_status::napi_ok). The call to the engine will not occur, the Rust code will panic, the panic will be caught and returned to JavaScript and no undefined behavior will occur.
Questions
Is throwing considered mutating Context<'_>?
Under normal circumstances, creating a type (e.g., cx.string("hello")) appears pure. A new type is created and the underlying Context<'_> does not change. However, it's possible for the call to transition the engine to the throwing state.
In this case, an unrelated call may start failing (e.g., another cx.string("hello")). This may violate expectations of the user that one "pure" function should not cause another "pure" function to fail.
"Pure" is quoted because these methods are not technically pure functions since they may cause side-effects.
Proposal
In my opinion, this behavior is acceptable and preferred because it mirrors patterns found in the standard library. For example, Mutex can be used with & shared references, but if the lock becomes poisoned, future unrelated locks will fail. With this justification, I propose that most Neon methods should accept &Context<'_> and not &mut Context<'_>.
Using shared references in more places improves Neon's ergonomics because user defined structs and functions may alias Context<'_>.
When should exclusive references be used?
Context<'_> is a trait and some implementations include additional methods and data. For example, ModuleContext<'a> wraps the module as a JsObject. Methods that mutate the underlying JsObject should take an exclusive &mut reference. E.g., export_function.
Running into this in the context of borrow, and specifically JsBuffer. I'd love to be able to safely borrow a JsBuffer's contents but still interact with the context in ways guaranteed not to run arbitrary code. Unfortunately I don't know how many of those operations there are in JavaScript…even something like accessing a property could turn out to be invoking a getter. (Maybe it'd be okay to assume that constructing the standard Error types isn't going to be a problem, though.)
Isn't it always the case that when you have a Context, you're either in the Locked or Throwing state? If that's so, then "&mut for anything that can change that state" seems potentially useful (plus the ModuleContext example from above).
Any N-API function call may result in a pending JavaScript exception. This is the case for any of the API functions, even those that may not cause the execution of JavaScript.
Welp, so much for that (particular) idea.