From 4773591c42fe1227ec8d086c096f0709ceac2eb1 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Tue, 30 Jul 2024 16:03:34 -0700 Subject: [PATCH] avm2: Use `Gc` instead of `GcCell` in `ClassObject` --- core/src/avm2/object.rs | 2 +- core/src/avm2/object/class_object.rs | 139 ++++++++++++-------------- core/src/display_object/movie_clip.rs | 6 +- 3 files changed, 66 insertions(+), 81 deletions(-) diff --git a/core/src/avm2/object.rs b/core/src/avm2/object.rs index dd3e1551d..3c309fb1b 100644 --- a/core/src/avm2/object.rs +++ b/core/src/avm2/object.rs @@ -1390,7 +1390,7 @@ impl<'gc> Object<'gc> { Self::RegExpObject(o) => WeakObject::RegExpObject(RegExpObjectWeak(Gc::downgrade(o.0))), Self::ByteArrayObject(o) => WeakObject::ByteArrayObject(ByteArrayObjectWeak(Gc::downgrade(o.0))), Self::LoaderInfoObject(o) => WeakObject::LoaderInfoObject(LoaderInfoObjectWeak(GcCell::downgrade(o.0))), - Self::ClassObject(o) => WeakObject::ClassObject(ClassObjectWeak(GcCell::downgrade(o.0))), + Self::ClassObject(o) => WeakObject::ClassObject(ClassObjectWeak(Gc::downgrade(o.0))), Self::VectorObject(o) => WeakObject::VectorObject(VectorObjectWeak(Gc::downgrade(o.0))), Self::SoundObject(o) => WeakObject::SoundObject(SoundObjectWeak(GcCell::downgrade(o.0))), Self::SoundChannelObject(o) => WeakObject::SoundChannelObject(SoundChannelObjectWeak(GcCell::downgrade(o.0))), diff --git a/core/src/avm2/object/class_object.rs b/core/src/avm2/object/class_object.rs index 034551409..e7d0fd66a 100644 --- a/core/src/avm2/object/class_object.rs +++ b/core/src/avm2/object/class_object.rs @@ -18,38 +18,42 @@ use crate::avm2::QName; use crate::avm2::TranslationUnit; use crate::string::AvmString; use fnv::FnvHashMap; -use gc_arena::{Collect, GcCell, GcWeakCell, Mutation}; -use std::cell::{BorrowError, Ref, RefMut}; +use gc_arena::barrier::unlock; +use gc_arena::{ + lock::{Lock, RefLock}, + Collect, Gc, GcWeak, Mutation, +}; +use std::cell::{Ref, RefMut}; use std::fmt::Debug; use std::hash::{Hash, Hasher}; /// An Object which can be called to execute its function code. #[derive(Collect, Clone, Copy)] #[collect(no_drop)] -pub struct ClassObject<'gc>(pub GcCell<'gc, ClassObjectData<'gc>>); +pub struct ClassObject<'gc>(pub Gc<'gc, ClassObjectData<'gc>>); #[derive(Collect, Clone, Copy, Debug)] #[collect(no_drop)] -pub struct ClassObjectWeak<'gc>(pub GcWeakCell<'gc, ClassObjectData<'gc>>); +pub struct ClassObjectWeak<'gc>(pub GcWeak<'gc, ClassObjectData<'gc>>); #[derive(Collect, Clone)] #[collect(no_drop)] pub struct ClassObjectData<'gc> { /// Base script object - base: ScriptObjectData<'gc>, + base: RefLock>, /// The class associated with this class object. class: Class<'gc>, /// The associated prototype. /// Should always be non-None after initialization. - prototype: Option>, + prototype: Lock>>, /// The captured scope that all class traits will use. class_scope: ScopeChain<'gc>, /// The captured scope that all instance traits will use. - instance_scope: ScopeChain<'gc>, + instance_scope: Lock>, /// The base class of this one. /// @@ -65,7 +69,7 @@ pub struct ClassObjectData<'gc> { /// as `None` here. AVM2 considers both applications to be separate /// classes, though we consider the parameter to be the class `Object` when /// we get a param of `null`. - applications: FnvHashMap>, ClassObject<'gc>>, + applications: RefLock>, ClassObject<'gc>>>, /// VTable used for instances of this class. instance_vtable: VTable<'gc>, @@ -160,30 +164,31 @@ impl<'gc> ClassObject<'gc> { } } - let class_object = ClassObject(GcCell::new( + let mc = activation.context.gc_context; + + let class_object = ClassObject(Gc::new( activation.context.gc_context, ClassObjectData { - base: ScriptObjectData::custom_new(c_class, None, None), + base: RefLock::new(ScriptObjectData::custom_new(c_class, None, None)), class, - prototype: None, + prototype: Lock::new(None), class_scope: scope, - instance_scope: scope, + instance_scope: Lock::new(scope), superclass_object, - applications: Default::default(), - instance_vtable: VTable::empty(activation.context.gc_context), + applications: RefLock::new(Default::default()), + instance_vtable: VTable::empty(mc), }, )); // instance scope = [..., class object] - let instance_scope = scope.chain( - activation.context.gc_context, - &[Scope::new(class_object.into())], - ); + let instance_scope = scope.chain(mc, &[Scope::new(class_object.into())]); - class_object - .0 - .write(activation.context.gc_context) - .instance_scope = instance_scope; + unlock!( + Gc::write(mc, class_object.0), + ClassObjectData, + instance_scope + ) + .set(instance_scope); class_object.init_instance_vtable(activation)?; class.add_class_object(activation.context.gc_context, class_object); @@ -261,13 +266,11 @@ impl<'gc> ClassObject<'gc> { activation: &mut Activation<'_, 'gc>, class_proto: Object<'gc>, ) -> Result<(), Error<'gc>> { - self.0.write(activation.context.gc_context).prototype = Some(class_proto); + let mc = activation.context.gc_context; + + unlock!(Gc::write(mc, self.0), ClassObjectData, prototype).set(Some(class_proto)); class_proto.set_string_property_local("constructor", self.into(), activation)?; - class_proto.set_local_property_is_enumerable( - activation.context.gc_context, - "constructor".into(), - false, - ); + class_proto.set_local_property_is_enumerable(mc, "constructor".into(), false); Ok(()) } @@ -309,9 +312,9 @@ impl<'gc> ClassObject<'gc> { /// `Class` and `Object`. All other types should pull `Class`'s prototype /// and type object from the `Avm2` instance. pub fn link_type(self, gc_context: &Mutation<'gc>, proto: Object<'gc>) { - let mut write = self.0.write(gc_context); - - write.base.set_proto(proto); + unlock!(Gc::write(gc_context, self.0), ClassObjectData, base) + .borrow_mut() + .set_proto(proto); } /// Run the class's initializer method. @@ -321,7 +324,7 @@ impl<'gc> ClassObject<'gc> { ) -> Result<(), Error<'gc>> { let object: Object<'gc> = self.into(); - let scope = self.0.read().class_scope; + let scope = self.0.class_scope; let c_class = self .inner_class_definition() .c_class() @@ -348,7 +351,7 @@ impl<'gc> ClassObject<'gc> { arguments: &[Value<'gc>], activation: &mut Activation<'_, 'gc>, ) -> Result, Error<'gc>> { - let scope = self.0.read().instance_scope; + let scope = self.0.instance_scope.get(); let method = self.constructor(); exec( method, @@ -372,7 +375,7 @@ impl<'gc> ClassObject<'gc> { arguments: &[Value<'gc>], activation: &mut Activation<'_, 'gc>, ) -> Result, Error<'gc>> { - let scope = self.0.read().instance_scope; + let scope = self.0.instance_scope.get(); let method = self.native_constructor(); exec( method, @@ -603,11 +606,13 @@ impl<'gc> ClassObject<'gc> { pub fn add_application( &self, - gc_context: &Mutation<'gc>, + mc: &Mutation<'gc>, param: Option>, cls: ClassObject<'gc>, ) { - self.0.write(gc_context).applications.insert(param, cls); + unlock!(Gc::write(mc, self.0), ClassObjectData, applications) + .borrow_mut() + .insert(param, cls); } /// Parametrize this class. This does not check to ensure that this class is generic. @@ -618,7 +623,7 @@ impl<'gc> ClassObject<'gc> { ) -> Result, Error<'gc>> { let self_class = self.inner_class_definition(); - if let Some(application) = self.0.read().applications.get(&class_param) { + if let Some(application) = self.0.applications.borrow().get(&class_param) { return Ok(*application); } @@ -636,10 +641,13 @@ impl<'gc> ClassObject<'gc> { let class_object = Self::from_class(activation, parameterized_class, Some(vector_star_cls))?; - self.0 - .write(activation.context.gc_context) - .applications - .insert(class_param, class_object); + unlock!( + Gc::write(activation.context.gc_context, self.0), + ClassObjectData, + applications + ) + .borrow_mut() + .insert(class_param, class_object); Ok(class_object) } @@ -665,36 +673,27 @@ impl<'gc> ClassObject<'gc> { } pub fn instance_vtable(self) -> VTable<'gc> { - self.0.read().instance_vtable - } - - /// Like `inner_class_definition`, but returns an `Err(BorrowError)` instead of panicking - /// if our `GcCell` is already mutably borrowed. This is useful - /// in contexts where panicking would be extremely undesirable, - /// and there's a fallback if we cannot obtain the `Class` - /// (such as `Debug` impls), - pub fn try_inner_class_definition(&self) -> Result, BorrowError> { - self.0.try_read().map(|c| c.class) + self.0.instance_vtable } pub fn inner_class_definition(self) -> Class<'gc> { - self.0.read().class + self.0.class } pub fn prototype(self) -> Object<'gc> { - self.0.read().prototype.unwrap() + self.0.prototype.get().unwrap() } pub fn class_scope(self) -> ScopeChain<'gc> { - self.0.read().class_scope + self.0.class_scope } pub fn instance_scope(self) -> ScopeChain<'gc> { - self.0.read().instance_scope + self.0.instance_scope.get() } pub fn superclass_object(self) -> Option> { - self.0.read().superclass_object + self.0.superclass_object } fn instance_allocator(self) -> AllocatorFn { @@ -709,9 +708,7 @@ impl<'gc> ClassObject<'gc> { /// we need infallible access to *something* to print /// out. pub fn debug_class_name(&self) -> Box { - let class_name = self - .try_inner_class_definition() - .and_then(|class| class.try_name()); + let class_name = self.inner_class_definition().try_name(); match class_name { Ok(class_name) => Box::new(class_name), @@ -722,21 +719,21 @@ impl<'gc> ClassObject<'gc> { impl<'gc> TObject<'gc> for ClassObject<'gc> { fn base(&self) -> Ref> { - Ref::map(self.0.read(), |read| &read.base) + self.0.base.borrow() } fn base_mut(&self, mc: &Mutation<'gc>) -> RefMut> { - RefMut::map(self.0.write(mc), |write| &mut write.base) + unlock!(Gc::write(mc, self.0), ClassObjectData, base).borrow_mut() } fn as_ptr(&self) -> *const ObjectPtr { - self.0.as_ptr() as *const ObjectPtr + Gc::as_ptr(self.0) as *const ObjectPtr } fn to_string(&self, activation: &mut Activation<'_, 'gc>) -> Result, Error<'gc>> { Ok(AvmString::new_utf8( activation.context.gc_context, - format!("[class {}]", self.0.read().class.name().local_name()), + format!("[class {}]", self.0.class.name().local_name()), ) .into()) } @@ -759,7 +756,7 @@ impl<'gc> TObject<'gc> for ClassObject<'gc> { activation: &mut Activation<'_, 'gc>, ) -> Result, Error<'gc>> { if let Some(call_handler) = self.call_handler() { - let scope = self.0.read().class_scope; + let scope = self.0.class_scope; exec( call_handler, scope, @@ -803,18 +800,6 @@ impl<'gc> TObject<'gc> for ClassObject<'gc> { Some(*self) } - fn set_local_property_is_enumerable( - &self, - mc: &Mutation<'gc>, - name: AvmString<'gc>, - is_enumerable: bool, - ) { - self.0 - .write(mc) - .base - .set_local_property_is_enumerable(name, is_enumerable); - } - fn apply( &self, activation: &mut Activation<'_, 'gc>, @@ -887,7 +872,7 @@ impl<'gc> Debug for ClassObject<'gc> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { f.debug_struct("ClassObject") .field("name", &self.debug_class_name()) - .field("ptr", &self.0.as_ptr()) + .field("ptr", &Gc::as_ptr(self.0)) .finish() } } diff --git a/core/src/display_object/movie_clip.rs b/core/src/display_object/movie_clip.rs index 9d20710b6..e4a55e317 100644 --- a/core/src/display_object/movie_clip.rs +++ b/core/src/display_object/movie_clip.rs @@ -2208,9 +2208,9 @@ impl<'gc> MovieClip<'gc> { "Got \"{:?}\" when constructing AVM2 side of movie clip of type {}", e, class_object - .try_inner_class_definition() - .map(|c| c.name().to_qualified_name(context.gc_context)) - .unwrap_or_else(|_| "[BorrowError!]".into()) + .inner_class_definition() + .name() + .to_qualified_name(context.gc_context) ); } }