Skip to content

Commit

Permalink
fix(neon): Fix a leak from global event queue Arc not being dropped
Browse files Browse the repository at this point in the history
  • Loading branch information
kjvalencik committed Jun 1, 2021
1 parent 8b26072 commit 41d30b7
Showing 1 changed file with 22 additions and 8 deletions.
30 changes: 22 additions & 8 deletions src/handle/root.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::ffi::c_void;
use std::marker::PhantomData;
use std::mem::ManuallyDrop;
#[cfg(feature = "napi-6")]
use std::sync::Arc;

use neon_runtime::reference;
Expand Down Expand Up @@ -32,7 +32,7 @@ unsafe impl Sync for NapiRef {}
/// A `Root<T>` may be sent across threads, but the referenced object may
/// only be accessed on the JavaScript thread that created it.
pub struct Root<T> {
internal: NapiRef,
internal: Option<NapiRef>,
#[cfg(feature = "napi-6")]
drop_queue: Arc<ThreadsafeFunction<NapiRef>>,
_phantom: PhantomData<T>,
Expand Down Expand Up @@ -65,7 +65,7 @@ impl<T: Object> Root<T> {
let internal = unsafe { reference::new(env, value.to_raw()) };

Self {
internal: NapiRef(internal as *mut _),
internal: Some(NapiRef(internal as *mut _)),
#[cfg(feature = "napi-6")]
drop_queue: InstanceData::drop_queue(cx),
_phantom: PhantomData,
Expand All @@ -86,7 +86,7 @@ impl<T: Object> Root<T> {
/// ```
pub fn clone<'a, C: Context<'a>>(&self, cx: &mut C) -> Self {
let env = cx.env();
let internal = self.internal.0 as *mut _;
let internal = self.as_napi_ref().0 as *mut _;

unsafe {
reference::reference(env.to_raw(), internal);
Expand All @@ -104,7 +104,7 @@ impl<T: Object> Root<T> {
/// object.
pub fn drop<'a, C: Context<'a>>(self, cx: &mut C) {
let env = cx.env().to_raw();
let internal = ManuallyDrop::new(self).internal.0 as *mut _;
let internal = self.into_napi_ref().0 as *mut _;

unsafe {
reference::unreference(env, internal);
Expand All @@ -114,7 +114,7 @@ impl<T: Object> Root<T> {
/// Return the referenced JavaScript object and allow it to be garbage collected
pub fn into_inner<'a, C: Context<'a>>(self, cx: &mut C) -> Handle<'a, T> {
let env = cx.env();
let internal = ManuallyDrop::new(self).internal.0 as *mut _;
let internal = self.into_napi_ref().0 as *mut _;

let local = unsafe { reference::get(env.to_raw(), internal) };

Expand All @@ -130,10 +130,18 @@ impl<T: Object> Root<T> {
/// can be used in place of a clone immediately followed by a call to `into_inner`.
pub fn to_inner<'a, C: Context<'a>>(&self, cx: &mut C) -> Handle<'a, T> {
let env = cx.env();
let local = unsafe { reference::get(env.to_raw(), self.internal.0 as *mut _) };
let local = unsafe { reference::get(env.to_raw(), self.as_napi_ref().0 as *mut _) };

Handle::new_internal(T::from_raw(env, local))
}

fn as_napi_ref(&self) -> &NapiRef {
self.internal.as_ref().unwrap()
}

fn into_napi_ref(mut self) -> NapiRef {
self.internal.take().unwrap()
}
}

// Allows putting `Root<T>` directly in a container that implements `Finalize`
Expand All @@ -147,6 +155,10 @@ impl<T: Object> Finalize for Root<T> {
impl<T> Drop for Root<T> {
#[cfg(not(feature = "napi-6"))]
fn drop(&mut self) {
if self.internal.is_none() {
return;
}

// Destructors are called during stack unwinding, prevent a double
// panic and instead prefer to leak.
if std::thread::panicking() {
Expand All @@ -165,6 +177,8 @@ impl<T> Drop for Root<T> {

#[cfg(feature = "napi-6")]
fn drop(&mut self) {
let _ = self.drop_queue.call(self.internal.clone(), None);
if let Some(internal) = self.internal.take() {
let _ = self.drop_queue.call(internal.clone(), None);
}
}
}

0 comments on commit 41d30b7

Please sign in to comment.