refactor: thread MutationContext token, resolve Gc borrow conflicts in environments

.github/workflows/pull_request.yml

@@ -5,6 +5,7 @@ on:
     branches:
       - main
       - releases/**
+      - dev/oscars-gc
 
 permissions:
   contents: read

.github/workflows/rust.yml

@@ -5,10 +5,12 @@ on:
     branches:
       - main
       - releases/**
+      - dev/oscars-gc
   push:
     branches:
       - main
       - releases/**
+      - dev/oscars-gc
   merge_group:
     types: [checks_requested]
   workflow_dispatch:

.github/workflows/test262.yml

@@ -5,6 +5,7 @@ on:
     branches:
       - main
       - releases/**
+      - dev/oscars-gc
 
 permissions:
   contents: read

.github/workflows/webassembly.yml

@@ -5,10 +5,12 @@ on:
     branches:
       - main
       - releases/**
+      - dev/oscars-gc
   push:
     branches:
       - main
       - releases/**
+      - dev/oscars-gc
   merge_group:
     types: [checks_requested]
 

core/engine/src/builtins/eval/mod.rs

@@ -320,10 +320,8 @@ impl Eval {
 
         compiler.compile_statement_list(body.statements(), true, false);
 
-        let code_block = Gc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
-            compiler.finish(),
-        );
+        let finished = compiler.finish();
+        let code_block = Gc::new(&context.gc(), finished);
 
         // Strict calls don't need extensions, since all strict eval calls push a new
         // function environment before evaluating.
@@ -350,9 +348,11 @@ impl Eval {
         {
             let frame = context.vm.frame_mut();
             let global = frame.realm.environment();
-            frame
-                .environments
-                .push_lexical(lexical_scope.num_bindings_non_local(), global);
+            frame.environments.push_lexical(
+                lexical_scope.num_bindings_non_local(),
+                global,
+                unsafe { boa_gc::MutationContext::dummy() },
+            );
         }
 
         context

core/engine/src/builtins/finalization_registry/mod.rs

@@ -158,10 +158,7 @@ impl BuiltInConstructor for FinalizationRegistry {
             },
         );
 
-        let weak_registry = WeakGc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
-            registry.inner(),
-        );
+        let weak_registry = WeakGc::new(&context.gc(), registry.inner());
 
         {
             async fn inner_cleanup(
@@ -205,7 +202,7 @@ impl FinalizationRegistry {
     /// [`FinalizationRegistry.prototype.register ( target, heldValue [ , unregisterToken ] )`][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/sec-finalization-registry.prototype.register
-    fn register(this: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult<JsValue> {
+    fn register(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult<JsValue> {
         // 1. Let finalizationRegistry be the this value.
         // 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]).
         let this = this.as_object();
@@ -257,10 +254,7 @@ impl FinalizationRegistry {
         //
         // TODO: support Symbols
         let unregister_token = match unregister_token.variant() {
-            JsVariant::Object(obj) => Some(WeakGc::new(
-                &unsafe { boa_gc::MutationContext::dummy() },
-                obj.inner(),
-            )),
+            JsVariant::Object(obj) => Some(WeakGc::new(&context.gc(), obj.inner())),
             // b. Set unregisterToken to empty.
             JsVariant::Undefined => None,
             // a. If unregisterToken is not undefined, throw a TypeError exception.
@@ -275,7 +269,7 @@ impl FinalizationRegistry {
         // 6. Let cell be the Record { [[WeakRefTarget]]: target, [[HeldValue]]: heldValue, [[UnregisterToken]]: unregisterToken }.
         let cell = RegistryCell {
             target: Ephemeron::new(
-                &unsafe { boa_gc::MutationContext::dummy() },
+                &context.gc(),
                 target_obj.inner(),
                 CleanupSignaler(Cell::new(Some(
                     registry.cleanup_notifier.clone().downgrade(),
@@ -295,7 +289,7 @@ impl FinalizationRegistry {
     /// [`FinalizationRegistry.prototype.unregister ( unregisterToken )`][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-finalization-registry.prototype.unregister
-    fn unregister(this: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult<JsValue> {
+    fn unregister(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult<JsValue> {
         // 1. Let finalizationRegistry be the this value.
         // 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]).
         let this = this.as_object();
@@ -338,20 +332,16 @@ impl FinalizationRegistry {
 
             // a. If cell.[[UnregisterToken]] is not empty and SameValue(cell.[[UnregisterToken]], unregisterToken) is true, then
             if let Some(tok) = cell.unregister_token.as_ref()
-                && let Some(tok) = tok.upgrade(&unsafe { boa_gc::MutationContext::dummy() })
+                && let Some(tok) = tok.upgrade(&context.gc())
                 && Gc::ptr_eq(&tok, unregister_token)
             {
                 // i. Remove cell from finalizationRegistry.[[Cells]].
                 let cell = registry.cells.swap_remove(i);
-                let _key = cell
-                    .target
-                    .key(&unsafe { boa_gc::MutationContext::dummy() });
+                let _key = cell.target.key(&context.gc());
 
                 // TODO: it might be better to add a special ref for the value that
                 // also preserves the original key instead.
-                cell.target
-                    .value(&unsafe { boa_gc::MutationContext::dummy() })
-                    .and_then(|v| v.0.take());
+                cell.target.value(&context.gc()).and_then(|v| v.0.take());
 
                 // ii. Set removed to true.
                 removed = true;

core/engine/src/builtins/function/mod.rs

@@ -1073,7 +1073,9 @@ pub(crate) fn function_call(
     if has_binding_identifier {
         let frame = context.vm.frame_mut();
         let global = frame.realm.environment();
-        let index = frame.environments.push_lexical(1, global);
+        let index = frame
+            .environments
+            .push_lexical(1, global, unsafe { boa_gc::MutationContext::dummy() });
         frame.environments.put_lexical_value(
             BindingLocatorScope::Stack(index),
             0,
@@ -1091,6 +1093,7 @@ pub(crate) fn function_call(
             scope,
             FunctionSlots::new(this, function_object.clone(), None),
             global,
+            unsafe { boa_gc::MutationContext::dummy() },
         );
     }
 
@@ -1181,7 +1184,9 @@ fn function_construct(
     if has_binding_identifier {
         let frame = context.vm.frame_mut();
         let global = frame.realm.environment();
-        let index = frame.environments.push_lexical(1, global);
+        let index = frame
+            .environments
+            .push_lexical(1, global, unsafe { boa_gc::MutationContext::dummy() });
         frame.environments.put_lexical_value(
             BindingLocatorScope::Stack(index),
             0,
@@ -1210,6 +1215,7 @@ fn function_construct(
                 ),
             ),
             global,
+            unsafe { boa_gc::MutationContext::dummy() },
         );
     }
 

core/engine/src/builtins/json/mod.rs

@@ -307,10 +307,8 @@ impl Json {
                 SourcePath::Json,
             );
             compiler.compile_statement_list(script.statements(), true, false);
-            Gc::new(
-                &unsafe { boa_gc::MutationContext::dummy() },
-                compiler.finish(),
-            )
+            let finished = compiler.finish();
+            Gc::new(&context.gc(), finished)
         };
 
         let realm = context.realm().clone();

core/engine/src/builtins/promise/mod.rs

@@ -244,7 +244,7 @@ impl PromiseCapability {
         // 2. NOTE: C is assumed to be a constructor function that supports the parameter conventions of the Promise constructor (see 27.2.3.1).
         // 3. Let promiseCapability be the PromiseCapability Record { [[Promise]]: undefined, [[Resolve]]: undefined, [[Reject]]: undefined }.
         let promise_capability = Gc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
+            &context.gc(),
             GcRefCell::new(RejectResolve {
                 reject: JsValue::undefined(),
                 resolve: JsValue::undefined(),
@@ -656,10 +656,7 @@ impl Promise {
         }
 
         // 1. Let values be a new empty List.
-        let values = Gc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
-            GcRefCell::new(Vec::new()),
-        );
+        let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new()));
 
         // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }.
         let remaining_elements_count = Rc::new(Cell::new(1));
@@ -874,10 +871,7 @@ impl Promise {
         }
 
         // 1. Let values be a new empty List.
-        let values = Gc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
-            GcRefCell::new(Vec::new()),
-        );
+        let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new()));
 
         // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }.
         let remaining_elements_count = Rc::new(Cell::new(1));
@@ -1244,10 +1238,7 @@ impl Promise {
         let keys = Rc::new(RefCell::new(Vec::new()));
 
         // 3. Let values be a new empty List.
-        let values = Gc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
-            GcRefCell::new(Vec::new()),
-        );
+        let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new()));
 
         // 4. Let remainingElementsCount be the Record { [[Value]]: 1 }.
         let remaining_elements_count = Rc::new(Cell::new(1));
@@ -1557,10 +1548,7 @@ impl Promise {
         }
 
         // 1. Let errors be a new empty List.
-        let errors = Gc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
-            GcRefCell::new(Vec::new()),
-        );
+        let errors = Gc::new(&context.gc(), GcRefCell::new(Vec::new()));
 
         // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }.
         let remaining_elements_count = Rc::new(Cell::new(1));
@@ -2460,10 +2448,7 @@ impl Promise {
         // 1. Let alreadyResolved be the Record { [[Value]]: false }.
         // 5. Set resolve.[[Promise]] to promise.
         // 6. Set resolve.[[AlreadyResolved]] to alreadyResolved.
-        let promise = Gc::new(
-            &unsafe { boa_gc::MutationContext::dummy() },
-            Cell::new(Some(promise.clone())),
-        );
+        let promise = Gc::new(&context.gc(), Cell::new(Some(promise.clone())));
 
         // 2. Let stepsResolve be the algorithm steps defined in Promise Resolve Functions.
         // 3. Let lengthResolve be the number of non-optional parameters of the function definition in Promise Resolve Functions.

core/engine/src/builtins/weak/weak_ref.rs

@@ -87,7 +87,7 @@ impl BuiltInConstructor for WeakRef {
         let weak_ref = JsObject::from_proto_and_data_with_shared_shape(
             context.root_shape(),
             prototype,
-            WeakGc::new(&unsafe { boa_gc::MutationContext::dummy() }, target.inner()),
+            WeakGc::new(&context.gc(), target.inner()),
         );
 
         // 4. Perform AddToKeptObjects(target).
@@ -124,7 +124,7 @@ impl WeakRef {
         // https://tc39.es/ecma262/multipage/managing-memory.html#sec-weakrefderef
         // 1. Let target be weakRef.[[WeakRefTarget]].
         // 2. If target is not empty, then
-        if let Some(object) = weak_ref.upgrade(&unsafe { boa_gc::MutationContext::dummy() }) {
+        if let Some(object) = weak_ref.upgrade(&context.gc()) {
             let object = JsObject::from(object);
 
             // a. Perform AddToKeptObjects(target).

core/engine/src/builtins/weak_map/mod.rs

@@ -97,7 +97,7 @@ impl BuiltInConstructor for WeakMap {
         let map = JsObject::from_proto_and_data_with_shared_shape(
             context.root_shape(),
             prototype,
-            NativeWeakMap::new(&unsafe { boa_gc::MutationContext::dummy() }),
+            NativeWeakMap::new(&context.gc()),
         )
         .upcast();
 
@@ -171,7 +171,7 @@ impl WeakMap {
     pub(crate) fn get(
         this: &JsValue,
         args: &[JsValue],
-        _context: &mut Context,
+        context: &mut Context,
     ) -> JsResult<JsValue> {
         // 1. Let M be the this value.
         // 2. Perform ? RequireInternalSlot(M, [[WeakMapData]]).
@@ -194,7 +194,7 @@ impl WeakMap {
         // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]].
         // 6. Return undefined.
         if let Some(entry) = map.get(key.inner())
-            && let Some(val) = entry.value(&unsafe { boa_gc::MutationContext::dummy() })
+            && let Some(val) = entry.value(&context.gc())
         {
             Ok(val.clone())
         } else {
@@ -298,7 +298,7 @@ impl WeakMap {
     pub(crate) fn get_or_insert(
         this: &JsValue,
         args: &[JsValue],
-        _context: &mut Context,
+        context: &mut Context,
     ) -> JsResult<JsValue> {
         // 1. Let M be the this value.
         // 2. Perform ? RequireInternalSlot(M, [[WeakMapData]]).
@@ -325,7 +325,7 @@ impl WeakMap {
 
         // 4. For each Record { [[Key]], [[Value]] } p of M.[[WeakMapData]]
         if let Some(existing) = map.borrow().data().get(key.inner())
-            && let Some(value) = existing.value(&unsafe { boa_gc::MutationContext::dummy() })
+            && let Some(value) = existing.value(&context.gc())
         {
             // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]].
             return Ok(value.clone());
@@ -387,7 +387,7 @@ impl WeakMap {
 
         // 5. For each Record { [[Key]], [[Value]] } p of M.[[WeakMapData]]
         if let Some(existing) = map.borrow().data().get(key_obj.inner())
-            && let Some(value) = existing.value(&unsafe { boa_gc::MutationContext::dummy() })
+            && let Some(value) = existing.value(&context.gc())
         {
             // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]].
             return Ok(value.clone());

core/engine/src/builtins/weak_set/mod.rs

@@ -86,7 +86,7 @@ impl BuiltInConstructor for WeakSet {
         let weak_set = JsObject::from_proto_and_data_with_shared_shape(
             context.root_shape(),
             prototype,
-            NativeWeakSet::new(&unsafe { boa_gc::MutationContext::dummy() }),
+            NativeWeakSet::new(&context.gc()),
         )
         .upcast();
 

core/engine/src/context/mod.rs

@@ -463,6 +463,20 @@ impl Context {
         &self.vm.frame().realm
     }
 
+    /// Returns [`boa_gc::MutationContext`] to allocate on the Gc heap
+    /// (eg. for [`Gc::new`])
+    ///
+    /// # Safety
+    /// Uses `dummy()` as a temporary bridge during the oscars GC migration.
+    /// Todo: replace with a real branding token in future
+    #[inline]
+    #[must_use]
+    pub fn gc(&self) -> boa_gc::MutationContext<'static, '_> {
+        // SAFETY: `MutationContext` is a ZST phantom type, this is sound
+        // under boa's single-threaded GC invariant until migration is complete
+        unsafe { boa_gc::MutationContext::dummy() }
+    }
+
     /// Set the value of trace on the context
     #[cfg(feature = "trace")]
     #[inline]