inFlight ref-counting of strong GCHandle for JSObject, while in fligh…

…t over the managed/JS boundary (#54453)

src/libraries/Common/src/Interop/Browser/Interop.Runtime.cs

@@ -70,6 +70,7 @@ public static string InvokeJS(string str)
             object res = CompileFunction(snippet, out int exception);
             if (exception != 0)
                 throw new JSException((string)res);
+            ReleaseInFlight(res);
             return res as System.Runtime.InteropServices.JavaScript.Function;
         }
 
@@ -97,6 +98,7 @@ public static object GetGlobalObject(string? str = null)
             if (exception != 0)
                 throw new JSException($"Error obtaining a handle to global {str}");
 
+            ReleaseInFlight(globalHandle);
             return globalHandle;
         }
 
@@ -121,5 +123,11 @@ public static unsafe void DumpAotProfileData(ref byte buf, int len, string extra
                 module.SetObjectProperty("aot_profile_data", Uint8Array.From(span));
             }
         }
+
+        public static void ReleaseInFlight(object? obj)
+        {
+            JSObject? jsObj = obj as JSObject;
+            jsObj?.ReleaseInFlight();
+        }
     }
 }

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.RemoteServer.cs

@@ -178,7 +178,6 @@ public async Task GetAsync_ServerNeedsAuthAndNoCredential_StatusCodeUnauthorized
         [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))]
         [Theory]
         [MemberData(nameof(RemoteServersAndHeaderEchoUrisMemberData))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/53872", TestPlatforms.Browser)]
         public async Task GetAsync_RequestHeadersAddCustomHeaders_HeaderAndEmptyValueSent(Configuration.Http.RemoteServer remoteServer, Uri uri)
         {
             if (IsWinHttpHandler && !PlatformDetection.IsWindows10Version1709OrGreater)

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs

@@ -577,7 +577,6 @@ public async Task PostAsync_ManyDifferentRequestHeaders_SentCorrectly()
 
         [Theory]
         [MemberData(nameof(GetAsync_ManyDifferentResponseHeaders_ParsedCorrectly_MemberData))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/54655", TestPlatforms.Browser)]
         public async Task GetAsync_ManyDifferentResponseHeaders_ParsedCorrectly(string newline, string fold, bool dribble)
         {
             if (LoopbackServerFactory.Version >= HttpVersion20.Value)

src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs

@@ -217,7 +217,6 @@ public async Task SendAsync_MultipleOutstandingSendOperations_Throws(Uri server)
 
         [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))]
         [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/53957", TestPlatforms.Browser)]
         public async Task ReceiveAsync_MultipleOutstandingReceiveOperations_Throws(Uri server)
         {
             using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output))
@@ -321,7 +320,6 @@ public async Task SendAsync_SendZeroLengthPayloadAsEndOfMessage_Success(Uri serv
 
         [OuterLoop("Uses external servers", typeof(PlatformDetection), nameof(PlatformDetection.LocalEchoServerIsNotAvailable))]
         [ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/53957", TestPlatforms.Browser)]
         public async Task SendReceive_VaryingLengthBuffers_Success(Uri server)
         {
             using (ClientWebSocket cws = await WebSocketHelper.GetConnectedWebSocket(server, TimeOutMilliseconds, _output))

src/libraries/System.Private.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/AnyRef.cs

@@ -11,6 +11,8 @@ namespace System.Runtime.InteropServices.JavaScript
 {
     public abstract class AnyRef : SafeHandleMinusOneIsInvalid
     {
+        private GCHandle? InFlight;
+        private int InFlightCounter;
         private GCHandle AnyRefHandle;
         public int JSHandle => (int)handle;
 
@@ -21,9 +23,41 @@ internal AnyRef(IntPtr jsHandle, bool ownsHandle) : base(ownsHandle)
         {
             SetHandle(jsHandle);
             AnyRefHandle = GCHandle.Alloc(this, ownsHandle ? GCHandleType.Weak : GCHandleType.Normal);
+            InFlight = null;
+            InFlightCounter = 0;
         }
         internal int Int32Handle => (int)(IntPtr)AnyRefHandle;
 
+        internal void AddInFlight()
+        {
+            lock (this)
+            {
+                InFlightCounter++;
+                if (InFlightCounter == 1)
+                {
+                    Debug.Assert(InFlight == null);
+                    InFlight = GCHandle.Alloc(this, GCHandleType.Normal);
+                }
+            }
+        }
+
+        internal void ReleaseInFlight()
+        {
+            lock (this)
+            {
+                Debug.Assert(InFlightCounter != 0);
+
+                InFlightCounter--;
+                if (InFlightCounter == 0)
+                {
+                    Debug.Assert(InFlight.HasValue);
+                    InFlight.Value.Free();
+                    InFlight = null;
+                }
+            }
+        }
+
+
         protected void FreeGCHandle()
         {
             AnyRefHandle.Free();

src/libraries/System.Private.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Array.cs

@@ -86,6 +86,7 @@ internal Array(IntPtr jsHandle, bool ownsHandle) : base(jsHandle, ownsHandle)
 
                 if (exception != 0)
                     throw new JSException((string)indexValue);
+                Interop.Runtime.ReleaseInFlight(indexValue);
                 return indexValue;
             }
             set

src/libraries/System.Private.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSObject.cs

@@ -73,6 +73,7 @@ public object Invoke(string method, params object?[] args)
             object res = Interop.Runtime.InvokeJSWithArgs(JSHandle, method, args, out int exception);
             if (exception != 0)
                 throw new JSException((string)res);
+            Interop.Runtime.ReleaseInFlight(res);
             return res;
         }
 
@@ -103,6 +104,7 @@ public object GetObjectProperty(string name)
             object propertyValue = Interop.Runtime.GetObjectProperty(JSHandle, name, out int exception);
             if (exception != 0)
                 throw new JSException((string)propertyValue);
+            Interop.Runtime.ReleaseInFlight(propertyValue);
             return propertyValue;
         }
 

src/libraries/System.Private.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs

@@ -91,6 +91,8 @@ public static int BindJSObject(int jsId, bool ownsHandle, int mappedType)
                 }
             }
 
+            target.AddInFlight();
+
             return target.Int32Handle;
         }
 
@@ -236,12 +238,21 @@ public static int GetJSObjectId(object rawObj)
             return jsObject?.JSHandle ?? -1;
         }
 
-        public static object? GetDotNetObject(int gcHandle)
+        /// <param name="gcHandle"></param>
+        /// <param name="shouldAddInflight">when true, we would create Normal GCHandle to the JSObject, so that it would not get collected before passing it back to managed code</param>
+        public static object? GetDotNetObject(int gcHandle, int shouldAddInflight)
         {
             GCHandle h = (GCHandle)(IntPtr)gcHandle;
 
-            return h.Target is JSObject js ?
-                js.GetWrappedObject() ?? h.Target : h.Target;
+            if (h.Target is JSObject jso)
+            {
+                if (shouldAddInflight != 0)
+                {
+                    jso.AddInFlight();
+                }
+                return jso.GetWrappedObject() ?? jso;
+            }
+            return h.Target;
         }
 
         public static bool IsSimpleArray(object a)

src/libraries/System.Private.Runtime.InteropServices.JavaScript/tests/System/Runtime/InteropServices/JavaScript/JavaScriptTests.cs

@@ -1,7 +1,10 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Generic;
+using System.Linq;
 using System.Runtime.InteropServices.JavaScript;
+using System.Threading.Tasks;
 using Xunit;
 
 namespace System.Runtime.InteropServices.JavaScript.Tests
@@ -115,5 +118,110 @@ public static void FunctionMath()
             minValue = (int)mathMin.Call(null, 5, 6, 2, 3, 7);
             Assert.Equal(2, minValue);
         }
+
+        [Fact]
+        public static async Task BagIterator()
+        {
+            await Task.Delay(1);
+
+            var bagFn = new Function(@"
+                    var same = {
+                        x:1
+                    };
+                    return Object.entries({
+                        a:1,
+                        b:'two',
+                        c:{fold:{}},
+                        d:same,
+                        e:same,
+                        f:same
+                    });
+                ");
+
+            for (int attempt = 0; attempt < 100_000; attempt++)
+            {
+                try
+                {
+                    using var bag = (JSObject)bagFn.Call(null);
+                    using var entriesIterator = (JSObject)bag.Invoke("entries");
+
+                    var cnt = entriesIterator.ToEnumerable().Count();
+                    Assert.Equal(6, cnt);
+
+                    // fill GC helps to repro
+                    var x = new byte[100 + attempt / 100];
+                    if (attempt % 1000 == 0)
+                    {
+                        GC.Collect();
+                    }
+                }
+                catch (Exception ex)
+                {
+                    throw new Exception(ex.Message + " At attempt=" + attempt, ex);
+                }
+            }
+        }
+
+        [Fact]
+        public static async Task Iterator()
+        {
+            await Task.Delay(1);
+
+            var makeRangeIterator = new Function("start", "end", "step", @"
+                    let nextIndex = start;
+                    let iterationCount = 0;
+
+                    const rangeIterator = {
+                       next: function() {
+                           let result;
+                           if (nextIndex < end) {
+                               result = { value: {}, done: false }
+                               nextIndex += step;
+                               iterationCount++;
+                               return result;
+                           }
+                           return { value: {}, done: true }
+                       }
+                    };
+                    return rangeIterator;
+                ");
+
+            for (int attempt = 0; attempt < 100_000; attempt++)
+            {
+                try
+                {
+                    using (var entriesIterator = (JSObject)makeRangeIterator.Call(null, 0, 500))
+                    {
+                        var cnt = entriesIterator.ToEnumerable().Count();
+                    }
+                }
+                catch (Exception ex)
+                {
+                    throw new Exception(ex.Message + " At attempt=" + attempt, ex);
+                }
+            }
+        }
+
+        public static IEnumerable<object> ToEnumerable(this JSObject iterrator)
+        {
+            JSObject nextResult = null;
+            try
+            {
+                nextResult = (JSObject)iterrator.Invoke("next");
+                var done = (bool)nextResult.GetObjectProperty("done");
+                while (!done)
+                {
+                    object value = nextResult.GetObjectProperty("value");
+                    nextResult.Dispose();
+                    yield return value;
+                    nextResult = (JSObject)iterrator.Invoke("next");
+                    done = (bool)nextResult.GetObjectProperty("done");
+                }
+            }
+            finally
+            {
+                nextResult?.Dispose();
+            }
+        }
     }
 }

src/mono/wasm/runtime/binding_support.js

@@ -126,7 +126,7 @@ var BindingSupportLib = {
 			this._bind_existing_obj = bind_runtime_method ("BindExistingObject", "mi");
 			this._unbind_raw_obj_and_free = bind_runtime_method ("UnBindRawJSObjectAndFree", "ii");
 			this._get_js_id = bind_runtime_method ("GetJSObjectId", "m");
-			this._get_raw_mono_obj = bind_runtime_method ("GetDotNetObject", "i!");
+			this._get_raw_mono_obj = bind_runtime_method ("GetDotNetObject", "ii!");
 
 			this._is_simple_array = bind_runtime_method ("IsSimpleArray", "m");
 			this.setup_js_cont = get_method ("SetupJSContinuation");
@@ -779,15 +779,21 @@ var BindingSupportLib = {
 			return this._get_js_id (mono_obj);
 		},
 
-		wasm_get_raw_obj: function (gchandle)
+		// when should_add_in_flight === true, the JSObject would be temporarily hold by Normal GCHandle, so that it would not get collected during transition to the managed stack.
+		// its InFlight handle would be freed when the instance arrives to managed side via Interop.Runtime.ReleaseInFlight
+		wasm_get_raw_obj: function (gchandle, should_add_in_flight)
 		{
-			return this._get_raw_mono_obj (gchandle);
+			if(!gchandle){
+				return 0;
+			}
+
+			return this._get_raw_mono_obj (gchandle, should_add_in_flight ? 1 : 0);
 		},
 
 		try_extract_mono_obj:function (js_obj) {
 			if (js_obj === null || typeof js_obj === "undefined" || typeof js_obj.__mono_gchandle__ === "undefined")
 				return 0;
-			return this.wasm_get_raw_obj (js_obj.__mono_gchandle__);
+			return this.wasm_get_raw_obj (js_obj.__mono_gchandle__, true);
 		},
 
 		mono_method_get_call_signature: function(method, mono_obj) {
@@ -810,7 +816,7 @@ var BindingSupportLib = {
 			tcs.is_mono_tcs_task_bound = true;
 			js_obj.__mono_bound_tcs__ = tcs.__mono_gchandle__;
 			tcs.__mono_bound_task__ = js_obj.__mono_gchandle__;
-			return this.wasm_get_raw_obj (js_obj.__mono_gchandle__);
+			return this.wasm_get_raw_obj (js_obj.__mono_gchandle__, true);
 		},
 
 		free_task_completion_source: function (tcs) {
@@ -831,7 +837,7 @@ var BindingSupportLib = {
 			var result = null;
 			var gc_handle = js_obj.__mono_gchandle__;
 			if (gc_handle) {
-				result = this.wasm_get_raw_obj (gc_handle);
+				result = this.wasm_get_raw_obj (gc_handle, true);
 
 				// It's possible the managed object corresponding to this JS object was collected,
 				//  in which case we need to make a new one.
@@ -842,8 +848,8 @@ var BindingSupportLib = {
 			}
 
 			if (!result) {
-				gc_handle = this.mono_wasm_register_obj(js_obj);
-				result = this.wasm_get_raw_obj (gc_handle);
+				var { gc_handle: new_gc_handle, should_add_in_flight } = this.mono_wasm_register_obj(js_obj);
+				result = this.wasm_get_raw_obj (new_gc_handle, should_add_in_flight);
 			}
 
 			return result;
@@ -1613,10 +1619,12 @@ var BindingSupportLib = {
 					js_obj.__owns_handle__ = true;
 					gc_handle = js_obj.__mono_gchandle__ = this.wasm_binding_obj_new(handle + 1, js_obj.__owns_handle__, typeof wasm_type === "undefined" ? -1 : wasm_type);
 					this.mono_wasm_object_registry[handle] = js_obj;
-
+					// as this instance was just created, it was already created with Inflight strong GCHandle, so we do not have to do it again
+					return { gc_handle, should_add_in_flight: false };
 				}
 			}
-			return gc_handle;
+			// this is pre-existing instance, we need to add Inflight strong GCHandle before passing it to managed
+			return { gc_handle, should_add_in_flight: true };
 		},
 		mono_wasm_require_handle: function(handle) {
 			if (handle > 0)