diff --git a/Jint.Tests/Runtime/InteropTests.cs b/Jint.Tests/Runtime/InteropTests.cs index 775718cec..58e7580ba 100644 --- a/Jint.Tests/Runtime/InteropTests.cs +++ b/Jint.Tests/Runtime/InteropTests.cs @@ -1181,6 +1181,29 @@ public void ExtractedClrLengthGetterCalledWithForeignThisThrowsTypeError() Assert.True(result.AsBoolean()); } + [Fact] + public void LazilyMaterializedLengthPropertyBehavesLikeEagerOne() + { + _engine.SetValue("list", new List { "a", "b" }); + _engine.SetValue("array", new[] { 1, 2, 3 }); + + // plain reads (served by the ICollection fast path) and own-property reflection agree + Assert.Equal(2, _engine.Evaluate("list.length").AsNumber()); + Assert.Equal(3, _engine.Evaluate("array.length").AsNumber()); + Assert.True(_engine.Evaluate("'length' in list").AsBoolean()); + Assert.True(_engine.Evaluate("list.hasOwnProperty('length')").AsBoolean()); + Assert.True(_engine.Evaluate("array.hasOwnProperty('length')").AsBoolean()); + + // the accessor descriptor keeps its eager-era shape + Assert.Equal("function", _engine.Evaluate("typeof Object.getOwnPropertyDescriptor(list, 'length').get").AsString()); + Assert.True(_engine.Evaluate("Object.getOwnPropertyDescriptor(list, 'length').configurable").AsBoolean()); + Assert.True(_engine.Evaluate("Object.getOwnPropertyDescriptor(list, 'length').set === undefined").AsBoolean()); + + // deleting the forwarder removes the own property for good + Assert.True(_engine.Evaluate("delete list.length").AsBoolean()); + Assert.False(_engine.Evaluate("list.hasOwnProperty('length')").AsBoolean()); + } + [Fact] public void ForOfOverRevokedProxyOfClrListThrowsTypeError() { diff --git a/Jint/Runtime/Interop/ObjectWrapper.Specialized.cs b/Jint/Runtime/Interop/ObjectWrapper.Specialized.cs index 3bffc9675..47cbce531 100644 --- a/Jint/Runtime/Interop/ObjectWrapper.Specialized.cs +++ b/Jint/Runtime/Interop/ObjectWrapper.Specialized.cs @@ -6,6 +6,34 @@ namespace Jint.Runtime.Interop; +/// +/// Instantiated once per exposed CLR type (see ObjectWrapper's array-like resolution cache) so that +/// per-wrapper creation is a plain virtual call and constructor invocation instead of +/// Activator.CreateInstance with argument binding. +/// +internal abstract class ArrayLikeWrapperFactory +{ + public abstract ArrayLikeWrapper Create(Engine engine, object target, Type type); +} + +internal sealed class ArrayWrapperFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicFields)] T> : ArrayLikeWrapperFactory +{ + public override ArrayLikeWrapper Create(Engine engine, object target, Type type) + => new ArrayWrapper(engine, (T[]) target, type); +} + +internal sealed class GenericListWrapperFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicFields)] T> : ArrayLikeWrapperFactory +{ + public override ArrayLikeWrapper Create(Engine engine, object target, Type type) + => new GenericListWrapper(engine, (IList) target, type); +} + +internal sealed class ReadOnlyListWrapperFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicFields)] T> : ArrayLikeWrapperFactory +{ + public override ArrayLikeWrapper Create(Engine engine, object target, Type type) + => new ReadOnlyListWrapper(engine, (IReadOnlyList) target, type); +} + internal abstract class ArrayLikeWrapper : ObjectWrapper { protected ArrayLikeWrapper( diff --git a/Jint/Runtime/Interop/ObjectWrapper.cs b/Jint/Runtime/Interop/ObjectWrapper.cs index 689462d10..b1379f7d4 100644 --- a/Jint/Runtime/Interop/ObjectWrapper.cs +++ b/Jint/Runtime/Interop/ObjectWrapper.cs @@ -22,6 +22,7 @@ namespace Jint.Runtime.Interop; public class ObjectWrapper : ObjectInstance, IObjectWrapper, IEquatable { internal readonly TypeDescriptor _typeDescriptor; + private bool _lengthPropertyPending; internal ObjectWrapper( Engine engine, @@ -39,10 +40,10 @@ internal ObjectWrapper( if (_typeDescriptor.LengthProperty is not null) { - // create a forwarder to produce length from Count or Length if one of them is present - var functionInstance = new ClrFunction(engine, "length", GetLength); - var descriptor = new GetSetPropertyDescriptor(functionInstance, Undefined, PropertyFlag.Configurable); - SetProperty(KnownKeys.Length, descriptor); + // the "length" forwarder (produced from Count or Length) is materialized lazily on first + // own-property consultation: plain length reads are served by the ICollection fast path in + // Get, so most wrappers never observe the descriptor itself + _lengthPropertyPending = true; if (_typeDescriptor.IsArrayLike && engine.Options.Interop.AttachArrayPrototype) { @@ -132,7 +133,7 @@ public static ObjectInstance Create(Engine engine, object target, Type? type = n return new ObjectWrapper(engine, target, type); } - private static readonly ConcurrentDictionary _arrayLikeWrapperResolution = new(); + private static readonly ConcurrentDictionary _arrayLikeWrapperResolution = new(); private static bool TryBuildArrayLikeWrapper( Engine engine, @@ -142,12 +143,17 @@ private static bool TryBuildArrayLikeWrapper( { result = null; - var arrayWrapperType = _arrayLikeWrapperResolution.GetOrAdd(type, static t => + // resolved once per exposed type: reflection (interface scan + generic instantiation + + // Activator) runs only on the first sighting, every later wrapper creation is a single + // virtual call into the cached factory + var factory = _arrayLikeWrapperResolution.GetOrAdd(type, static t => { #pragma warning disable IL2055 #pragma warning disable IL2070 #pragma warning disable IL3050 + Type? factoryType = null; + // single-rank zero-based CLR arrays (T[]) get a fixed-size live wrapper; T[] implements // IList and would otherwise flow into GenericListWrapper below, whose growth paths // call IList.Add and would leak NotSupportedException from the underlying array. @@ -155,52 +161,57 @@ private static bool TryBuildArrayLikeWrapper( // (T[*]) arrays, which keep their previous handling. if (t.IsArray && t.GetElementType() is { } elementType && t == elementType.MakeArrayType()) { - return typeof(ArrayWrapper<>).MakeGenericType(elementType); + factoryType = typeof(ArrayWrapperFactory<>).MakeGenericType(elementType); } - - // check for generic interfaces - foreach (var i in t.GetInterfaces()) + else { - if (!i.IsGenericType) + // check for generic interfaces + foreach (var i in t.GetInterfaces()) { - continue; - } + if (!i.IsGenericType) + { + continue; + } - var arrayItemType = i.GenericTypeArguments[0]; + var arrayItemType = i.GenericTypeArguments[0]; - if (i.GetGenericTypeDefinition() == typeof(IList<>)) - { - return typeof(GenericListWrapper<>).MakeGenericType(arrayItemType); - } + if (i.GetGenericTypeDefinition() == typeof(IList<>)) + { + factoryType = typeof(GenericListWrapperFactory<>).MakeGenericType(arrayItemType); + break; + } - if (i.GetGenericTypeDefinition() == typeof(IReadOnlyList<>)) - { - return typeof(ReadOnlyListWrapper<>).MakeGenericType(arrayItemType); + if (i.GetGenericTypeDefinition() == typeof(IReadOnlyList<>)) + { + factoryType = typeof(ReadOnlyListWrapperFactory<>).MakeGenericType(arrayItemType); + break; + } } } #pragma warning restore IL3050 #pragma warning restore IL2070 #pragma warning restore IL2055 - return null; - }); + if (factoryType is null) + { + return null; + } - if (arrayWrapperType is not null) - { // Activator.CreateInstance may fail in trimmed/AOT scenarios where the constructor // was removed by the linker - fall back to the non-generic ListWrapper in that case try { - result = (ArrayLikeWrapper) Activator.CreateInstance(arrayWrapperType, engine, target, type)!; + return (ArrayLikeWrapperFactory) Activator.CreateInstance(factoryType)!; } catch (MissingMethodException) { - // Constructor was trimmed, fall back to non-generic wrapper - if (target is IList list) - { - result = new ListWrapper(engine, list, type); - } + return null; } + }); + + if (factory is not null) + { + result = factory.Create(engine, target, type); } else if (target is IList list) { @@ -226,6 +237,10 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) if (property is JsString stringKey) { var member = stringKey.ToString(); + if (_lengthPropertyPending && string.Equals(member, "length", StringComparison.Ordinal)) + { + MaterializeLengthProperty(); + } if (_properties is null || !_properties.ContainsKey(member)) { // can try utilize fast path @@ -348,6 +363,13 @@ public override bool DefineOwnProperty(JsValue property, PropertyDescriptor desc public override void RemoveOwnProperty(JsValue property) { + if (_lengthPropertyPending && CommonProperties.Length.Equals(property)) + { + // an explicit removal of the not-yet-materialized forwarder must behave like removing the + // eagerly-created one did: the property is gone and does not come back + _lengthPropertyPending = false; + } + if (_engine.Options.Interop.AllowWrite) { if (property is JsString jsString && _typeDescriptor.IsStringKeyedGenericDictionary) @@ -628,6 +650,11 @@ private PropertyDescriptor GetOwnProperty(JsValue property, bool mustBeReadable, return x; } + if (_lengthPropertyPending && CommonProperties.Length.Equals(property)) + { + return MaterializeLengthProperty(); + } + // if we have array-like or dictionary or expando, we can provide iterator if (property.IsSymbol()) { @@ -832,6 +859,16 @@ private static JsValue Iterator(JsValue thisObject, JsCallArguments arguments) : new EnumerableIterator(wrapper._engine, (IEnumerable) wrapper.Target); } + private GetSetPropertyDescriptor MaterializeLengthProperty() + { + _lengthPropertyPending = false; + // create a forwarder to produce length from Count or Length if one of them is present + var functionInstance = new ClrFunction(_engine, "length", GetLength); + var descriptor = new GetSetPropertyDescriptor(functionInstance, Undefined, PropertyFlag.Configurable); + SetProperty(KnownKeys.Length, descriptor); + return descriptor; + } + private static JsNumber GetLength(JsValue thisObject, JsCallArguments arguments) { var wrapper = UnwrapReceiver(thisObject, "length");