diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs index 9375b83b013401..9f4e4a6cfcafb2 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs @@ -170,6 +170,8 @@ public static void InvokeJS(JSFunctionBinding signature, SpanThe method is executed on an architecture other than WebAssembly. // 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 signatures) { if (RuntimeInformation.OSArchitecture != Architecture.Wasm) @@ -281,6 +283,10 @@ internal static unsafe void DispatchJSFunctionSync(JSObject jsFunction, Span 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 diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSProxyContext.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSProxyContext.cs index 07c613ee937ce1..13a090332f2d60 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSProxyContext.cs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSProxyContext.cs @@ -26,6 +26,19 @@ internal sealed class JSProxyContext : IDisposable internal Dictionary> JSExportByHandle = new Dictionary>(); 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 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; diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj index f8338254bfdda4..29d1354b1f857b 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj @@ -37,6 +37,7 @@ + diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.cs index 531e56179ea5f5..a6c00491858baf 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.cs +++ b/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] string message) [JSImport("reject", "JavaScriptTestHelper")] public static partial Task Reject([JSMarshalAs] 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 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 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> s_pendingExports = new(); + + // hands JS a distinct Task that stays pending until CompletePendingExports settles them all + [JSExport] + internal static Task ReturnPendingTaskOfInt() + { + var tcs = new TaskCompletionSource(); + 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>] Task arg1) + { + await arg1; + } + + // the managed side abandons the Task without ever observing it + [JSExport] + internal static void IgnorePromiseParameter([JSMarshalAs>] Task arg1) + { + } + [JSExport] [return: JSMarshalAs>] public static async Task AwaitTaskOfObject([JSMarshalAs>] Task arg1) @@ -1226,6 +1305,26 @@ public static JSObject EchoIJSObject([JSMarshalAs] 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>] Task arg1); + + [JSImport("tryGetAssemblyExports", "JavaScriptTestHelper")] + internal static partial Task TryGetAssemblyExports(string assemblyName); + static JSObject _module; public static async Task InitializeAsync() { diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.mjs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.mjs index 1c91dbdb1dfe6b..c331cd68ce3162 100644 --- a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.mjs +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.mjs @@ -291,6 +291,71 @@ export async function invokeReturnCompletedTask() { return "resolved"; } +function resolveExport(exportName) { + const fn = dllExports.System.Runtime.InteropServices.JavaScript.Tests.JavaScriptTestHelper[exportName]; + if (typeof fn !== "function") throw new Error(`No such export ${exportName}`); + return fn; +} + +// calls a [JSExport] returning a Task. "drop" is the fire-and-forget shape reported in +// dotnet/runtime#132966, "catch" swallows the rejection without keeping the promise, +// "await" observes it, "throws" expects the call itself to throw instead of returning a Task. +export async function invokeExportAsyncNTimes(exportName, count, mode) { + const fn = resolveExport(exportName); + const observed = []; + for (let i = 0; i < count; i++) { + let res; + try { + res = fn(); + } catch (ex) { + if (mode !== "throws") throw ex; + // there is no promise to observe, the eagerly created one had to be released + continue; + } + const thenable = res && typeof res.then === "function"; + if (mode === "await" && thenable) { + observed.push(res.then(() => { }, () => { })); + } else if (mode === "catch" && thenable) { + res.then(() => { }, () => { }); + } + } + await Promise.all(observed); +} + +// passes a JS promise into a [JSExport] whose parameter is a Task +export async function invokeExportWithPromiseNTimes(exportName, count, settled) { + const fn = resolveExport(exportName); + const observed = []; + for (let i = 0; i < count; i++) { + const arg = settled ? Promise.resolve(42) : delay(1).then(() => 42); + const res = fn(arg); + if (res && typeof res.then === "function") { + observed.push(res.then(() => { }, () => { })); + } + } + await Promise.all(observed); +} + +// counterpart of thenvoid: JS never observes the promise it was handed +export function dropArg(arg1) { +} + +export async function tryGetAssemblyExports(assemblyName) { + try { + await App.runtime.getAssemblyExports(assemblyName); + return "resolved"; + } catch (ex) { + return "" + ex; + } +} + +// requires --expose-gc, which this test project passes via WasmXHarnessArgs +export function forceJsGc() { + if (typeof globalThis.gc === "function") { + globalThis.gc(); + } +} + export function invokeFuncWithOffset(fn, arg, offset) { return fn(arg + offset); } @@ -507,6 +572,11 @@ export function reject(what) { return new Promise((_, reject) => globalThis.setTimeout(() => reject(what), 0)); } +// throws instead of returning a Promise, so the pre-created Task is never adopted +export function throwBeforePromise() { + throw new Error("intentionally thrown before returning a promise"); +} + let setTimeoutHit = false; let promiseThenHit = false; export function beforeYield() { diff --git a/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/ProxyLeakTest.cs b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/ProxyLeakTest.cs new file mode 100644 index 00000000000000..9f76b9f623c5a4 --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/ProxyLeakTest.cs @@ -0,0 +1,243 @@ +// 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.Runtime.CompilerServices; +using System.Threading.Tasks; +using Xunit; + +namespace System.Runtime.InteropServices.JavaScript.Tests +{ + // Every async marshaling path eagerly allocates one half of a Task/Promise pair before it knows + // whether the other half will ever arrive. These tests pin the JSHandle tables to their baseline + // across all four crossings, for both completion states and for observed as well as abandoned + // results, so that a missed release on any non-normal path shows up as a growing table. + // + // Chromium only: draining a proxy requires forcing a JS collection, and globalThis.gc is exposed + // by the --expose-gc engine argument this project passes for Chrome. Elsewhere the proxies are + // released on the engine's own schedule and the counts would not settle within a test. + // + // Single-threaded only: with managed threads the census also counts proxies held by other + // threads, which drain independently of this test, and getAssemblyExports never settles. + [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsChromium), nameof(PlatformDetection.IsNotMultithreadingSupported))] + public class ProxyLeakTest : JSInteropTestBase, IAsyncLifetime + { + private const int Iterations = 100; + + // Drains proxies whose peer is already unreachable on either side, so that only genuinely + // rooted proxies remain counted. WaitForPendingFinalizers is a no-op on single-threaded wasm, + // so finalizers are driven by yielding between collections. + private static async Task Quiesce() + { + for (int i = 0; i < 3; i++) + { + await Task.Yield(); + await JavaScriptTestHelper.Delay(1); + JavaScriptTestHelper.ForceJsGc(); + GC.Collect(); + await Task.Yield(); + GC.Collect(); + } + } + + // run is invoked with the number of round trips to perform. + private static async Task AssertNoLeak(Func run) + { + // warm up the bindings so that their one-time allocations are not counted + await run(1); + await Quiesce(); + + int[] before = JavaScriptTestHelper.GetProxyCounts(); + await run(Iterations); + await Quiesce(); + int[] after = JavaScriptTestHelper.GetProxyCounts(); + + // Only the JSHandle tables are asserted on. They are maintained by explicit release + // calls, which is precisely where a missed release shows up, and they move only in + // response to this test. The GCHandle table behind them is drained by the JS + // FinalizationRegistry a few entries per turn, so it lags by an unbounded amount and + // would make these assertions fragile rather than stricter. + // The contract is that a round trip must not add a proxy, so this asserts on growth + // rather than equality: an unrelated proxy draining mid-test lowers a count without + // saying anything about the path under test, while a missed release adds Iterations. + string census = "[csOwnedByJsHandle, csOwnedByJsvHandle, jsOwnedRegistered, jsOwnedAlive, importWrappers]" + + $"{Environment.NewLine}before: {string.Join(", ", before)}" + + $"{Environment.NewLine}after: {string.Join(", ", after)}"; + Assert.True(after[0] <= before[0] && after[1] <= before[1], census); + } + + // managed Task -> JS Promise, as the return value of a [JSExport] + // https://github.com/dotnet/runtime/issues/132966 + [Theory] + [InlineData(nameof(JavaScriptTestHelper.ReturnCompletedTask), "drop")] + [InlineData(nameof(JavaScriptTestHelper.ReturnCompletedTask), "await")] + [InlineData(nameof(JavaScriptTestHelper.ReturnCompletedTaskOfInt), "drop")] + [InlineData(nameof(JavaScriptTestHelper.ReturnCompletedTaskOfInt), "await")] + [InlineData(nameof(JavaScriptTestHelper.ReturnFaultedTask), "catch")] + [InlineData(nameof(JavaScriptTestHelper.ThrowBeforeTask), "throws")] + [InlineData(nameof(JavaScriptTestHelper.ReturnGenuinelyAsyncTask), "drop")] + [InlineData(nameof(JavaScriptTestHelper.ReturnGenuinelyAsyncTask), "await")] + [InlineData(nameof(JavaScriptTestHelper.ReturnDelayedTaskOfInt), "drop")] + [InlineData(nameof(JavaScriptTestHelper.ReturnDelayedTaskOfInt), "await")] + [InlineData(nameof(JavaScriptTestHelper.ReturnDelayedFaultedTask), "catch")] + [InlineData(nameof(JavaScriptTestHelper.ReturnVoidSynchronously), "drop")] + public Task JSExportReturningTask_DoesNotLeakProxies(string exportName, string mode) + => AssertNoLeak(count => JavaScriptTestHelper.InvokeExportAsyncNTimes(exportName, count, mode)); + + // a promise JS abandons while it is still pending must be released once the Task completes + [Fact] + public Task JSExportReturningPendingTask_ReleasesProxiesOnCompletion() + => AssertNoLeak(async count => + { + await JavaScriptTestHelper.InvokeExportAsyncNTimes(nameof(JavaScriptTestHelper.ReturnPendingTaskOfInt), count, "drop"); + JavaScriptTestHelper.CompletePendingExports(); + }); + + // JS Promise -> managed Task, as the return value of a [JSImport] + [Theory] + [InlineData("resolved", true)] + [InlineData("resolved", false)] + [InlineData("delayed", true)] + [InlineData("delayed", false)] + [InlineData("rejected", true)] + [InlineData("rejected", false)] + public Task JSImportReturningPromise_DoesNotLeakProxies(string kind, bool observed) + => AssertNoLeak(async count => + { + var started = new List(count); + for (int i = 0; i < count; i++) + { + Task task = kind switch + { + "resolved" => JavaScriptTestHelper.ReturnResolvedPromise(), + "delayed" => JavaScriptTestHelper.sleep(1), + _ => JavaScriptTestHelper.Reject("intentionally orphaned"), + }; + + if (observed) + { + started.Add(task); + } + } + + foreach (Task task in started) + { + try + { + await task; + } + catch (JSException) + { + } + } + + // give the abandoned ones a chance to settle before the census is taken + await JavaScriptTestHelper.Delay(10); + }); + + // managed Task -> JS Promise, as an argument of a [JSImport] + [Theory] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public Task JSImportWithTaskArgument_DoesNotLeakProxies(bool completed, bool observedByJs) + => AssertNoLeak(async count => + { + var pending = new List(count); + for (int i = 0; i < count; i++) + { + // a fresh source every time, so that each iteration marshals a distinct Task + var tcs = new TaskCompletionSource(); + if (completed) + { + tcs.SetResult(); + } + else + { + pending.Add(tcs); + } + + if (observedByJs) + { + JavaScriptTestHelper.thenvoid(tcs.Task); + } + else + { + JavaScriptTestHelper.DropTask(tcs.Task); + } + } + + foreach (TaskCompletionSource tcs in pending) + { + tcs.SetResult(); + } + + await JavaScriptTestHelper.Delay(10); + }); + + // JS Promise -> managed Task, as an argument of a [JSExport] + [Theory] + [InlineData(nameof(JavaScriptTestHelper.AwaitPromiseParameter), true)] + [InlineData(nameof(JavaScriptTestHelper.AwaitPromiseParameter), false)] + [InlineData(nameof(JavaScriptTestHelper.IgnorePromiseParameter), true)] + [InlineData(nameof(JavaScriptTestHelper.IgnorePromiseParameter), false)] + public Task JSExportWithPromiseArgument_DoesNotLeakProxies(string exportName, bool settled) + => AssertNoLeak(count => JavaScriptTestHelper.InvokeExportWithPromiseNTimes(exportName, count, settled)); + + // CoreCLR only: its BindAssemblyExports marshals the failure back as a managed exception, + // while on Mono a missing assembly trips a native assert that aborts the runtime, leaving + // managed code nothing to catch. + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMonoRuntime))] + public Task FailingGetAssemblyExports_DoesNotLeakProxies() + => AssertNoLeak(async count => + { + for (int i = 0; i < count; i++) + { + string result = await JavaScriptTestHelper.TryGetAssemblyExports("System.Runtime.InteropServices.JavaScript.Tests.NoSuchAssembly"); + Assert.DoesNotContain("resolved", result); + } + }); + } + + // Separate from ProxyLeakTest because it counts managed PromiseHolders rather than JS proxies. + // That table is per-context and released explicitly, so it needs neither a forced collection + // nor a single-threaded runtime to settle. + public class PromiseHolderLeakTest : JSInteropTestBase, IAsyncLifetime + { + private const int Iterations = 100; + + [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "get_PromiseHolderCount")] + private static extern int GetPromiseHolderCount(JSFunctionBinding binding); + + private static async Task ThrowNTimes(int count) + { + for (int i = 0; i < count; i++) + { + try + { + // the JS side throws instead of returning a Promise, so the eagerly created + // holder is never handed over + await JavaScriptTestHelper.ThrowBeforePromise(); + Assert.Fail("expected the JS side to throw"); + } + catch (JSException) + { + } + } + } + + [Fact] + public async Task ThrowingAsyncImport_DoesNotLeakHolders() + { + // warm up the binding so its one-time allocations are not counted + await ThrowNTimes(1); + + int before = GetPromiseHolderCount(null); + await ThrowNTimes(Iterations); + int after = GetPromiseHolderCount(null); + + Assert.True(after <= before, $"promise holders before: {before}, after: {after}"); + } + } +} diff --git a/src/mono/browser/runtime/exports-internal.ts b/src/mono/browser/runtime/exports-internal.ts index 174bf360de612e..5a69b2802a3cc9 100644 --- a/src/mono/browser/runtime/exports-internal.ts +++ b/src/mono/browser/runtime/exports-internal.ts @@ -18,7 +18,7 @@ import { getOptions, applyOptions } from "./jiterpreter-support"; import { mono_wasm_gc_lock, mono_wasm_gc_unlock } from "./gc-lock"; import { loadLazyAssembly } from "./lazyLoading"; import { loadSatelliteAssemblies } from "./satelliteAssemblies"; -import { forceDisposeProxies } from "./gc-handles"; +import { forceDisposeProxies, get_proxy_counts } from "./gc-handles"; import { mono_wasm_get_func_id_to_name_mappings } from "./logging"; import { monoStringToStringUnsafe } from "./strings"; import { mono_wasm_bind_cs_function } from "./invoke-cs"; @@ -32,6 +32,7 @@ export function export_internal (): any { Module.err("early exit " + exit_code); }, forceDisposeProxies, + getProxyCounts: get_proxy_counts, mono_wasm_dump_threads: WasmEnableThreads ? mono_wasm_dump_threads : undefined, // with mono_wasm_debugger_log and mono_wasm_trace_logger diff --git a/src/mono/browser/runtime/gc-handles.ts b/src/mono/browser/runtime/gc-handles.ts index cd8bebeb4877b9..6ddd0a0deadcab 100644 --- a/src/mono/browser/runtime/gc-handles.ts +++ b/src/mono/browser/runtime/gc-handles.ts @@ -60,6 +60,8 @@ if (_use_finalization_registry) { export const js_owned_gc_handle_symbol = Symbol.for("wasm js_owned_gc_handle"); export const cs_owned_js_handle_symbol = Symbol.for("wasm cs_owned_js_handle"); export const do_not_force_dispose = Symbol.for("wasm do_not_force_dispose"); +// links an eagerly created Promise back to the JSHandle of its TaskHolder +export const eager_task_handle_symbol = Symbol.for("wasm eager_task_handle"); export function mono_wasm_get_jsobj_from_js_handle (js_handle: JSHandle): any { @@ -185,6 +187,32 @@ function _js_owned_object_finalized (gc_handle: GCHandle): void { teardown_managed_proxy(null, gc_handle); } +// Counts of live proxies, for leak diagnostics and tests. Exposed as INTERNAL.getProxyCounts. +// Order: [csOwnedByJsHandle, csOwnedByJsvHandle, jsOwnedRegistered, jsOwnedAlive, importWrappers] +export function get_proxy_counts (): number[] { + // index 0 of each list is always a dummy + const count_live = (list: any[]): number => { + let live = 0; + for (let i = 1; i < list.length; i++) { + if (list[i] !== undefined && list[i] !== null) live++; + } + return live; + }; + + let js_owned_alive = 0; + for (const wr of _js_owned_object_table.values()) { + if (wr.deref() !== undefined) js_owned_alive++; + } + + return [ + count_live(_cs_owned_objects_by_js_handle), + count_live(_cs_owned_objects_by_jsv_handle), + _js_owned_object_table.size, + js_owned_alive, + count_live(js_import_wrapper_by_fn_handle), + ]; +} + export function _lookup_js_owned_object (gc_handle: GCHandle): any { if (!gc_handle) return null; diff --git a/src/mono/browser/runtime/invoke-cs.ts b/src/mono/browser/runtime/invoke-cs.ts index 3e78c48301774a..83a28a73f8b472 100644 --- a/src/mono/browser/runtime/invoke-cs.ts +++ b/src/mono/browser/runtime/invoke-cs.ts @@ -6,7 +6,7 @@ import WasmEnableThreads from "consts:wasmEnableThreads"; import { Module, loaderHelpers, mono_assert, runtimeHelpers } from "./globals"; import { bind_arg_marshal_to_cs } from "./marshal-to-cs"; -import { bind_arg_marshal_to_js, end_marshal_task_to_js } from "./marshal-to-js"; +import { bind_arg_marshal_to_js, end_marshal_task_to_js, release_eager_task_holder } from "./marshal-to-js"; import { get_sig, get_signature_argument_count, bound_cs_function_symbol, get_signature_version, alloc_stack_frame, get_signature_type, @@ -203,8 +203,14 @@ function bind_fn_1RA (closure: BindingClosure) { // pre-allocate the promise let promise = res_converter(args); - // call C# side - invoke_async_jsexport(runtimeHelpers.managedThreadTID, method, args, size); + try { + // call C# side + invoke_async_jsexport(runtimeHelpers.managedThreadTID, method, args, size); + } catch (ex) { + // the throw unwinds past end_marshal_task_to_js, which would otherwise adopt it + release_eager_task_holder(promise); + throw ex; + } // in case the C# side returned synchronously promise = end_marshal_task_to_js(args, undefined, promise); @@ -270,8 +276,14 @@ function bind_fn_2RA (closure: BindingClosure) { // pre-allocate the promise let promise = res_converter(args); - // call C# side - invoke_async_jsexport(runtimeHelpers.managedThreadTID, method, args, size); + try { + // call C# side + invoke_async_jsexport(runtimeHelpers.managedThreadTID, method, args, size); + } catch (ex) { + // the throw unwinds past end_marshal_task_to_js, which would otherwise adopt it + release_eager_task_holder(promise); + throw ex; + } // in case the C# side returned synchronously promise = end_marshal_task_to_js(args, undefined, promise); @@ -317,7 +329,13 @@ function bind_fn (closure: BindingClosure) { // call C# side if (is_async) { - invoke_async_jsexport(runtimeHelpers.managedThreadTID, method, args, size); + try { + invoke_async_jsexport(runtimeHelpers.managedThreadTID, method, args, size); + } catch (ex) { + // the throw unwinds past end_marshal_task_to_js, which would otherwise adopt it + release_eager_task_holder(js_result); + throw ex; + } // in case the C# side returned synchronously js_result = end_marshal_task_to_js(args, undefined, js_result); } else if (is_discard_no_wait) { diff --git a/src/mono/browser/runtime/invoke-js.ts b/src/mono/browser/runtime/invoke-js.ts index 63e6731f46b909..bcc08dc0370f7a 100644 --- a/src/mono/browser/runtime/invoke-js.ts +++ b/src/mono/browser/runtime/invoke-js.ts @@ -5,7 +5,7 @@ import WasmEnableThreads from "consts:wasmEnableThreads"; import BuildConfiguration from "consts:configuration"; import { marshal_exception_to_cs, bind_arg_marshal_to_cs, marshal_task_to_cs } from "./marshal-to-cs"; -import { get_signature_argument_count, bound_js_function_symbol, get_sig, get_signature_version, get_signature_type, imported_js_function_symbol, get_signature_handle, get_signature_function_name, get_signature_module_name, is_receiver_should_free, get_caller_native_tid, get_sync_done_semaphore_ptr, get_arg } from "./marshal"; +import { get_signature_argument_count, bound_js_function_symbol, get_sig, get_signature_version, get_signature_type, imported_js_function_symbol, get_signature_handle, get_signature_function_name, get_signature_module_name, is_receiver_should_free, get_caller_native_tid, get_sync_done_semaphore_ptr, get_arg, get_arg_type } from "./marshal"; import { fixupPointer, forceThreadMemoryViewRefresh, free } from "./memory"; import { JSFunctionSignature, JSMarshalerArguments, BoundMarshalerToJs, JSFnHandle, BoundMarshalerToCs, JSHandle, MarshalerType, VoidPtrNull } from "./types/internal"; import { VoidPtr } from "./types/emscripten"; @@ -335,7 +335,14 @@ function bind_fn (closure: BindingClosure) { } } } catch (ex) { - marshal_exception_to_cs(args, ex); + // on the async post path the caller already has the pre-created Task and is gone, so it + // would never read the exception slot. Deliver the failure through the Task instead. + const res = receiver_should_free ? get_arg(args, 1) : null; + if (res && get_arg_type(res) === MarshalerType.TaskPreCreated) { + marshal_task_to_cs(res, Promise.reject(ex)); + } else { + marshal_exception_to_cs(args, ex); + } } finally { if (receiver_should_free) { free(args as any); diff --git a/src/mono/browser/runtime/managed-exports.ts b/src/mono/browser/runtime/managed-exports.ts index e1d2dc06cb5027..59e6e2e8e693d7 100644 --- a/src/mono/browser/runtime/managed-exports.ts +++ b/src/mono/browser/runtime/managed-exports.ts @@ -8,7 +8,7 @@ import cwraps, { threads_c_functions as twraps } from "./cwraps"; import { runtimeHelpers, Module, loaderHelpers, mono_assert } from "./globals"; import { JavaScriptMarshalerArgSize, alloc_stack_frame, get_arg, get_arg_gc_handle, is_args_exception, set_arg_i32, set_arg_intptr, set_arg_type, set_gc_handle, set_receiver_should_free } from "./marshal"; import { marshal_array_to_cs, marshal_array_to_cs_impl, marshal_bool_to_cs, marshal_exception_to_cs, marshal_intptr_to_cs, marshal_string_to_cs } from "./marshal-to-cs"; -import { marshal_int32_to_js, end_marshal_task_to_js, marshal_string_to_js, begin_marshal_task_to_js, marshal_exception_to_js } from "./marshal-to-js"; +import { marshal_int32_to_js, end_marshal_task_to_js, marshal_string_to_js, begin_marshal_task_to_js, marshal_exception_to_js, release_eager_task_holder } from "./marshal-to-js"; import { do_not_force_dispose, is_gcv_handle } from "./gc-handles"; import { assert_c_interop, assert_js_interop } from "./invoke-js"; import { monoThreadInfo, mono_wasm_main_thread_ptr } from "./pthreads"; @@ -62,7 +62,13 @@ export function call_entry_point (main_assembly_name: string, program_args: stri // because this is async, we could pre-allocate the promise let promise = begin_marshal_task_to_js(res, MarshalerType.TaskPreCreated, marshal_int32_to_js); - invoke_async_jsexport(runtimeHelpers.managedThreadTID, managedExports.CallEntrypoint, args, size); + try { + invoke_async_jsexport(runtimeHelpers.managedThreadTID, managedExports.CallEntrypoint, args, size); + } catch (ex) { + // the throw unwinds past end_marshal_task_to_js, which would otherwise adopt the promise + release_eager_task_holder(promise); + throw ex; + } // in case the C# side returned synchronously promise = end_marshal_task_to_js(args, marshal_int32_to_js, promise); @@ -343,7 +349,13 @@ export function bind_assembly_exports (assemblyName: string): Promise { // because this is async, we could pre-allocate the promise let promise = begin_marshal_task_to_js(res, MarshalerType.TaskPreCreated); - invoke_async_jsexport(runtimeHelpers.managedThreadTID, managedExports.BindAssemblyExports, args, size); + try { + invoke_async_jsexport(runtimeHelpers.managedThreadTID, managedExports.BindAssemblyExports, args, size); + } catch (ex) { + // the throw unwinds past end_marshal_task_to_js, which would otherwise adopt the promise + release_eager_task_holder(promise); + throw ex; + } // in case the C# side returned synchronously promise = end_marshal_task_to_js(args, marshal_int32_to_js, promise); diff --git a/src/mono/browser/runtime/marshal-to-js.ts b/src/mono/browser/runtime/marshal-to-js.ts index 0036d4a0ef33cf..c7fb7976116e0b 100644 --- a/src/mono/browser/runtime/marshal-to-js.ts +++ b/src/mono/browser/runtime/marshal-to-js.ts @@ -6,7 +6,7 @@ import BuildConfiguration from "consts:configuration"; import WasmEnableJsInteropByValue from "consts:wasmEnableJsInteropByValue"; import cwraps from "./cwraps"; -import { _lookup_js_owned_object, mono_wasm_get_js_handle, mono_wasm_get_jsobj_from_js_handle, SystemInteropJS_ReleaseCSOwnedObject, register_with_jsv_handle, setup_managed_proxy, teardown_managed_proxy } from "./gc-handles"; +import { _lookup_js_owned_object, mono_wasm_get_js_handle, mono_wasm_get_jsobj_from_js_handle, SystemInteropJS_ReleaseCSOwnedObject, register_with_jsv_handle, setup_managed_proxy, teardown_managed_proxy, eager_task_handle_symbol } from "./gc-handles"; import { loaderHelpers, mono_assert } from "./globals"; import { ManagedObject, ManagedError, @@ -256,9 +256,21 @@ export function begin_marshal_task_to_js (arg: JSMarshalerArgument, _?: Marshale } set_js_handle(arg, js_handle); set_arg_type(arg, MarshalerType.TaskPreCreated); + // the caller only gets the promise back, so it needs a way to find the handle again. + // storing the number rather than the holder keeps the promise from retaining it. + (holder.promise as any)[eager_task_handle_symbol] = js_handle; return holder.promise; } +// the eagerly created Promise was never handed to managed code, drop its proxy +export function release_eager_task_holder (eagerPromise: Promise | null | undefined): void { + if (!eagerPromise) return; + const js_handle = (eagerPromise as any)[eager_task_handle_symbol]; + mono_assert(js_handle, "Expected JSHandle on the eagerly created promise"); + (eagerPromise as any)[eager_task_handle_symbol] = undefined; + SystemInteropJS_ReleaseCSOwnedObject(js_handle); +} + export function end_marshal_task_to_js (args: JSMarshalerArguments, res_converter: MarshalerToJs | undefined, eagerPromise: Promise | null) { // this path is used when Task is returned from JSExport/call_entry_point const res = get_arg(args, 1); @@ -270,8 +282,7 @@ export function end_marshal_task_to_js (args: JSMarshalerArguments, res_converte } // otherwise drop the eagerPromise's handle - const js_handle = mono_wasm_get_js_handle(eagerPromise); - SystemInteropJS_ReleaseCSOwnedObject(js_handle); + release_eager_task_holder(eagerPromise); // get the synchronous result const promise = try_marshal_sync_task_to_js(res, type, res_converter); diff --git a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/gc-handles.ts b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/gc-handles.ts index e33c23262a636a..2693770789297a 100644 --- a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/gc-handles.ts +++ b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/gc-handles.ts @@ -16,6 +16,8 @@ export const boundJsFunctionSymbol = Symbol.for("wasm bound_js_function"); export const importedJsFunctionSymbol = Symbol.for("wasm imported_js_function"); export const proxyDebugSymbol = Symbol.for("wasm proxyDebug"); export const promiseHolderSymbol = Symbol.for("wasm promise_holder"); +// links an eagerly created Promise back to the JSHandle of its TaskHolder +export const eagerTaskHandleSymbol = Symbol.for("wasm eager_task_handle"); let forceDisposeProxiesInProgress = false; @@ -194,6 +196,32 @@ function _jsOwnedObjectFinalized(gcHandle: GCHandle): void { teardownManagedProxy(null, gcHandle); } +// Counts of live proxies, for leak diagnostics and tests. Exposed as INTERNAL.getProxyCounts. +// Order: [csOwnedByJsHandle, csOwnedByJsvHandle, jsOwnedRegistered, jsOwnedAlive, importWrappers] +export function getProxyCounts(): number[] { + // index 0 of each list is always a dummy + const countLive = (list: any[]): number => { + let live = 0; + for (let i = 1; i < list.length; i++) { + if (list[i] !== undefined && list[i] !== null) live++; + } + return live; + }; + + let jsOwnedAlive = 0; + for (const wr of jsOwnedObjectTable.values()) { + if (wr.deref() !== undefined) jsOwnedAlive++; + } + + return [ + countLive(_CsOwnedObjectsByJsHandle), + countLive(_CsOwnedObjectsByJsvHandle), + jsOwnedObjectTable.size, + jsOwnedAlive, + countLive(jsImportWrapperByFnHandle), + ]; +} + export function lookupJsOwnedObject(gcHandle: GCHandle): any { if (!gcHandle) return null; diff --git a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/index.ts b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/index.ts index 8cc5a92ac823fa..2963e980fe3c41 100644 --- a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/index.ts +++ b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/index.ts @@ -14,7 +14,7 @@ import { import { bindCsFunction, getAssemblyExports } from "./invoke-cs"; import { initializeMarshalersToJs, resolveOrRejectPromise } from "./marshal-to-js"; import { initializeMarshalersToCs } from "./marshal-to-cs"; -import { forceDisposeProxies, releaseCSOwnedObject } from "./gc-handles"; +import { forceDisposeProxies, getProxyCounts, releaseCSOwnedObject } from "./gc-handles"; import { cancelPromise } from "./cancelable-promise"; import { loadLazyAssembly, loadSatelliteAssemblies } from "./lazy"; import { jsInteropState } from "./marshal"; @@ -52,6 +52,7 @@ export function dotnetInitializeModule(internals: InternalExchange): void { bindCsFunction, loadSatelliteAssemblies, loadLazyAssembly, + getProxyCounts, // WebSocket wsCreate, diff --git a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-cs.ts b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-cs.ts index ca7b7af49027fd..11bc9ddec26d72 100644 --- a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-cs.ts +++ b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-cs.ts @@ -10,7 +10,7 @@ import { dotnetAssert, dotnetLogger, Module } from "./cross-module"; import { bindAssemblyExports, invokeJSExport } from "./managed-exports"; import { allocStackFrame, getSig, getSignatureType, getSignatureArgumentCount, getSignatureVersion, jsInteropState } from "./marshal"; import { bindArgMarshalToCs } from "./marshal-to-cs"; -import { bindArgMarshalToJs, endMarshalTaskToJs } from "./marshal-to-js"; +import { bindArgMarshalToJs, endMarshalTaskToJs, releaseEagerTaskHolder } from "./marshal-to-js"; import { assertJsInterop, assertRuntimeRunning, endMeasure, isRuntimeRunning, startMeasure } from "./utils"; import { MarshalerType, MeasuredBlock } from "./types"; import { boundCsFunctionSymbol, exportsByAssembly } from "./gc-handles"; @@ -201,8 +201,14 @@ function bindFn_1RA(closure: BindingClosureCS) { // pre-allocate the promise let promise = resConverter(args); - // call C# side - invokeJSExport(methodHandle, args); + try { + // call C# side + invokeJSExport(methodHandle, args); + } catch (ex) { + // the throw unwinds past endMarshalTaskToJs, which would otherwise adopt it + releaseEagerTaskHolder(promise); + throw ex; + } // in case the C# side returned synchronously promise = endMarshalTaskToJs(args, undefined, promise); @@ -266,8 +272,14 @@ function bindFn_2RA(closure: BindingClosureCS) { // pre-allocate the promise let promise = resConverter(args); - // call C# side - invokeJSExport(methodHandle, args); + try { + // call C# side + invokeJSExport(methodHandle, args); + } catch (ex) { + // the throw unwinds past endMarshalTaskToJs, which would otherwise adopt it + releaseEagerTaskHolder(promise); + throw ex; + } // in case the C# side returned synchronously promise = endMarshalTaskToJs(args, undefined, promise); @@ -311,7 +323,13 @@ function bindFn(closure: BindingClosureCS) { // call C# side if (isAsync) { - invokeJSExport(methodHandle, args); + try { + invokeJSExport(methodHandle, args); + } catch (ex) { + // the throw unwinds past endMarshalTaskToJs, which would otherwise adopt it + releaseEagerTaskHolder(jsResult); + throw ex; + } // in case the C# side returned synchronously jsResult = endMarshalTaskToJs(args, undefined, jsResult); } else if (isDiscardNoWait) { diff --git a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-js.ts b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-js.ts index e355d9e3e44157..a70928bf55f488 100644 --- a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-js.ts +++ b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/invoke-js.ts @@ -227,6 +227,7 @@ function bindFn(closure: BindingClosureJS) { const fqn = closure.fqn; (closure) = null; return function boundFn(args: JSMarshalerArguments) { + // TODO-MT: always false until threads are enabled, nothing sets ReceiverShouldFree yet const receiverShouldFree = isReceiverShouldFree(args); const mark = startMeasure(); try { @@ -253,6 +254,9 @@ function bindFn(closure: BindingClosureJS) { } } } catch (ex) { + // TODO-MT: once threads are enabled, an async import posted to another thread leaves the + // caller awaiting the pre-created Task and it never reads this slot, so the failure has + // to be delivered through the Task instead. See bind_fn in src/mono/browser/runtime. marshalExceptionToCs(args, ex); } finally { if (receiverShouldFree) { diff --git a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/managed-exports.ts b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/managed-exports.ts index 8b82fc3da2b47c..f7ce07d981d9ca 100644 --- a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/managed-exports.ts +++ b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/managed-exports.ts @@ -6,7 +6,7 @@ import type { JSMarshalerArguments, GCHandle, MarshalerToCs, MarshalerToJs, CSFn import { dotnetAssert, dotnetBrowserUtilsExports, dotnetInteropJSExports, Module } from "./cross-module"; import { allocStackFrame, getArg, isArgsException, setArgType, setGcHandle } from "./marshal"; import { marshalExceptionToCs, marshalStringToCs } from "./marshal-to-cs"; -import { beginMarshalTaskToJs, endMarshalTaskToJs, marshalExceptionToJs, marshalInt32ToJs, marshalStringToJs } from "./marshal-to-js"; +import { beginMarshalTaskToJs, endMarshalTaskToJs, marshalExceptionToJs, marshalInt32ToJs, marshalStringToJs, releaseEagerTaskHolder } from "./marshal-to-js"; import { assertJsInterop, assertRuntimeRunning, isRuntimeRunning } from "./utils"; import { MarshalerType } from "./types"; @@ -167,10 +167,11 @@ export function bindAssemblyExports(assemblyName: string): Promise { if (!error || typeof error.status !== "number") { dotnetBrowserUtilsExports.abortPosix(1, error, true); } + releaseEagerTaskHolder(promise); throw error; } if (isArgsException(args)) { - // TODO free pre-created promise + releaseEagerTaskHolder(promise); const exc = getArg(args, 0); throw marshalExceptionToJs(exc); } diff --git a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-js.ts b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-js.ts index 52b97150814fa0..03dc3da41ef682 100644 --- a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-js.ts +++ b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-js.ts @@ -16,7 +16,7 @@ import { isReceiverShouldFree, } from "./marshal"; import { marshalExceptionToCs } from "./marshal-to-cs"; -import { lookupJsOwnedObject, getJsHandleFromJSObject, getJSObjectFromJSHandle, registerWithJsvHandle, releaseCSOwnedObject, setupManagedProxy, teardownManagedProxy, proxyDebugSymbol } from "./gc-handles"; +import { lookupJsOwnedObject, getJsHandleFromJSObject, getJSObjectFromJSHandle, registerWithJsvHandle, releaseCSOwnedObject, setupManagedProxy, teardownManagedProxy, proxyDebugSymbol, eagerTaskHandleSymbol } from "./gc-handles"; import { assertRuntimeRunning, fixupPointer, isRuntimeRunning } from "./utils"; import { ArraySegment, ManagedError, ManagedObject, MemoryViewType, Span } from "./marshaled-types"; import { callDelegate } from "./managed-exports"; @@ -243,9 +243,21 @@ export function beginMarshalTaskToJs(arg: JSMarshalerArgument, _?: MarshalerType } setJsHandle(arg, jsHandle); setArgType(arg, MarshalerType.TaskPreCreated); + // the caller only gets the promise back, so it needs a way to find the handle again. + // storing the number rather than the holder keeps the promise from retaining it. + (holder.promise as any)[eagerTaskHandleSymbol] = jsHandle; return holder.promise; } +// the eagerly created Promise was never handed to managed code, drop its proxy +export function releaseEagerTaskHolder(eagerPromise: Promise | null | undefined): void { + if (!eagerPromise) return; + const jsHandle = (eagerPromise as any)[eagerTaskHandleSymbol]; + dotnetAssert.check(jsHandle, "Expected JSHandle on the eagerly created promise"); + (eagerPromise as any)[eagerTaskHandleSymbol] = undefined; + releaseCSOwnedObject(jsHandle); +} + export function endMarshalTaskToJs(args: JSMarshalerArguments, resConverter: MarshalerToJs | undefined, eagerPromise: Promise | null) { // this path is used when Task is returned from JSExport/call_entry_point const res = getArg(args, 1); @@ -257,8 +269,7 @@ export function endMarshalTaskToJs(args: JSMarshalerArguments, resConverter: Mar } // otherwise drop the eagerPromise's handle - const jsHandle = getJsHandleFromJSObject(eagerPromise); - releaseCSOwnedObject(jsHandle); + releaseEagerTaskHolder(eagerPromise); // get the synchronous result const promise = tryMarshalSyncTaskToJs(res, type, resConverter); @@ -526,6 +537,8 @@ export function resolveOrRejectPromise(args: JSMarshalerArguments): void { } args = fixupPointer(args, 0); const exc = getArg(args, 0); + // TODO-MT: always false until threads are enabled, only the cross-thread post paths set it. + // Keep in sync with resolve_or_reject_promise in src/mono/browser/runtime. const receiverShouldFree = isReceiverShouldFree(args); try { assertRuntimeRunning(); diff --git a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal.ts b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal.ts index b937624441becb..c82e2134cbf0b1 100644 --- a/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal.ts +++ b/src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal.ts @@ -73,6 +73,7 @@ export function getCallerNativeTid(args: JSMarshalerArguments): PThreadPtr { return dotnetApi.getHeapI32(args + JSMarshalerArgumentOffsets.CallerNativeTID) as any; } +// TODO-MT: unused until threads are enabled, the cross-thread post paths set this in the Mono tree export function setReceiverShouldFree(args: JSMarshalerArguments): void { dotnetApi.setHeapB8(args + JSMarshalerArgumentOffsets.ReceiverShouldFree, true); }