[browser] release eagerly created Task/Promise proxies on non-normal async interop paths

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

@@ -170,6 +170,8 @@ public static void InvokeJS(JSFunctionBinding signature, Span<JSMarshalerArgumen
         /// <exception cref="PlatformNotSupportedException">The method is executed on an architecture other than WebAssembly.</exception>
         // JavaScriptExports need to be protected from trimming because they are used from C/JS code which IL linker can't see
         [DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.JavaScriptExports", "System.Runtime.InteropServices.JavaScript")]
+        // PromiseHolderCount has no callers here, it is read by the leak tests through UnsafeAccessor
+        [DynamicDependency("get_PromiseHolderCount", typeof(JSFunctionBinding))]
         public static JSFunctionBinding BindJSFunction(string functionName, string moduleName, ReadOnlySpan<JSMarshalerType> signatures)
         {
             if (RuntimeInformation.OSArchitecture != Architecture.Wasm)
@@ -281,6 +283,10 @@ internal static unsafe void DispatchJSFunctionSync(JSObject jsFunction, Span<JSM
         }
 #endif
 
+        // A pre-created holder has no JS-side proxy until JS adopts it, so a missed release is
+        // invisible to any JS-side census. Read from the leak tests with UnsafeAccessor.
+        private static int PromiseHolderCount => JSProxyContext.AssertIsInteropThread().PromiseHolderCount;
+
 #if !DEBUG
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
 #endif
@@ -297,10 +303,12 @@ internal static unsafe void InvokeJSImportImpl(JSFunctionBinding signature, Span
             var targetContext = JSProxyContext.MainThreadContext;
 #endif
 
+            JSHostImplementation.PromiseHolder? preCreatedHolder = null;
             if (signature.IsAsync)
             {
                 // pre-allocate the result handle and Task
                 var holder = targetContext.CreatePromiseHolder();
+                preCreatedHolder = holder;
                 res.slot.Type = MarshalerType.TaskPreCreated;
                 res.slot.GCHandle = holder.GCHandle;
 #if FEATURE_WASM_MANAGED_THREADS
@@ -323,39 +331,65 @@ internal static unsafe void InvokeJSImportImpl(JSFunctionBinding signature, Span
             }
 
 #if FEATURE_WASM_MANAGED_THREADS
-            // if we are on correct thread already or this is synchronous call, just call it
-            if (targetContext.IsCurrentThread())
+            try
             {
-                InvokeJSImportCurrent(signature, arguments);
+                // if we are on correct thread already or this is synchronous call, just call it
+                if (targetContext.IsCurrentThread())
+                {
+                    InvokeJSImportCurrent(signature, arguments);
 
+                    // if js synchronously returned null
+                    if (signature.IsAsync && arguments[1].slot.Type == MarshalerType.None)
+                    {
+                        targetContext.ReleasePromiseHolder(preCreatedHolder!.GCHandle);
+                        // cleared so the catch below does not release it a second time
+                        preCreatedHolder = null;
 #if DEBUG
-                if (signature.IsAsync && arguments[1].slot.Type == MarshalerType.None)
+                        throw new InvalidOperationException("null Task/Promise return is not supported");
+#endif
+                    }
+                }
+                else if (signature.IsAsync || signature.IsDiscardNoWait)
                 {
-                    throw new InvalidOperationException("null Task/Promise return is not supported");
+                    //async
+                    DispatchJSImportAsyncPost(signature, targetContext, arguments);
+                }
+                else
+                {
+                    //sync
+                    DispatchJSImportSyncSend(signature, targetContext, arguments);
                 }
-#endif
-
             }
-            else if (signature.IsAsync || signature.IsDiscardNoWait)
+            catch
             {
-                //async
-                DispatchJSImportAsyncPost(signature, targetContext, arguments);
+                // JS threw before it could take ownership of the pre-created holder
+                if (preCreatedHolder != null)
+                {
+                    targetContext.ReleasePromiseHolder(preCreatedHolder.GCHandle);
+                }
+                throw;
             }
-            else
+#else
+            try
             {
-                //sync
-                DispatchJSImportSyncSend(signature, targetContext, arguments);
+                InvokeJSImportCurrent(signature, arguments);
+            }
+            catch
+            {
+                // JS threw before it could take ownership of the pre-created holder
+                if (preCreatedHolder != null)
+                {
+                    targetContext.ReleasePromiseHolder(preCreatedHolder.GCHandle);
+                }
+                throw;
             }
-#else
-            InvokeJSImportCurrent(signature, arguments);
 
             if (signature.IsAsync)
             {
                 // if js synchronously returned null
                 if (arguments[1].slot.Type == MarshalerType.None)
                 {
-                    var holderHandle = (GCHandle)arguments[1].slot.GCHandle;
-                    holderHandle.Free();
+                    targetContext.ReleasePromiseHolder(preCreatedHolder!.GCHandle);
                 }
             }
 #endif

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

@@ -26,6 +26,19 @@ internal sealed class JSProxyContext : IDisposable
         internal Dictionary<int, Action<IntPtr>> JSExportByHandle = new Dictionary<int, Action<IntPtr>>();
         internal int NextJSExportHandle = 1;
 
+        public int PromiseHolderCount
+        {
+            get
+            {
+#if FEATURE_WASM_MANAGED_THREADS
+                lock (this)
+#endif
+                {
+                    return ThreadJsOwnedHolders.Count;
+                }
+            }
+        }
+
 #if !FEATURE_WASM_MANAGED_THREADS
         private JSProxyContext()
         {
@@ -340,7 +353,9 @@ public PromiseHolder CreatePromiseHolder()
             lock (this)
 #endif
             {
-                return new PromiseHolder(this);
+                var holder = new PromiseHolder(this);
+                ThreadJsOwnedHolders.Add(holder.GCHandle, holder);
+                return holder;
             }
         }
 
@@ -394,6 +409,7 @@ public void ReleasePromiseHolder(nint holderGCHandle)
                     {
                         throw new InvalidOperationException("ReleasePromiseHolder expected PromiseHolder" + holderGCHandle);
                     }
+                    ThreadJsOwnedHolders.Remove(holderGCHandle);
                     holder.IsDisposed = true;
                     handle.Free();
                 }
@@ -429,6 +445,7 @@ public unsafe void ReleaseJSOwnedObjectByGCHandle(nint gcHandle)
                     if (target is PromiseHolder holder2)
                     {
                         holder = holder2;
+                        ThreadJsOwnedHolders.Remove(gcHandle);
                     }
                     else
                     {
@@ -561,17 +578,35 @@ private void Dispose(bool disposing)
                         GCHandle gcHandle = (GCHandle)gch;
                         gcHandle.Free();
                     }
-                    foreach (var holder in ThreadJsOwnedHolders.Values)
+                    // the callback can re-enter and release a holder, which would mutate the
+                    // dictionary, so walk a snapshot and skip whatever it already took
+                    List<PromiseHolder> holders = new(ThreadJsOwnedHolders.Values);
+                    foreach (var holder in holders)
                     {
+                        if (holder.IsDisposed)
+                        {
+                            continue;
+                        }
+                        holder.IsDisposed = true;
                         unsafe
                         {
-                            holder.Callback!.Invoke(null);
+                            // a pre-created holder has no callback until JS adopts it
+                            holder.Callback?.Invoke(null);
+#if FEATURE_WASM_MANAGED_THREADS
+                            NativeMemory.Free(holder.State);
+                            holder.State = null;
+#endif
+                        }
+                        // a GCVHandle is a synthetic index, not a real GCHandle, so it must not be freed
+                        if (!IsGCVHandle(holder.GCHandle))
+                        {
+                            ((GCHandle)holder.GCHandle).Free();
                         }
-                        ((GCHandle)holder.GCHandle).Free();
                     }
 
                     ThreadCsOwnedObjects.Clear();
                     ThreadJsOwnedObjects.Clear();
+                    ThreadJsOwnedHolders.Clear();
                     JSVHandleFreeList.Clear();
                     NextJSVHandle = IntPtr.Zero;
 

src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj

@@ -37,6 +37,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\JavaScript\JSExportTest.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\JavaScript\YieldAwaitableTests.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\JavaScript\JSInteropTestBase.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\JavaScript\ProxyLeakTest.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\JavaScript\Utils.cs" />
 
     <None Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\JavaScript\JavaScriptTestHelper.mjs" />

src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.cs

@@ -40,6 +40,9 @@ public static void ConsoleWriteLine([JSMarshalAs<JSType.String>] string message)
         [JSImport("reject", "JavaScriptTestHelper")]
         public static partial Task Reject([JSMarshalAs<JSType.Any>] object what);
 
+        [JSImport("throwBeforePromise", "JavaScriptTestHelper")]
+        internal static partial Task ThrowBeforePromise();
+
         [JSImport("intentionallyMissingImport", "JavaScriptTestHelper")]
         public static partial void IntentionallyMissingImport();
 
@@ -575,6 +578,82 @@ internal static Task ReturnCompletedTask()
             return Task.CompletedTask;
         }
 
+        [JSExport]
+        internal static Task<int> ReturnCompletedTaskOfInt()
+        {
+            return Task.FromResult(42);
+        }
+
+        [JSExport]
+        internal static Task ReturnFaultedTask()
+        {
+            return Task.FromException(new ArgumentException("ReturnFaultedTask"));
+        }
+
+        // throws during the invocation itself, so JS never gets the Task it eagerly created for it
+        [JSExport]
+        internal static Task ThrowBeforeTask()
+        {
+            throw new ArgumentException("ThrowBeforeTask");
+        }
+
+        [JSExport]
+        internal static void ReturnVoidSynchronously()
+        {
+        }
+
+        [JSExport]
+        internal static async Task ReturnGenuinelyAsyncTask()
+        {
+            await Task.Yield();
+        }
+
+        [JSExport]
+        internal static async Task<int> ReturnDelayedTaskOfInt()
+        {
+            await Task.Delay(1);
+            return 42;
+        }
+
+        [JSExport]
+        internal static async Task ReturnDelayedFaultedTask()
+        {
+            await Task.Delay(1);
+            throw new ArgumentException(nameof(ReturnDelayedFaultedTask));
+        }
+
+        private static readonly List<TaskCompletionSource<int>> s_pendingExports = new();
+
+        // hands JS a distinct Task that stays pending until CompletePendingExports settles them all
+        [JSExport]
+        internal static Task<int> ReturnPendingTaskOfInt()
+        {
+            var tcs = new TaskCompletionSource<int>();
+            s_pendingExports.Add(tcs);
+            return tcs.Task;
+        }
+
+        internal static void CompletePendingExports()
+        {
+            foreach (var tcs in s_pendingExports)
+            {
+                tcs.TrySetResult(42);
+            }
+            s_pendingExports.Clear();
+        }
+
+        [JSExport]
+        internal static async Task AwaitPromiseParameter([JSMarshalAs<JSType.Promise<JSType.Number>>] Task<int> arg1)
+        {
+            await arg1;
+        }
+
+        // the managed side abandons the Task without ever observing it
+        [JSExport]
+        internal static void IgnorePromiseParameter([JSMarshalAs<JSType.Promise<JSType.Number>>] Task<int> arg1)
+        {
+        }
+
         [JSExport]
         [return: JSMarshalAs<JSType.Promise<JSType.Any>>]
         public static async Task<object> AwaitTaskOfObject([JSMarshalAs<JSType.Promise<JSType.Any>>] Task<object> arg1)
@@ -1226,6 +1305,26 @@ public static JSObject EchoIJSObject([JSMarshalAs<JSType.Object>] JSObject arg1)
         [JSImport("INTERNAL.forceDisposeProxies")]
         internal static partial void ForceDisposeProxies(bool disposeMethods, bool verbose);
 
+        // [csOwnedByJsHandle, csOwnedByJsvHandle, jsOwnedRegistered, jsOwnedAlive, importWrappers]
+        [JSImport("INTERNAL.getProxyCounts")]
+        internal static partial int[] GetProxyCounts();
+
+        [JSImport("forceJsGc", "JavaScriptTestHelper")]
+        internal static partial void ForceJsGc();
+
+        // mode is "await", "catch" or "drop"
+        [JSImport("invokeExportAsyncNTimes", "JavaScriptTestHelper")]
+        internal static partial Task InvokeExportAsyncNTimes(string exportName, int count, string mode);
+
+        [JSImport("invokeExportWithPromiseNTimes", "JavaScriptTestHelper")]
+        internal static partial Task InvokeExportWithPromiseNTimes(string exportName, int count, bool settled);
+
+        [JSImport("dropArg", "JavaScriptTestHelper")]
+        internal static partial void DropTask([JSMarshalAs<JSType.Promise<JSType.Void>>] Task arg1);
+
+        [JSImport("tryGetAssemblyExports", "JavaScriptTestHelper")]
+        internal static partial Task<string> TryGetAssemblyExports(string assemblyName);
+
         static JSObject _module;
         public static async Task InitializeAsync()
         {