diff --git a/src/context.rs b/src/context.rs index d086e25609..37b65e25a9 100644 --- a/src/context.rs +++ b/src/context.rs @@ -180,10 +180,10 @@ impl Context { // We also check above that `isolate` is the context's isolate. let self_ref_handle = unsafe { UnsafeRefHandle::new(self, isolate) }; - Weak::with_finalizer( + Weak::with_guaranteed_finalizer( isolate, self_ref_handle, - Box::new(move |_| { + Box::new(move || { // SAFETY: The lifetimes of references to the annex returned by this // method are always tied to the context, and because this is the // context's finalizer, we know there are no living references to diff --git a/src/handle.rs b/src/handle.rs index 48ebdda9c3..3435366920 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -558,7 +558,22 @@ impl Weak { ) -> Self { let HandleInfo { data, host } = handle.get_handle_info(); host.assert_match_isolate(isolate); - let finalizer_id = isolate.get_finalizer_map_mut().add(finalizer); + let finalizer_id = isolate + .get_finalizer_map_mut() + .add(FinalizerCallback::Regular(finalizer)); + Self::new_raw(isolate, data, Some(finalizer_id)) + } + + pub fn with_guaranteed_finalizer( + isolate: &mut Isolate, + handle: impl Handle, + finalizer: Box, + ) -> Self { + let HandleInfo { data, host } = handle.get_handle_info(); + host.assert_match_isolate(isolate); + let finalizer_id = isolate + .get_finalizer_map_mut() + .add(FinalizerCallback::Guaranteed(finalizer)); Self::new_raw(isolate, data, Some(finalizer_id)) } @@ -608,13 +623,17 @@ impl Weak { &self, finalizer: Box, ) -> Self { - self.clone_raw(Some(finalizer)) + self.clone_raw(Some(FinalizerCallback::Regular(finalizer))) } - fn clone_raw( + pub fn clone_with_guaranteed_finalizer( &self, - finalizer: Option>, + finalizer: Box, ) -> Self { + self.clone_raw(Some(FinalizerCallback::Guaranteed(finalizer))) + } + + fn clone_raw(&self, finalizer: Option) -> Self { if let Some(data) = self.get_pointer() { // SAFETY: We're in the isolate's thread, because Weak isn't Send or // Sync. @@ -781,7 +800,7 @@ impl Weak { let ptr = v8__WeakCallbackInfo__GetParameter(wci); &*(ptr as *mut WeakData) }; - let finalizer: Option> = { + let finalizer: Option = { let finalizer_id = weak_data.finalizer_id.unwrap(); isolate.get_finalizer_map_mut().map.remove(&finalizer_id) }; @@ -794,8 +813,10 @@ impl Weak { }; } - if let Some(finalizer) = finalizer { - finalizer(isolate); + match finalizer { + Some(FinalizerCallback::Regular(finalizer)) => finalizer(isolate), + Some(FinalizerCallback::Guaranteed(finalizer)) => finalizer(), + None => {} } } } @@ -907,17 +928,19 @@ struct WeakCallbackInfo(Opaque); type FinalizerId = usize; +pub(crate) enum FinalizerCallback { + Regular(Box), + Guaranteed(Box), +} + #[derive(Default)] pub(crate) struct FinalizerMap { - map: std::collections::HashMap>, + map: std::collections::HashMap, next_id: FinalizerId, } impl FinalizerMap { - pub(crate) fn add( - &mut self, - finalizer: Box, - ) -> FinalizerId { + fn add(&mut self, finalizer: FinalizerCallback) -> FinalizerId { let id = self.next_id; // TODO: Overflow. self.next_id += 1; @@ -928,4 +951,10 @@ impl FinalizerMap { pub(crate) fn is_empty(&self) -> bool { self.map.is_empty() } + + pub(crate) fn drain( + &mut self, + ) -> impl Iterator + '_ { + self.map.drain().map(|(_, finalizer)| finalizer) + } } diff --git a/src/isolate.rs b/src/isolate.rs index d43e3b225f..3b7de262ee 100644 --- a/src/isolate.rs +++ b/src/isolate.rs @@ -1,6 +1,7 @@ use crate::PromiseResolver; // Copyright 2019-2021 the Deno authors. All rights reserved. MIT license. use crate::function::FunctionCallbackInfo; +use crate::handle::FinalizerCallback; use crate::handle::FinalizerMap; use crate::isolate_create_params::raw; use crate::isolate_create_params::CreateParams; @@ -887,6 +888,13 @@ impl Isolate { annex.create_param_allocations = Box::new(()); annex.slots.clear(); + // Run through any remaining guaranteed finalizers. + for finalizer in annex.finalizer_map.drain() { + if let FinalizerCallback::Guaranteed(callback) = finalizer { + callback(); + } + } + // Subtract one from the Arc reference count. Arc::from_raw(annex); self.set_data(0, null_mut()); diff --git a/tests/slots.rs b/tests/slots.rs index 30489eb8f8..9ebf56d443 100644 --- a/tests/slots.rs +++ b/tests/slots.rs @@ -294,6 +294,34 @@ fn dropped_context_slots() { assert!(dropped.get()); } +#[test] +fn dropped_context_slots_on_kept_context() { + use std::cell::Cell; + + struct DropMarker(Rc>); + impl Drop for DropMarker { + fn drop(&mut self) { + println!("Dropping the drop marker"); + self.0.set(true); + } + } + + let mut isolate = CoreIsolate::new(Default::default()); + let dropped = Rc::new(Cell::new(false)); + let _global_context; + { + let scope = &mut v8::HandleScope::new(isolate.deref_mut()); + let context = v8::Context::new(scope); + + context.set_slot(scope, DropMarker(dropped.clone())); + + _global_context = v8::Global::new(scope, context); + } + + drop(isolate); + assert!(dropped.get()); +} + #[test] fn clear_all_context_slots() { setup();