diff --git a/core/src/avm2/globals.rs b/core/src/avm2/globals.rs index d97dd36a6..384ace3f2 100644 --- a/core/src/avm2/globals.rs +++ b/core/src/avm2/globals.rs @@ -822,7 +822,7 @@ fn load_playerglobal<'gc>( let activation = $activation; $( // Lookup with the highest version, so we we see all defined classes here - let ns = Namespace::package($package, ApiVersion::VM_INTERNAL, &mut activation.borrow_gc()); + let ns = Namespace::package($package, ApiVersion::VM_INTERNAL, activation.context); let name = QName::new(ns, $class_name); let class_object = activation.domain().get_defined_value(activation, name).unwrap_or_else(|e| panic!("Failed to lookup {name:?}: {e:?}")); let class_object = class_object.as_object().unwrap().as_class_object().unwrap(); @@ -837,7 +837,7 @@ fn load_playerglobal<'gc>( let activation = $activation; $( // Lookup with the highest version, so we we see all defined classes here - let ns = Namespace::package($package, ApiVersion::VM_INTERNAL, &mut activation.borrow_gc()); + let ns = Namespace::package($package, ApiVersion::VM_INTERNAL, activation.context); let name = QName::new(ns, $class_name); let class_object = activation.domain().get_defined_value(activation, name).unwrap_or_else(|e| panic!("Failed to lookup {name:?}: {e:?}")); let class_def = class_object.as_object().unwrap().as_class_object().unwrap().inner_class_definition(); diff --git a/core/src/avm2/globals/flash/net/shared_object.rs b/core/src/avm2/globals/flash/net/shared_object.rs index 816494c48..53212e12f 100644 --- a/core/src/avm2/globals/flash/net/shared_object.rs +++ b/core/src/avm2/globals/flash/net/shared_object.rs @@ -1,11 +1,10 @@ //! `flash.net.SharedObject` builtin/prototype -use crate::avm2::api_version::ApiVersion; use crate::avm2::error::error; use crate::avm2::object::TObject; use crate::avm2::Error::AvmError; use crate::avm2::Multiname; -use crate::avm2::{Activation, Error, Namespace, Object, Value}; +use crate::avm2::{Activation, Error, Object, Value}; use crate::string::AvmString; use crate::{avm2_stub_getter, avm2_stub_method, avm2_stub_setter}; use flash_lso::types::{AMFVersion, Lso}; diff --git a/core/src/avm2/globals/namespace.rs b/core/src/avm2/globals/namespace.rs index f47dff8e3..30a5d86fb 100644 --- a/core/src/avm2/globals/namespace.rs +++ b/core/src/avm2/globals/namespace.rs @@ -33,8 +33,7 @@ pub fn instance_init<'gc>( } else { uri.coerce_to_string(activation)? }; - let namespace = - Namespace::package(namespace_uri, api_version, &mut activation.borrow_gc()); + let namespace = Namespace::package(namespace_uri, api_version, activation.context); let prefix_str = prefix.coerce_to_string(activation)?; // The order is important here to match Flash @@ -55,7 +54,7 @@ pub fn instance_init<'gc>( [Value::Object(Object::QNameObject(qname))] => { let ns = qname .uri() - .map(|uri| Namespace::package(uri, api_version, &mut activation.borrow_gc())) + .map(|uri| Namespace::package(uri, api_version, activation.context)) .unwrap_or_else(Namespace::any); if ns.as_uri().is_empty() { (Some("".into()), ns) @@ -68,7 +67,7 @@ pub fn instance_init<'gc>( let ns = Namespace::package( val.coerce_to_string(activation)?, api_version, - &mut activation.borrow_gc(), + activation.context, ); if ns.as_uri().is_empty() { (Some("".into()), ns) diff --git a/core/src/avm2/globals/q_name.rs b/core/src/avm2/globals/q_name.rs index aa637f3d3..216481111 100644 --- a/core/src/avm2/globals/q_name.rs +++ b/core/src/avm2/globals/q_name.rs @@ -62,19 +62,15 @@ pub fn init<'gc>( let namespace = match ns_arg { Value::Object(Object::NamespaceObject(ns)) => Some(ns.namespace()), - Value::Object(Object::QNameObject(qname)) => qname.uri().map(|uri| { - Namespace::package(uri, ApiVersion::AllVersions, &mut activation.borrow_gc()) - }), + Value::Object(Object::QNameObject(qname)) => qname + .uri() + .map(|uri| Namespace::package(uri, ApiVersion::AllVersions, activation.context)), Value::Null => None, - Value::Undefined => Some(Namespace::package( - "", - api_version, - &mut activation.borrow_gc(), - )), + Value::Undefined => Some(activation.avm2().namespaces.public_for(api_version)), v => Some(Namespace::package( v.coerce_to_string(activation)?, api_version, - &mut activation.borrow_gc(), + activation.context, )), }; diff --git a/core/src/avm2/multiname.rs b/core/src/avm2/multiname.rs index 860ed27b6..d2bd5a691 100644 --- a/core/src/avm2/multiname.rs +++ b/core/src/avm2/multiname.rs @@ -1,12 +1,12 @@ use crate::avm2::activation::Activation; use crate::avm2::error::{make_error_1032, make_error_1080}; +use crate::avm2::namespace::{CommonNamespaces, Namespace}; use crate::avm2::script::TranslationUnit; use crate::avm2::Error; use crate::avm2::QName; use crate::avm2::{Object, Value}; use crate::context::GcContext; use crate::string::{AvmString, WStr, WString}; -use crate::avm2::namespace::{CommonNamespaces, Namespace}; use bitflags::bitflags; use gc_arena::Gc; use gc_arena::{Collect, Mutation}; @@ -543,10 +543,7 @@ pub struct CommonMultinames<'gc> { } impl<'gc> CommonMultinames<'gc> { - pub fn new( - context: &mut GcContext<'_, 'gc>, - namespaces: &CommonNamespaces<'gc>, - ) -> Self { + pub fn new(context: &mut GcContext<'_, 'gc>, namespaces: &CommonNamespaces<'gc>) -> Self { let mc = context.gc_context; let mut create_pub_multiname = |local_name: &'static [u8]| -> Gc<'gc, Multiname<'gc>> { diff --git a/core/src/avm2/namespace.rs b/core/src/avm2/namespace.rs index cb130faf6..d10f0e6a0 100644 --- a/core/src/avm2/namespace.rs +++ b/core/src/avm2/namespace.rs @@ -1,7 +1,11 @@ use crate::avm2::activation::Activation; use crate::avm2::Error; +use crate::context::UpdateContext; use crate::string::{AvmAtom, AvmString}; +use crate::utils::weak_set::WeakSet; use crate::{avm2::script::TranslationUnit, context::GcContext}; +use gc_arena::barrier::unlock; +use gc_arena::lock::RefLock; use gc_arena::{Collect, Gc}; use num_traits::FromPrimitive; use ruffle_wstr::WStr; @@ -10,28 +14,62 @@ use swf::avm2::types::{Index, Namespace as AbcNamespace}; use super::api_version::ApiVersion; -#[derive(Clone, Copy, Collect, Debug, PartialEq)] +#[derive(Clone, Copy, Collect, Debug)] #[collect(no_drop)] pub struct Namespace<'gc>( // `None` represents the wildcard namespace `Namespace::any()`. - Option>>, + Option>>>, ); /// Represents the name of a namespace. #[allow(clippy::enum_variant_names)] -#[derive(Copy, Clone, Collect, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Collect, Debug, PartialEq, Eq, Hash)] #[collect(no_drop)] -enum NamespaceData<'gc> { +enum NamespaceData { // note: this is the default "public namespace", corresponding to both // ABC Namespace and PackageNamespace - Namespace(AvmAtom<'gc>, #[collect(require_static)] ApiVersion), - PackageInternal(AvmAtom<'gc>), - Protected(AvmAtom<'gc>), - Explicit(AvmAtom<'gc>), - StaticProtected(AvmAtom<'gc>), - // note: private namespaces are always compared by pointer identity - // of the enclosing `Gc`. - Private(AvmAtom<'gc>), + Namespace(N, #[collect(require_static)] ApiVersion), + PackageInternal(N), + Protected(N), + Explicit(N), + StaticProtected(N), + // note: private namespaces are the only kind that do not get interned, + // so that each instance is considered separate. + Private(N), +} + +type NamespaceInterner<'gc> = WeakSet<'gc, NamespaceData>>; + +impl NamespaceData { + fn map(self, f: impl FnOnce(N) -> M) -> NamespaceData { + match self { + Self::Namespace(n, v) => NamespaceData::Namespace(f(n), v), + Self::PackageInternal(n) => NamespaceData::PackageInternal(f(n)), + Self::Protected(n) => NamespaceData::Protected(f(n)), + Self::Explicit(n) => NamespaceData::Explicit(f(n)), + Self::StaticProtected(n) => NamespaceData::StaticProtected(f(n)), + Self::Private(n) => NamespaceData::Private(f(n)), + } + } +} + +// TODO(moulins): allow passing an AvmAtom or a non-static `&WStr` directly +impl<'gc, N: Into>> NamespaceData { + fn intern( + self, + interner: &mut NamespaceInterner<'gc>, + context: &mut GcContext<'_, 'gc>, + ) -> Namespace<'gc> { + debug_assert!( + !matches!(self, NamespaceData::Private(_)), + "private namespaces shouldn't be interned" + ); + + let mc = context.gc(); + let raw = self.map(|name| context.interner.intern(mc, name.into())); + let entry = interner.entry(mc, &raw).or_insert(|| Gc::new(mc, raw)); + Namespace(Some(entry.get())) + } } fn strip_version_mark(url: &WStr, is_playerglobals: bool) -> Option<(&WStr, ApiVersion)> { @@ -114,7 +152,7 @@ impl<'gc> Namespace<'gc> { let mut namespace_name = translation_unit.pool_string(index.0, &mut activation.borrow_gc())?; - // Private namespaces don't get any of the namespace version checks + // Private namespaces don't get any of the namespace version checks, and shouldn't be interned. if let AbcNamespace::Private(_) = abc_namespace { return Ok(Self(Some(Gc::new( mc, @@ -217,7 +255,8 @@ impl<'gc> Namespace<'gc> { AbcNamespace::StaticProtected(_) => NamespaceData::StaticProtected(namespace_name), AbcNamespace::Private(_) => unreachable!(), }; - Ok(Self(Some(Gc::new(mc, ns)))) + + Ok(Self::intern(ns, activation.context)) } pub fn any() -> Self { @@ -228,29 +267,19 @@ impl<'gc> Namespace<'gc> { pub fn package( package_name: impl Into>, api_version: ApiVersion, - context: &mut GcContext<'_, 'gc>, + context: &mut UpdateContext<'gc>, ) -> Self { - let atom = context - .interner - .intern(context.gc_context, package_name.into()); - Self(Some(Gc::new( - context.gc_context, - NamespaceData::Namespace(atom, api_version), - ))) + Self::intern(NamespaceData::Namespace(package_name, api_version), context) } // TODO(moulins): allow passing an AvmAtom or a non-static `&WStr` directly - pub fn internal( - package_name: impl Into>, - context: &mut GcContext<'_, 'gc>, - ) -> Self { - let atom = context - .interner - .intern(context.gc_context, package_name.into()); - Self(Some(Gc::new( - context.gc_context, - NamespaceData::PackageInternal(atom), - ))) + fn intern( + raw: NamespaceData>>, + context: &mut UpdateContext<'gc>, + ) -> Namespace<'gc> { + let namespaces = Gc::write(context.gc(), context.avm2.namespaces); + let interner = unlock!(namespaces, CommonNamespaces, interner); + raw.intern(&mut interner.borrow_mut(), &mut context.borrow_gc()) } pub fn is_public(&self) -> bool { @@ -298,13 +327,9 @@ impl<'gc> Namespace<'gc> { /// Namespace does not implement `PartialEq`, so that each caller is required /// to explicitly choose either `exact_version_match` or `matches_ns`. pub fn exact_version_match(&self, other: Self) -> bool { - if self.0.map(Gc::as_ptr) == other.0.map(Gc::as_ptr) { - true - } else if self.is_private() || other.is_private() { - false - } else { - self.0 == other.0 - } + // Namespaces are interned (unless they are private), so comparing Gc pointers + // is enough to determine equality. + self.0.map(Gc::as_ptr) == other.0.map(Gc::as_ptr) } /// Compares this namespace to another, considering them equal if this namespace's version @@ -358,37 +383,54 @@ pub struct CommonNamespaces<'gc> { pub(super) flash_net_internal: Namespace<'gc>, pub(super) __ruffle__: Namespace<'gc>, + + interner: RefLock>, } impl<'gc> CommonNamespaces<'gc> { const PUBLIC_LEN: usize = ApiVersion::VM_INTERNAL as usize + 1; pub fn new(context: &mut GcContext<'_, 'gc>) -> Self { + let mut interner = WeakSet::new(); + let mut intern = |raw: NamespaceData<&'static [u8]>| { + let raw = raw.map(WStr::from_units); + raw.intern(&mut interner, context) + }; + Self { public_namespaces: std::array::from_fn(|val| { - Namespace::package("", ApiVersion::from_usize(val).unwrap(), context) + let version = ApiVersion::from_usize(val).unwrap(); + intern(NamespaceData::Namespace(b"", version)) }), - internal: Namespace::internal("", context), - as3: Namespace::package( - "http://adobe.com/AS3/2006/builtin", + internal: intern(NamespaceData::PackageInternal(b"")), + as3: intern(NamespaceData::Namespace( + b"http://adobe.com/AS3/2006/builtin", ApiVersion::AllVersions, - context, - ), - vector_public: Namespace::package("__AS3__.vec", ApiVersion::AllVersions, context), - vector_internal: Namespace::internal("__AS3__.vec", context), - proxy: Namespace::package( - "http://www.adobe.com/2006/actionscript/flash/proxy", + )), + vector_public: intern(NamespaceData::Namespace( + b"__AS3__.vec", ApiVersion::AllVersions, - context, - ), - flash_display_internal: Namespace::internal("flash.display", context), - flash_utils_internal: Namespace::internal("flash.utils", context), - flash_geom_internal: Namespace::internal("flash.geom", context), - flash_events_internal: Namespace::internal("flash.events", context), - flash_text_engine_internal: Namespace::internal("flash.text.engine", context), - flash_net_internal: Namespace::internal("flash.net", context), + )), + vector_internal: intern(NamespaceData::PackageInternal(b"__AS3__.vec")), + proxy: intern(NamespaceData::Namespace( + b"http://www.adobe.com/2006/actionscript/flash/proxy", + ApiVersion::AllVersions, + )), + flash_display_internal: intern(NamespaceData::PackageInternal(b"flash.display")), + flash_utils_internal: intern(NamespaceData::PackageInternal(b"flash.utils")), + flash_geom_internal: intern(NamespaceData::PackageInternal(b"flash.geom")), + flash_events_internal: intern(NamespaceData::PackageInternal(b"flash.events")), + flash_text_engine_internal: intern(NamespaceData::PackageInternal( + b"flash.text.engine", + )), + flash_net_internal: intern(NamespaceData::PackageInternal(b"flash.net")), - __ruffle__: Namespace::package("__ruffle__", ApiVersion::AllVersions, context), + __ruffle__: intern(NamespaceData::Namespace( + b"__ruffle__", + ApiVersion::AllVersions, + )), + + interner: RefLock::new(interner), } } diff --git a/core/src/avm2/object/xml_list_object.rs b/core/src/avm2/object/xml_list_object.rs index 77a7206cb..81c48716b 100644 --- a/core/src/avm2/object/xml_list_object.rs +++ b/core/src/avm2/object/xml_list_object.rs @@ -191,11 +191,9 @@ impl<'gc> XmlListObject<'gc> { if !matches!(*last_node.kind(), E4XNodeKind::ProcessingInstruction(_)) { if let Some(name) = last_node.local_name() { let ns = match last_node.namespace() { - Some(ns) => Namespace::package( - ns.uri, - ApiVersion::AllVersions, - &mut activation.context.borrow_gc(), - ), + Some(ns) => { + Namespace::package(ns.uri, ApiVersion::AllVersions, activation.context) + } None => activation.avm2().namespaces.public_all(), }; diff --git a/core/src/avm2/qname.rs b/core/src/avm2/qname.rs index b661e44cc..1549c305a 100644 --- a/core/src/avm2/qname.rs +++ b/core/src/avm2/qname.rs @@ -96,7 +96,7 @@ impl<'gc> QName<'gc> { let package_name = context.interner.intern_wstr(context.gc(), package_name); Self { - ns: Namespace::package(package_name, api_version, &mut context.borrow_gc()), + ns: Namespace::package(package_name, api_version, context), name: AvmString::new(context.gc(), local_name), } } else {