Harden LiveView array wrappers and interop wrapper caches for 4.14

Jint.Tests/Runtime/InteropTests.ArrayIdentityCache.cs

@@ -1,3 +1,5 @@
+using Jint.Runtime.Interop;
+
 namespace Jint.Tests.Runtime;
 
 public partial class InteropTests
@@ -58,4 +60,39 @@ public void ArrayConversionReusesSnapshotWithRecentObjectWrapperCache()
 
         Assert.True(engine.Evaluate("host.Numbers === host.Numbers").AsBoolean());
     }
+
+    [Fact]
+    public void IdentityMapHandlesExposedTypeChangeForArrays()
+    {
+        // the identity map may hold a wrapper for a different exposed view of the same array;
+        // a type-guard miss must replace the entry (last view wins), not throw on Add
+        var engine = new Engine(options =>
+        {
+            options.Interop.TrackObjectWrapperIdentity = true;
+            options.Interop.WrapObjectHandler = static (e, target, type) =>
+                ObjectWrapper.Create(e, target, target is int[] ? typeof(System.Collections.IList) : type);
+        });
+
+        var array = new[] { 1, 2, 3 };
+        engine.SetValue("a", array);
+        Assert.Equal(1, engine.Evaluate("a[0]").AsNumber());
+
+        engine.Options.Interop.WrapObjectHandler = static (e, target, type) => ObjectWrapper.Create(e, target, type);
+        engine.SetValue("b", array);
+        Assert.Equal(1, engine.Evaluate("b[0]").AsNumber());
+    }
+
+    [Fact]
+    public void DisposeReleasesRecentWrapperCache()
+    {
+        // the recent-wrapper ring is on by default and strongly roots its targets;
+        // Dispose must release them even when the opt-in identity map was never created
+        var engine = new Engine();
+        engine.SetValue("host", new ArrayConversionHost());
+        engine.Evaluate("host.Numbers[0]");
+        Assert.NotNull(engine._recentObjectWrapperCache);
+
+        engine.Dispose();
+        Assert.Null(engine._recentObjectWrapperCache);
+    }
 }

Jint.Tests/Runtime/InteropTests.ClrArrayLiveView.cs

@@ -65,9 +65,95 @@ public void LiveViewElementReadsConvertPrimitiveItemTypes()
         Assert.Equal(8, engine.Evaluate("intList[1]").AsNumber());
         Assert.True(engine.Evaluate("stringList[1] === null").AsBoolean());
 
-        // out-of-range keeps the general converter's result
-        Assert.True(engine.Evaluate("ints[99] === null").AsBoolean());
-        Assert.True(engine.Evaluate("intList[99] === null").AsBoolean());
+        // out-of-range and negative indices read like JS array holes
+        Assert.True(engine.Evaluate("ints[99] === undefined").AsBoolean());
+        Assert.True(engine.Evaluate("ints[-1] === undefined").AsBoolean());
+        Assert.True(engine.Evaluate("intList[99] === undefined").AsBoolean());
+    }
+
+    [Fact]
+    public void LiveViewHonorsNonArrayDeclaredType()
+    {
+        var engine = new Engine();
+        var host = new ReadOnlyDeclaredArrayHolder();
+        engine.SetValue("h", host);
+
+        // reads are live views over the underlying array
+        Assert.Equal(2, engine.Evaluate("h.Data[1]").AsNumber());
+        host.Mutate(1, 42);
+        Assert.Equal(42, engine.Evaluate("h.Data[1]").AsNumber());
+
+        // an array exposed under a read-only declared type must not produce a writable view
+        engine.Evaluate("h.Data[0] = 99;");
+        Assert.Equal(1, host.Raw[0]);
+        var ex = Assert.Throws<JavaScriptException>(() => engine.Evaluate("'use strict'; h.Data[0] = 99;"));
+        Assert.Contains("read only", ex.Message, StringComparison.OrdinalIgnoreCase);
+        Assert.Equal(1, host.Raw[0]);
+    }
+
+    private class ReadOnlyDeclaredArrayHolder
+    {
+        private readonly int[] _data = [1, 2, 3];
+        public IReadOnlyList<int> Data => _data;
+        public int[] Raw => _data;
+        public void Mutate(int index, int value) => _data[index] = value;
+    }
+
+    [Fact]
+    public void ArrayUnderNonArrayDeclaredTypeDoesNotPoisonTypeMapper()
+    {
+        // the static type-mapper memoization must not register the array lane under a non-array
+        // declared type: a later non-array value of the same declared type would hit an
+        // InvalidCastException — and poison every engine in the process
+        var engine = new Engine();
+        engine.SetValue("a", new EnumerableHolder { Data = [1] });
+        Assert.Equal(1, engine.Evaluate("a.Data[0]").AsNumber());
+
+        engine.SetValue("b", new EnumerableHolder { Data = new List<int> { 2 } });
+        Assert.Equal(2, engine.Evaluate("b.Data[0]").AsNumber());
+    }
+
+    private class EnumerableHolder
+    {
+        public IEnumerable<int> Data { get; set; }
+    }
+
+    [Fact]
+    public void InOperatorMatchesJsArraySemantics()
+    {
+        var engine = new Engine();
+        engine.SetValue("a", new[] { 1, 2, 3 });
+        engine.SetValue("l", new List<int> { 1, 2, 3 });
+
+        Assert.True(engine.Evaluate("0 in a").AsBoolean());
+        Assert.True(engine.Evaluate("2 in a").AsBoolean());
+        Assert.False(engine.Evaluate("3 in a").AsBoolean());
+        Assert.False(engine.Evaluate("-1 in a").AsBoolean());
+        Assert.False(engine.Evaluate("'-1' in a").AsBoolean());
+        Assert.False(engine.Evaluate("4000000000 in a").AsBoolean());
+        Assert.True(engine.Evaluate("'1' in a").AsBoolean());
+        Assert.False(engine.Evaluate("'08' in a").AsBoolean());
+
+        Assert.True(engine.Evaluate("0 in l").AsBoolean());
+        Assert.False(engine.Evaluate("3 in l").AsBoolean());
+        Assert.False(engine.Evaluate("-1 in l").AsBoolean());
+    }
+
+    [Fact]
+    public void KeyEnumerationYieldsIndexKeys()
+    {
+        var engine = new Engine();
+        engine.SetValue("a", new[] { 1, 2, 3 });
+        engine.SetValue("l", new List<string> { "x", "y" });
+
+        Assert.Equal("0,1,2", engine.Evaluate("Object.keys(a).join()").AsString());
+        Assert.Equal("0,1", engine.Evaluate("Object.keys(l).join()").AsString());
+        Assert.Equal("0,1,2", engine.Evaluate("var s=[];for(var k in a)s.push(k);s.join()").AsString());
+        Assert.Equal("1,2,3", engine.Evaluate("Object.values(a).join()").AsString());
+        Assert.Equal("""{"0":1,"1":2,"2":3}""", engine.Evaluate("JSON.stringify({...a})").AsString());
+
+        // members stay accessible even though they no longer enumerate
+        Assert.Equal(3, engine.Evaluate("a.length").AsNumber());
     }
 
     private static Engine CreateCopyEngine(Action<Options> additionalConfiguration = null)

Jint/Engine.cs

@@ -2269,6 +2269,11 @@ internal ref readonly ExecutionContext GetExecutionContext(int fromTop)
 
     public void Dispose()
     {
+        // the recent-wrapper ring (on by default since 4.14) strongly roots its targets and wrappers,
+        // so a disposed-but-still-referenced engine must release them
+        _recentObjectWrapperCache?.Clear();
+        _recentObjectWrapperCache = null;
+
         if (_objectWrapperCache is null)
         {
             return;

Jint/Native/Array/ArrayOperations.cs

@@ -640,7 +640,8 @@ public override bool TryGetValue(ulong index, out JsValue value)
 
         private JsValue ReadValue(int index)
         {
-            return (uint) index < _target.Length ? JsValue.FromObject(_target.Engine, _target.GetAt(index)) : JsValue.Undefined;
+            // GetJsValueAt converts common primitive element types without boxing
+            return (uint) index < _target.Length ? _target.GetJsValueAt(index) : JsValue.Undefined;
         }
 
         public override bool HasProperty(ulong index) => index < (ulong) _target.Length;

Jint/Options.cs

@@ -335,9 +335,10 @@ public class InteropOptions
         /// <summary>
         /// Whether identity map is persisted for object wrappers in order to maintain object identity. This can cause
         /// memory usage to grow when targeting large set and freeing of memory can be delayed due to ConditionalWeakTable semantics.
-        /// Also covers CLR arrays: repeated conversions of the same array instance return the same JsArray snapshot
-        /// (script-side mutations persist across reads; CLR-side mutations after the first conversion are not re-copied)
-        /// instead of copying every element on every read.
+        /// Also covers CLR arrays: repeated conversions of the same array instance return the same result — the live
+        /// wrapper view under <see cref="ArrayConversionMode.LiveView"/> (the default), or the same JsArray snapshot
+        /// under <see cref="ArrayConversionMode.Copy"/> (script-side mutations persist across reads; CLR-side mutations
+        /// after the first conversion are not re-copied) — instead of re-converting on every read.
         /// Defaults to false.
         /// </summary>
         public bool TrackObjectWrapperIdentity { get; set; }
@@ -757,11 +758,19 @@ public enum ArrayConversionMode
     /// <see cref="System.Collections.Generic.List{T}"/> instances already behave: element reads and writes go
     /// straight to the array in both directions, and wrapper identity across repeated crossings follows the same
     /// caches as other wrapped objects (<see cref="Options.InteropOptions.TrackObjectWrapperIdentity"/> /
-    /// <see cref="Options.InteropOptions.CacheRecentObjectWrappers"/>). The view is array-like — iteration,
-    /// <c>Array.prototype</c> methods and JSON serialization as an array all work — but it is not a JavaScript
-    /// array: <c>Array.isArray</c> returns <c>false</c>, and because CLR arrays are fixed-size any attempt to
-    /// resize the view (growing writes, <c>push</c>/<c>pop</c>, <c>length</c> writes) throws a <c>TypeError</c>.
+    /// <see cref="Options.InteropOptions.CacheRecentObjectWrappers"/>). An array crossing under a non-array
+    /// declared type keeps honoring that contract (for example a member typed <c>IReadOnlyList&lt;T&gt;</c>
+    /// produces a read-only view).
+    /// <para>
+    /// The view is array-like — iteration, <c>Array.prototype</c> methods, index-key enumeration
+    /// (<c>Object.keys</c> / <c>for-in</c>), out-of-range reads yielding <c>undefined</c> and JSON
+    /// serialization as an array all work — but it is not a JavaScript array: <c>Array.isArray</c> returns
+    /// <c>false</c> (while <c>instanceof Array</c> is <c>true</c> through the attached prototype), and because
+    /// CLR arrays are fixed-size the view behaves like an integer-indexed exotic object: resizing attempts
+    /// (growing writes, <c>push</c>/<c>pop</c>, <c>length</c> writes) throw a <c>TypeError</c>, and like for
+    /// typed arrays, <c>shift</c>/<c>splice</c> may move elements before their length change throws.
     /// Multi-dimensional arrays are not supported by the view and behave as under <see cref="Copy"/>.
+    /// </para>
     /// </summary>
     LiveView,
 }

Jint/Runtime/Interop/DefaultObjectConverter.cs

@@ -50,15 +50,34 @@ public static bool TryConvert(Engine engine, object value, Type? type, [NotNullW
         {
             if (value is Array a)
             {
-                // racy, we don't care, worst case we'll catch up later
-                Interlocked.CompareExchange(ref _typeMappers,
-                    new Dictionary<Type, Func<Engine, object, JsValue>>(typeMappers)
-                    {
-                        [valueType] = ConvertArray
-                    }, typeMappers);
+                if (valueType.IsArray)
+                {
+                    // memoization is only valid when the exposed type itself is an array type: every
+                    // future value crossing under it is an Array. A non-array exposed type (IEnumerable<T>,
+                    // IReadOnlyList<T>, ...) can later carry a List<T> or other non-array value, and
+                    // _typeMappers is a static cross-engine map — baking ConvertArray under such a type
+                    // would poison every engine in the process.
+                    // racy, we don't care, worst case we'll catch up later
+                    Interlocked.CompareExchange(ref _typeMappers,
+                        new Dictionary<Type, Func<Engine, object, JsValue>>(typeMappers)
+                        {
+                            [valueType] = ConvertArray
+                        }, typeMappers);
 
-                result = ConvertArray(engine, a);
-                return result is not null;
+                    result = ConvertArray(engine, a);
+                    return result is not null;
+                }
+
+                if (engine.Options.Interop.ArrayConversion == ArrayConversionMode.Copy)
+                {
+                    result = ConvertArray(engine, a);
+                    return result is not null;
+                }
+
+                // LiveView with a non-array exposed type: fall through to the wrapper lane below so the
+                // view honors the declared contract the same way other collections do — an array exposed
+                // as IReadOnlyList<T> must produce a read-only view, not a writable one keyed off the
+                // runtime type.
             }
 
             if (value is IConvertible convertible && TryConvertConvertible(engine, convertible, out result))
@@ -264,7 +283,7 @@ private static JsValue ConvertArray(Engine e, object v)
         }
 
         if (e.Options.Interop.ArrayConversion == ArrayConversionMode.LiveView
-            && TryConvertArrayLiveView(e, v, out var liveView))
+            && TryConvertArrayLiveView(e, v, arrayType, out var liveView))
         {
             return liveView;
         }
@@ -283,6 +302,9 @@ private static JsValue ConvertArray(Engine e, object v)
         if (e.Options.Interop.TrackObjectWrapperIdentity)
         {
             e._objectWrapperCache ??= new ConditionalWeakTable<object, ObjectInstance>();
+            // the table may hold a wrapper for a different exposed view of the same
+            // object (the type-guarded lookup above missed it) — last view wins
+            e._objectWrapperCache.Remove(v);
             e._objectWrapperCache.Add(v, result);
         }
         else if (e.Options.Interop.CacheRecentObjectWrappers)
@@ -301,9 +323,8 @@ private static JsValue ConvertArray(Engine e, object v)
     /// plus the flag-gated identity caches — already consulted by the caller). Multi-rank (T[,]) and
     /// non-zero-based (T[*]) arrays return false so they keep the Copy-mode failure behavior.
     /// </summary>
-    private static bool TryConvertArrayLiveView(Engine e, object v, [NotNullWhen(true)] out JsValue? result)
+    private static bool TryConvertArrayLiveView(Engine e, object v, Type arrayType, [NotNullWhen(true)] out JsValue? result)
     {
-        var arrayType = v.GetType();
         if (arrayType.GetElementType() is not { } elementType
             || arrayType != elementType.MakeArrayType())
         {
@@ -322,6 +343,9 @@ private static bool TryConvertArrayLiveView(Engine e, object v, [NotNullWhen(tru
         if (e.Options.Interop.TrackObjectWrapperIdentity)
         {
             e._objectWrapperCache ??= new ConditionalWeakTable<object, ObjectInstance>();
+            // the table may hold a wrapper for a different exposed view of the same
+            // object (the type-guarded lookup above missed it) — last view wins
+            e._objectWrapperCache.Remove(v);
             e._objectWrapperCache.Add(v, wrapped);
         }
         else if (e.Options.Interop.CacheRecentObjectWrappers)
@@ -377,4 +401,11 @@ public void Add(object target, ObjectInstance wrapper)
         _wrappers[index] = wrapper;
         _next = (index + 1) & (Capacity - 1);
     }
+
+    public void Clear()
+    {
+        Array.Clear(_targets, 0, _targets.Length);
+        Array.Clear(_wrappers, 0, _wrappers.Length);
+        _next = 0;
+    }
 }