Copy safety

crates/neon-sys/src/lib.rs

@@ -7,7 +7,7 @@ use std::ptr::null_mut;
 /// `Local` handles get associated to a V8 `HandleScope` container. Note: Node.js creates a
 /// `HandleScope` right before calling functions in native addons.
 #[repr(C)]
-#[derive(Clone, Copy)]
+#[derive(Debug, Clone, Copy)]
 pub struct Local {
     pub handle: *mut c_void,
 }

src/context/mod.rs

@@ -714,7 +714,7 @@ impl<'a> ModuleContext<'a> {
         f: fn(FunctionContext) -> JsResult<T>,
     ) -> NeonResult<()> {
         let value = JsFunction::new(self, f)?.upcast::<JsValue>();
-        self.exports.set(self, key, value)?;
+        self.exports.clone().set(self, key, value)?;
         Ok(())
     }
 
@@ -726,21 +726,23 @@ impl<'a> ModuleContext<'a> {
         V: Value,
     {
         let value = JsFunction::new(self, f)?.upcast::<JsValue>();
-        self.exports.set(self, key, value)?;
+        // Note: Cloning `exports` is necessary to avoid holding a shared reference to
+        // `self` while attempting to use it mutably in `set`.
+        self.exports.clone().set(self, key, value)?;
         Ok(())
     }
 
     #[cfg(feature = "legacy-runtime")]
     /// Convenience method for exporting a Neon class constructor from a module.
     pub fn export_class<T: Class>(&mut self, key: &str) -> NeonResult<()> {
         let constructor = T::constructor(self)?;
-        self.exports.set(self, key, constructor)?;
+        self.exports.clone().set(self, key, constructor)?;
         Ok(())
     }
 
     /// Exports a JavaScript value from a Neon module.
     pub fn export_value<T: Value>(&mut self, key: &str, val: Handle<T>) -> NeonResult<()> {
-        self.exports.set(self, key, val)?;
+        self.exports.clone().set(self, key, val)?;
         Ok(())
     }
 

src/handle/internal.rs

@@ -1,5 +1,30 @@
+use std::fmt::Debug;
+use std::mem;
+
 use crate::types::Value;
 
 pub trait SuperType<T: Value> {
-    fn upcast_internal(v: T) -> Self;
+    fn upcast_internal(v: &T) -> Self;
+}
+
+#[doc(hidden)]
+/// Trait asserting that `Self` is a transparent wrapper around `Self::Inner`
+/// with identical representation and may be safely transmuted.
+///
+/// # Safety
+/// `Self` must be `#[repr(transparent)]` with a field `Self::Inner`
+pub unsafe trait TransparentNoCopyWrapper: Sized {
+    type Inner: Debug + Copy;
+
+    // A default implementation cannot be provided because it would create
+    // dependently sized types. This may be supported in a future Rust version.
+    fn into_inner(self) -> Self::Inner;
+
+    fn wrap_ref(s: &Self::Inner) -> &Self {
+        unsafe { mem::transmute(s) }
+    }
+
+    fn wrap_mut(s: &mut Self::Inner) -> &mut Self {
+        unsafe { mem::transmute(s) }
+    }
 }

src/handle/mod.rs

@@ -58,7 +58,7 @@ pub(crate) mod root;
 #[cfg(feature = "napi-1")]
 pub use self::root::Root;
 
-use self::internal::SuperType;
+use self::internal::{SuperType, TransparentNoCopyWrapper};
 use crate::context::internal::Env;
 use crate::context::Context;
 use crate::result::{JsResult, JsResultExt};
@@ -68,20 +68,23 @@ use neon_runtime::raw;
 use std::error::Error;
 use std::fmt::{self, Debug, Display};
 use std::marker::PhantomData;
+use std::mem;
 use std::ops::{Deref, DerefMut};
 
 /// The trait of data owned by the JavaScript engine and that can only be accessed via handles.
-pub trait Managed: Copy {
-    fn to_raw(self) -> raw::Local;
+pub trait Managed: TransparentNoCopyWrapper {
+    fn to_raw(&self) -> raw::Local;
 
     fn from_raw(env: Env, h: raw::Local) -> Self;
 }
 
 /// A handle to a JavaScript value that is owned by the JavaScript engine.
-#[repr(C)]
-#[derive(Clone, Copy, Debug)]
+#[derive(Debug)]
+#[repr(transparent)]
 pub struct Handle<'a, T: Managed + 'a> {
-    value: T,
+    // Contains the actual `Copy` JavaScript value data. It will be wrapped in
+    // in a `!Copy` type when dereferencing. Only `T` should be visible to the user.
+    value: <T as TransparentNoCopyWrapper>::Inner,
     phantom: PhantomData<&'a T>,
 }
 
@@ -95,10 +98,21 @@ impl<'a, T: Managed + 'a> PartialEq for Handle<'a, T> {
 #[cfg(feature = "legacy-runtime")]
 impl<'a, T: Managed + 'a> Eq for Handle<'a, T> {}
 
+impl<'a, T: Managed> Clone for Handle<'a, T> {
+    fn clone(&self) -> Self {
+        Self {
+            value: self.value.clone(),
+            phantom: PhantomData,
+        }
+    }
+}
+
+impl<'a, T: Managed> Copy for Handle<'a, T> {}
+
 impl<'a, T: Managed + 'a> Handle<'a, T> {
     pub(crate) fn new_internal(value: T) -> Handle<'a, T> {
         Handle {
-            value,
+            value: value.into_inner(),
             phantom: PhantomData,
         }
     }
@@ -151,7 +165,7 @@ impl<'a, T: Value> Handle<'a, T> {
     ///
     /// This method does not require an execution context because it only copies a handle.
     pub fn upcast<U: Value + SuperType<T>>(&self) -> Handle<'a, U> {
-        Handle::new_internal(SuperType::upcast_internal(self.value))
+        Handle::new_internal(SuperType::upcast_internal(self.deref()))
     }
 
     #[cfg(feature = "legacy-runtime")]
@@ -170,7 +184,7 @@ impl<'a, T: Value> Handle<'a, T> {
     /// # }
     /// ```
     pub fn is_a<U: Value>(&self) -> bool {
-        U::is_typeof(Env::current(), self.value)
+        U::is_typeof(Env::current(), self.deref())
     }
 
     #[cfg(feature = "napi-1")]
@@ -189,7 +203,7 @@ impl<'a, T: Value> Handle<'a, T> {
     /// # }
     /// ```
     pub fn is_a<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> bool {
-        U::is_typeof(cx.env(), self.value)
+        U::is_typeof(cx.env(), self.deref())
     }
 
     #[cfg(feature = "legacy-runtime")]
@@ -198,7 +212,7 @@ impl<'a, T: Value> Handle<'a, T> {
     /// continue interacting with the JS engine if this method produces an `Err`
     /// result.
     pub fn downcast<U: Value>(&self) -> DowncastResult<'a, T, U> {
-        match U::downcast(Env::current(), self.value) {
+        match U::downcast(Env::current(), self.deref()) {
             Some(v) => Ok(Handle::new_internal(v)),
             None => Err(DowncastError::new()),
         }
@@ -210,7 +224,7 @@ impl<'a, T: Value> Handle<'a, T> {
     /// continue interacting with the JS engine if this method produces an `Err`
     /// result.
     pub fn downcast<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> DowncastResult<'a, T, U> {
-        match U::downcast(cx.env(), self.value) {
+        match U::downcast(cx.env(), self.deref()) {
             Some(v) => Ok(Handle::new_internal(v)),
             None => Err(DowncastError::new()),
         }
@@ -247,12 +261,12 @@ impl<'a, T: Value> Handle<'a, T> {
 impl<'a, T: Managed> Deref for Handle<'a, T> {
     type Target = T;
     fn deref(&self) -> &T {
-        &self.value
+        unsafe { mem::transmute(&self.value) }
     }
 }
 
 impl<'a, T: Managed> DerefMut for Handle<'a, T> {
     fn deref_mut(&mut self) -> &mut T {
-        &mut self.value
+        unsafe { mem::transmute(&mut self.value) }
     }
 }

src/lib.rs

@@ -336,10 +336,17 @@ macro_rules! class_definition {
 #[macro_export(local_inner_macros)]
 macro_rules! impl_managed {
     ($cls:ident) => {
+        unsafe impl $crate::macro_internal::TransparentNoCopyWrapper for $cls {
+            type Inner = $crate::macro_internal::runtime::raw::Local;
+
+            fn into_inner(self) -> Self::Inner {
+                self.0
+            }
+        }
+
         impl $crate::handle::Managed for $cls {
-            fn to_raw(self) -> $crate::macro_internal::runtime::raw::Local {
-                let $cls(raw) = self;
-                raw
+            fn to_raw(&self) -> $crate::macro_internal::runtime::raw::Local {
+                self.0
             }
 
             fn from_raw(
@@ -406,8 +413,8 @@ macro_rules! declare_types {
     };
 
     { $(#[$attr:meta])* pub class $cls:ident as $cname:ident for $typ:ty { $($body:tt)* } $($rest:tt)* } => {
-        #[derive(Copy, Clone)]
-        #[repr(C)]
+        #[derive(Debug)]
+        #[repr(transparent)]
         $(#[$attr])*
         pub struct $cls($crate::macro_internal::runtime::raw::Local);
 

src/macro_internal/mod.rs

@@ -1,5 +1,7 @@
 //! Internals needed by macros. These have to be exported for the macros to work
 pub use crate::context::internal::{initialize_module, Env};
+#[cfg(feature = "legacy-runtime")]
+pub use crate::handle::internal::TransparentNoCopyWrapper;
 /// but are subject to change and should never be explicitly used.
 
 #[cfg(feature = "legacy-runtime")]

src/object/class/mod.rs

@@ -240,7 +240,7 @@ impl<T: Class> ValueInternal for T {
         }
     }
 
-    fn is_typeof<Other: Value>(mut env: Env, value: Other) -> bool {
+    fn is_typeof<Other: Value>(mut env: Env, value: &Other) -> bool {
         let map = env.class_map();
         match map.get(&TypeId::of::<T>()) {
             None => false,

src/object/mod.rs

@@ -91,7 +91,7 @@ mod traits {
     /// The trait of all object types.
     pub trait Object: Value {
         fn get<'a, V: Value, C: Context<'a>, K: PropertyKey>(
-            self,
+            &self,
             cx: &mut C,
             key: K,
         ) -> NeonResult<Handle<'a, V>> {
@@ -100,15 +100,15 @@ mod traits {
             v.downcast_or_throw(cx)
         }
 
-        fn get_own_property_names<'a, C: Context<'a>>(self, cx: &mut C) -> JsResult<'a, JsArray> {
+        fn get_own_property_names<'a, C: Context<'a>>(&self, cx: &mut C) -> JsResult<'a, JsArray> {
             let env = cx.env();
             build(env, |out| unsafe {
                 neon_runtime::object::get_own_property_names(out, env.to_raw(), self.to_raw())
             })
         }
 
         fn set<'a, C: Context<'a>, K: PropertyKey, W: Value>(
-            self,
+            &self,
             _: &mut C,
             key: K,
             val: Handle<W>,
@@ -238,7 +238,7 @@ mod traits {
     /// The trait of all object types.
     pub trait Object: Value {
         fn get<'a, V: Value, C: Context<'a>, K: PropertyKey>(
-            self,
+            &self,
             cx: &mut C,
             key: K,
         ) -> NeonResult<Handle<'a, V>> {
@@ -250,7 +250,7 @@ mod traits {
 
         #[cfg(feature = "napi-6")]
         #[cfg_attr(docsrs, doc(cfg(feature = "napi-6")))]
-        fn get_own_property_names<'a, C: Context<'a>>(self, cx: &mut C) -> JsResult<'a, JsArray> {
+        fn get_own_property_names<'a, C: Context<'a>>(&self, cx: &mut C) -> JsResult<'a, JsArray> {
             let env = cx.env();
 
             build(cx.env(), |out| unsafe {
@@ -259,7 +259,7 @@ mod traits {
         }
 
         fn set<'a, C: Context<'a>, K: PropertyKey, W: Value>(
-            self,
+            &self,
             cx: &mut C,
             key: K,
             val: Handle<W>,

src/types/binary.rs

@@ -2,7 +2,7 @@ use crate::borrow::internal::Pointer;
 use crate::borrow::{Borrow, BorrowMut, LoanError, Ref, RefMut};
 use crate::context::internal::Env;
 use crate::context::{Context, Lock};
-use crate::handle::Managed;
+use crate::handle::{internal::TransparentNoCopyWrapper, Managed};
 use crate::result::JsResult;
 use crate::types::private::ValueInternal;
 use crate::types::{build, Object, Value};
@@ -14,10 +14,18 @@ use std::os::raw::c_void;
 use std::slice;
 
 /// The Node [`Buffer`](https://nodejs.org/api/buffer.html) type.
-#[repr(C)]
-#[derive(Clone, Copy)]
+#[derive(Debug)]
+#[repr(transparent)]
 pub struct JsBuffer(raw::Local);
 
+unsafe impl TransparentNoCopyWrapper for JsBuffer {
+    type Inner = raw::Local;
+
+    fn into_inner(self) -> Self::Inner {
+        self.0
+    }
+}
+
 impl JsBuffer {
     /// Constructs a new `Buffer` object, safely zero-filled.
     pub fn new<'a, C: Context<'a>>(cx: &mut C, size: u32) -> JsResult<'a, JsBuffer> {
@@ -40,7 +48,7 @@ impl JsBuffer {
 }
 
 impl Managed for JsBuffer {
-    fn to_raw(self) -> raw::Local {
+    fn to_raw(&self) -> raw::Local {
         self.0
     }
 
@@ -54,7 +62,7 @@ impl ValueInternal for JsBuffer {
         "Buffer".to_string()
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: Other) -> bool {
+    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
         unsafe { neon_runtime::tag::is_buffer(env.to_raw(), other.to_raw()) }
     }
 }
@@ -64,8 +72,8 @@ impl Value for JsBuffer {}
 impl Object for JsBuffer {}
 
 /// The standard JS [`ArrayBuffer`](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer) type.
-#[repr(C)]
-#[derive(Clone, Copy)]
+#[derive(Debug)]
+#[repr(transparent)]
 pub struct JsArrayBuffer(raw::Local);
 
 impl JsArrayBuffer {
@@ -77,8 +85,16 @@ impl JsArrayBuffer {
     }
 }
 
+unsafe impl TransparentNoCopyWrapper for JsArrayBuffer {
+    type Inner = raw::Local;
+
+    fn into_inner(self) -> Self::Inner {
+        self.0
+    }
+}
+
 impl Managed for JsArrayBuffer {
-    fn to_raw(self) -> raw::Local {
+    fn to_raw(&self) -> raw::Local {
         self.0
     }
 
@@ -92,7 +108,7 @@ impl ValueInternal for JsArrayBuffer {
         "ArrayBuffer".to_string()
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: Other) -> bool {
+    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
         unsafe { neon_runtime::tag::is_arraybuffer(env.to_raw(), other.to_raw()) }
     }
 }