Refactor guards into a `ContextCleanupGuard` abstraction (#2890)

Noticed we were using this pattern on a couple of places, so I abstracted it behind a `ContextCleanupGuard` struct.

Let me know if you remember another place where this pattern would apply.

boa_engine/src/builtins/eval/mod.rs

@@ -89,20 +89,6 @@ impl Eval {
             Restore(Vec<Environment>),
         }
 
-        /// Restores the environment after calling `eval` or after throwing an error.
-        fn restore_environment(context: &mut Context<'_>, action: EnvStackAction) {
-            match action {
-                EnvStackAction::Truncate(size) => {
-                    context.vm.environments.truncate(size);
-                }
-                EnvStackAction::Restore(envs) => {
-                    // Pop all environments created during the eval execution and restore the original stack.
-                    context.vm.environments.truncate(1);
-                    context.vm.environments.extend(envs);
-                }
-            }
-        }
-
         // 1. Assert: If direct is false, then strictCaller is also false.
         debug_assert!(direct || !strict);
 
@@ -230,6 +216,14 @@ impl Eval {
             EnvStackAction::Restore(environments)
         };
 
+        let context = &mut context.guard(move |ctx| match action {
+            EnvStackAction::Truncate(len) => ctx.vm.environments.truncate(len),
+            EnvStackAction::Restore(envs) => {
+                ctx.vm.environments.truncate(1);
+                ctx.vm.environments.extend(envs);
+            }
+        });
+
         // Only need to check on non-strict mode since strict mode automatically creates a function
         // environment for all eval calls.
         if !strict {
@@ -239,7 +233,6 @@ impl Eval {
                 .environments
                 .has_lex_binding_until_function_environment(&top_level_var_declared_names(&body))
             {
-                restore_environment(context, action);
                 let name = context.interner().resolve_expect(name.sym());
                 let msg = format!("variable declaration {name} in eval function already exists as a lexical variable");
                 return Err(JsNativeError::syntax().with_message(msg).into());
@@ -267,10 +260,7 @@ impl Eval {
         if direct && !strict {
             context.vm.environments.extend_outer_function_environment();
         }
-        let result = context.execute(code_block);
-
-        restore_environment(context, action);
 
-        result
+        context.execute(code_block)
     }
 }

boa_engine/src/context/mod.rs

@@ -561,9 +561,17 @@ impl Context<'_> {
     }
 }
 
-#[cfg(feature = "intl")]
 impl<'host> Context<'host> {
+    /// Creates a `ContextCleanupGuard` that executes some cleanup after being dropped.
+    pub(crate) fn guard<F>(&mut self, cleanup: F) -> ContextCleanupGuard<'_, 'host, F>
+    where
+        F: FnOnce(&mut Context<'_>) + 'static,
+    {
+        ContextCleanupGuard::new(self, cleanup)
+    }
+
     /// Get the ICU related utilities
+    #[cfg(feature = "intl")]
     pub(crate) const fn icu(&self) -> &icu::Icu<'host> {
         &self.icu
     }
@@ -742,3 +750,57 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
         Ok(context)
     }
 }
+
+/// A cleanup guard for a [`Context`] that is executed when dropped.
+#[derive(Debug)]
+pub(crate) struct ContextCleanupGuard<'a, 'host, F>
+where
+    F: FnOnce(&mut Context<'_>) + 'static,
+{
+    context: &'a mut Context<'host>,
+    cleanup: Option<F>,
+}
+
+impl<'a, 'host, F> ContextCleanupGuard<'a, 'host, F>
+where
+    F: FnOnce(&mut Context<'_>) + 'static,
+{
+    /// Creates a new `ContextCleanupGuard` from the current context and its cleanup operation.
+    pub(crate) fn new(context: &'a mut Context<'host>, cleanup: F) -> Self {
+        Self {
+            context,
+            cleanup: Some(cleanup),
+        }
+    }
+}
+
+impl<'host, F> std::ops::Deref for ContextCleanupGuard<'_, 'host, F>
+where
+    F: FnOnce(&mut Context<'_>) + 'static,
+{
+    type Target = Context<'host>;
+
+    fn deref(&self) -> &Self::Target {
+        self.context
+    }
+}
+
+impl<F> std::ops::DerefMut for ContextCleanupGuard<'_, '_, F>
+where
+    F: FnOnce(&mut Context<'_>) + 'static,
+{
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        self.context
+    }
+}
+
+impl<F> Drop for ContextCleanupGuard<'_, '_, F>
+where
+    F: FnOnce(&mut Context<'_>) + 'static,
+{
+    fn drop(&mut self) {
+        if let Some(cleanup) = self.cleanup.take() {
+            cleanup(self.context);
+        }
+    }
+}

boa_engine/src/vm/code_block.rs

@@ -14,7 +14,6 @@ use crate::{
     error::JsNativeError,
     object::{internal_methods::get_prototype_from_constructor, JsObject, ObjectData, PROTOTYPE},
     property::PropertyDescriptor,
-    realm::Realm,
     string::utf16,
     vm::CallFrame,
     Context, JsError, JsResult, JsString, JsValue,
@@ -846,49 +845,6 @@ pub(crate) fn create_generator_function_object(
     constructor
 }
 
-struct ContextCleanupGuard<'a, 'host> {
-    context: &'a mut Context<'host>,
-    old_realm: Realm,
-    old_active_function: Option<JsObject>,
-}
-
-impl<'a, 'host> ContextCleanupGuard<'a, 'host> {
-    /// Creates a new guard that resets the realm of the context on exit.
-    fn new(context: &'a mut Context<'host>, realm: Realm, active_function: JsObject) -> Self {
-        let old_realm = context.enter_realm(realm);
-        let old_active_function = context.vm.active_function.replace(active_function);
-        Self {
-            context,
-            old_realm,
-            old_active_function,
-        }
-    }
-}
-
-impl Drop for ContextCleanupGuard<'_, '_> {
-    fn drop(&mut self) {
-        self.context.enter_realm(self.old_realm.clone());
-        std::mem::swap(
-            &mut self.context.vm.active_function,
-            &mut self.old_active_function,
-        );
-    }
-}
-
-impl<'host> std::ops::Deref for ContextCleanupGuard<'_, 'host> {
-    type Target = Context<'host>;
-
-    fn deref(&self) -> &Self::Target {
-        self.context
-    }
-}
-
-impl std::ops::DerefMut for ContextCleanupGuard<'_, '_> {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        self.context
-    }
-}
-
 impl JsObject {
     pub(crate) fn call_internal(
         &self,
@@ -896,13 +852,22 @@ impl JsObject {
         args: &[JsValue],
         context: &mut Context<'_>,
     ) -> JsResult<JsValue> {
+        let old_realm = context.realm().clone();
+        let old_active_fn = context.vm.active_function.clone();
+
+        let context = &mut context.guard(move |ctx| {
+            ctx.enter_realm(old_realm);
+            ctx.vm.active_function = old_active_fn;
+        });
+
         let this_function_object = self.clone();
         let active_function = self.clone();
         let object = self.borrow();
         let function_object = object.as_function().expect("not a function");
         let realm = function_object.realm().clone();
 
-        let context = &mut ContextCleanupGuard::new(context, realm, active_function);
+        context.enter_realm(realm);
+        context.vm.active_function = Some(active_function);
 
         let (code, mut environments, class_object, async_, gen) = match function_object.kind() {
             FunctionKind::Native {
@@ -941,7 +906,6 @@ impl JsObject {
                     false,
                 )
             }
-
             FunctionKind::Async {
                 code,
                 environments,
@@ -1182,13 +1146,22 @@ impl JsObject {
         this_target: &JsValue,
         context: &mut Context<'_>,
     ) -> JsResult<Self> {
+        let old_realm = context.realm().clone();
+        let old_active_fn = context.vm.active_function.clone();
+        let context = &mut context.guard(move |ctx| {
+            ctx.enter_realm(old_realm);
+            ctx.vm.active_function = old_active_fn;
+        });
+
         let this_function_object = self.clone();
         let active_function = self.clone();
         let object = self.borrow();
         let function_object = object.as_function().expect("not a function");
         let realm = function_object.realm().clone();
 
-        let context = &mut ContextCleanupGuard::new(context, realm, active_function);
+        context.enter_realm(realm);
+        context.vm.active_function = Some(active_function);
+
         match function_object.kind() {
             FunctionKind::Native {
                 function,