[browser] dispose more JS proxies (#89559)
authorPavel Savara <pavel.savara@gmail.com>
Fri, 28 Jul 2023 20:56:05 +0000 (22:56 +0200)
committerGitHub <noreply@github.com>
Fri, 28 Jul 2023 20:56:05 +0000 (22:56 +0200)
src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs
src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSException.cs
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSHostImplementation.cs
src/mono/wasm/runtime/globals.ts
src/mono/wasm/runtime/loader/globals.ts
src/mono/wasm/runtime/marshal-to-js.ts
src/mono/wasm/runtime/startup.ts
src/mono/wasm/runtime/types/internal.ts
src/mono/wasm/runtime/web-socket.ts

index 73cb9f7..2cf419e 100644 (file)
@@ -233,7 +233,7 @@ namespace System.Net.Http
 
                 cancellationToken.ThrowIfCancellationRequested();
                 JSObject fetchResponse = await BrowserHttpInterop.CancelationHelper(promise, cancellationToken, abortController, null).ConfigureAwait(true);
-                return new WasmFetchResponse(fetchResponse, abortRegistration.Value);
+                return new WasmFetchResponse(fetchResponse, abortController, abortRegistration.Value);
             }
             catch (JSException jse)
             {
@@ -243,6 +243,7 @@ namespace System.Net.Http
             {
                 // this would also trigger abort
                 abortRegistration?.Dispose();
+                abortController?.Dispose();
                 throw;
             }
         }
@@ -317,15 +318,18 @@ namespace System.Net.Http
         public readonly object ThisLock = new object();
 #endif
         public JSObject? FetchResponse;
+        private readonly JSObject _abortController;
         private readonly CancellationTokenRegistration _abortRegistration;
         private bool _isDisposed;
 
-        public WasmFetchResponse(JSObject fetchResponse, CancellationTokenRegistration abortRegistration)
+        public WasmFetchResponse(JSObject fetchResponse, JSObject abortController, CancellationTokenRegistration abortRegistration)
         {
             ArgumentNullException.ThrowIfNull(fetchResponse);
+            ArgumentNullException.ThrowIfNull(abortController);
 
             FetchResponse = fetchResponse;
             _abortRegistration = abortRegistration;
+            _abortController = abortController;
         }
 
         public void ThrowIfDisposed()
@@ -354,6 +358,7 @@ namespace System.Net.Http
                         return;
                     self._isDisposed = true;
                     self._abortRegistration.Dispose();
+                    self._abortController.Dispose();
                     if (!self.FetchResponse!.IsDisposed)
                     {
                         BrowserHttpInterop.AbortResponse(self.FetchResponse);
@@ -366,6 +371,7 @@ namespace System.Net.Http
 #else
             _isDisposed = true;
             _abortRegistration.Dispose();
+            _abortController.Dispose();
             if (FetchResponse != null)
             {
                 if (!FetchResponse.IsDisposed)
index 746feb4..111957c 100644 (file)
@@ -615,6 +615,7 @@ namespace System.Net.Http.Functional.Tests
         }
 
         [Fact]
+        [SkipOnPlatform(TestPlatforms.Browser, "Browser is relaxed about validating HTTP headers")]
         public async Task FailedRequests_ConnectionClosedWhileReceivingHeaders_Recorded()
         {
             using CancellationTokenSource cancelServerCts = new CancellationTokenSource();
index c2cf295..f06834b 100644 (file)
@@ -13,6 +13,7 @@ namespace System.Runtime.InteropServices.JavaScript
     public sealed class JSException : Exception
     {
         internal JSObject? jsException;
+        internal string? combinedStackTrace;
 
         /// <summary>
         /// Initializes a new instance of the JSException class with a specified error message.
@@ -21,11 +22,13 @@ namespace System.Runtime.InteropServices.JavaScript
         public JSException(string msg) : base(msg)
         {
             jsException = null;
+            combinedStackTrace = null;
         }
 
         internal JSException(string msg, JSObject? jsException) : base(msg)
         {
             this.jsException = jsException;
+            this.combinedStackTrace = null;
         }
 
         /// <inheritdoc />
@@ -33,6 +36,10 @@ namespace System.Runtime.InteropServices.JavaScript
         {
             get
             {
+                if (combinedStackTrace != null)
+                {
+                    return combinedStackTrace;
+                }
                 var bs = base.StackTrace;
                 if (jsException == null)
                 {
@@ -42,16 +49,19 @@ namespace System.Runtime.InteropServices.JavaScript
 #if FEATURE_WASM_THREADS
                 if (jsException.OwnerThreadId != Thread.CurrentThread.ManagedThreadId)
                 {
-                    return null;
+                    return bs;
                 }
 #endif
                 string? jsStackTrace = jsException.GetPropertyAsString("stack");
+
+                // after this, we don't need jsException proxy anymore
+                jsException.Dispose();
+                jsException = null;
+
                 if (jsStackTrace == null)
                 {
-                    if (bs == null)
-                    {
-                        return null;
-                    }
+                    combinedStackTrace = bs;
+                    return combinedStackTrace;
                 }
                 else if (jsStackTrace.StartsWith(Message + "\n"))
                 {
@@ -62,11 +72,15 @@ namespace System.Runtime.InteropServices.JavaScript
 
                 if (bs == null)
                 {
-                    return jsStackTrace;
+                    combinedStackTrace = jsStackTrace;
                 }
-                return base.StackTrace + "\r\n" + jsStackTrace;
-            }
 
+                combinedStackTrace = bs != null
+                    ? bs + Environment.NewLine + jsStackTrace
+                    : jsStackTrace;
+
+                return combinedStackTrace;
+            }
         }
 
         /// <inheritdoc />
index 94707e6..248740b 100644 (file)
@@ -275,20 +275,6 @@ namespace System.Runtime.InteropServices.JavaScript
                             jso.Dispose();
                         }
                     }
-                    foreach (var gch in ThreadJsOwnedObjects.Values)
-                    {
-                        GCHandle gcHandle = (GCHandle)gch;
-
-                        // if this is pending promise we reject it
-                        if (gcHandle.Target is TaskCallback holder)
-                        {
-                            unsafe
-                            {
-                                holder.Callback!.Invoke(null);
-                            }
-                        }
-                        gcHandle.Free();
-                    }
                     SynchronizationContext.SetSynchronizationContext(ctx!.previousSynchronizationContext);
                     JSSynchronizationContext.CurrentJSSynchronizationContext = null;
                     ctx.isDisposed = true;
@@ -309,9 +295,36 @@ namespace System.Runtime.InteropServices.JavaScript
                     Environment.FailFast($"There should be no JS proxies of managed objects on this thread, ManagedThreadId: {Thread.CurrentThread.ManagedThreadId}");
                 }
             }
+
+            Interop.Runtime.UninstallWebWorkerInterop(uninstallJSSynchronizationContext);
+
+            if (uninstallJSSynchronizationContext)
+            {
+                try
+                {
+                    foreach (var gch in ThreadJsOwnedObjects.Values)
+                    {
+                        GCHandle gcHandle = (GCHandle)gch;
+
+                        // if this is pending promise we reject it
+                        if (gcHandle.Target is TaskCallback holder)
+                        {
+                            unsafe
+                            {
+                                holder.Callback!.Invoke(null);
+                            }
+                        }
+                        gcHandle.Free();
+                    }
+                }
+                catch(Exception ex)
+                {
+                    Environment.FailFast($"Unexpected error in UninstallWebWorkerInterop, ManagedThreadId: {Thread.CurrentThread.ManagedThreadId}. " + ex);
+                }
+            }
+
             ThreadCsOwnedObjects.Clear();
             ThreadJsOwnedObjects.Clear();
-            Interop.Runtime.UninstallWebWorkerInterop(uninstallJSSynchronizationContext);
         }
 
         private static FieldInfo? thread_id_Field;
index 9ca3d5d..2443e20 100644 (file)
@@ -5,7 +5,6 @@
 /// <reference path="./types/v8.d.ts" />
 /// <reference path="./types/node.d.ts" />
 
-import { mono_log_error } from "./logging";
 import { RuntimeAPI } from "./types/index";
 import type { GlobalObjects, EmscriptenInternals, RuntimeHelpers, LoaderHelpers, DotnetModuleInternal, PromiseAndController } from "./types/internal";
 
@@ -87,10 +86,6 @@ export function mono_assert(condition: unknown, messageFactory: string | (() =>
     const message = "Assert failed: " + (typeof messageFactory === "function"
         ? messageFactory()
         : messageFactory);
-    const abort = runtimeHelpers.mono_wasm_abort;
-    if (abort) {
-        mono_log_error(message);
-        abort();
-    }
-    throw new Error(message);
+    const error = new Error(message);
+    runtimeHelpers.abort(error);
 }
\ No newline at end of file
index 5f4e833..8905bc0 100644 (file)
@@ -8,7 +8,7 @@ import type { MonoConfig, RuntimeAPI } from "../types";
 import { assert_runtime_running, is_exited, is_runtime_running, mono_exit } from "./exit";
 import { assertIsControllablePromise, createPromiseController, getPromiseController } from "./promise-controller";
 import { mono_download_assets, resolve_asset_path } from "./assets";
-import { mono_log_error, setup_proxy_console } from "./logging";
+import { setup_proxy_console } from "./logging";
 import { hasDebuggingEnabled } from "./blazor/_Polyfill";
 import { invokeLibraryInitializers } from "./libraryInitializers";
 
@@ -17,20 +17,20 @@ export const ENVIRONMENT_IS_WEB = typeof window == "object";
 export const ENVIRONMENT_IS_WORKER = typeof importScripts == "function";
 export const ENVIRONMENT_IS_SHELL = !ENVIRONMENT_IS_WEB && !ENVIRONMENT_IS_NODE && !ENVIRONMENT_IS_WORKER;
 
-export let runtimeHelpers: RuntimeHelpers = null as any;
-export let loaderHelpers: LoaderHelpers = null as any;
-export let exportedRuntimeAPI: RuntimeAPI = null as any;
-export let INTERNAL: any;
+export let runtimeHelpers: RuntimeHelpers = {} as any;
+export let loaderHelpers: LoaderHelpers = {} as any;
+export let exportedRuntimeAPI: RuntimeAPI = {} as any;
+export let INTERNAL: any = {};
 export let _loaderModuleLoaded = false; // please keep it in place also as rollup guard
 
 export const globalObjectsRoot: GlobalObjects = {
     mono: {},
     binding: {},
-    internal: {},
+    internal: INTERNAL,
     module: {},
-    loaderHelpers: {},
-    runtimeHelpers: {},
-    api: {}
+    loaderHelpers,
+    runtimeHelpers,
+    api: exportedRuntimeAPI,
 } as any;
 
 setLoaderGlobals(globalObjectsRoot);
@@ -59,7 +59,8 @@ export function setLoaderGlobals(
         mono_wasm_bindings_is_ready: false,
         javaScriptExports: {} as any,
         config: globalObjects.module.config,
-        diagnosticTracing: false
+        diagnosticTracing: false,
+        abort: (reason: any) => { throw reason; },
     });
     Object.assign(loaderHelpers, {
         config: globalObjects.module.config,
@@ -111,10 +112,6 @@ export function mono_assert(condition: unknown, messageFactory: string | (() =>
     const message = "Assert failed: " + (typeof messageFactory === "function"
         ? messageFactory()
         : messageFactory);
-    const abort = globalObjectsRoot.runtimeHelpers.mono_wasm_abort;
-    if (abort) {
-        mono_log_error(message);
-        abort();
-    }
-    throw new Error(message);
+    const error = new Error(message);
+    runtimeHelpers.abort(error);
 }
\ No newline at end of file
index d861cd4..9b5a931 100644 (file)
@@ -5,7 +5,7 @@ import MonoWasmThreads from "consts:monoWasmThreads";
 import BuildConfiguration from "consts:configuration";
 
 import cwraps from "./cwraps";
-import { _lookup_js_owned_object, mono_wasm_get_jsobj_from_js_handle, mono_wasm_get_js_handle, setup_managed_proxy } from "./gc-handles";
+import { _lookup_js_owned_object, mono_wasm_get_jsobj_from_js_handle, mono_wasm_get_js_handle, setup_managed_proxy, teardown_managed_proxy } from "./gc-handles";
 import { Module, createPromiseController, loaderHelpers, mono_assert, runtimeHelpers } from "./globals";
 import {
     ManagedObject, ManagedError,
@@ -197,7 +197,10 @@ function _marshal_delegate_to_js(arg: JSMarshalerArgument, _?: MarshalerType, re
             return runtimeHelpers.javaScriptExports.call_delegate(gc_handle, arg1_js, arg2_js, arg3_js, res_converter, arg1_converter, arg2_converter, arg3_converter);
         };
         result.dispose = () => {
-            result.isDisposed = true;
+            if (!result.isDisposed) {
+                result.isDisposed = true;
+                teardown_managed_proxy(result, gc_handle);
+            }
         };
         result.isDisposed = false;
         if (BuildConfiguration === "Debug") {
index 12e8341..be65c39 100644 (file)
@@ -344,7 +344,12 @@ function mono_wasm_pre_init_essential(isWorker: boolean): void {
 
     init_c_exports();
     runtimeHelpers.mono_wasm_exit = cwraps.mono_wasm_exit;
-    runtimeHelpers.mono_wasm_abort = cwraps.mono_wasm_abort;
+    runtimeHelpers.abort = (reason: any) => {
+        if (!loaderHelpers.is_exited()) {
+            cwraps.mono_wasm_abort();
+        }
+        throw reason;
+    };
     cwraps_internal(INTERNAL);
     if (WasmEnableLegacyJsInterop && !linkerDisableLegacyJsInterop) {
         cwraps_mono_api(MONO);
index 3b54d6b..66e7fb4 100644 (file)
@@ -174,7 +174,7 @@ export type RuntimeHelpers = {
     ExitStatus: ExitStatusError;
     quit: Function,
     mono_wasm_exit?: (code: number) => void,
-    mono_wasm_abort?: () => void,
+    abort: (reason: any) => void,
     javaScriptExports: JavaScriptExports,
     storeMemorySnapshotPending: boolean,
     memorySnapshotCacheKey: string,
index b085cfe..b5de8be 100644 (file)
@@ -11,6 +11,7 @@ import { VoidPtr } from "./types/emscripten";
 import { PromiseController } from "./types/internal";
 import { mono_log_warn } from "./logging";
 import { viewOrCopy, utf8ToStringRelaxed, stringToUTF8 } from "./strings";
+import { IDisposable } from "./marshal";
 
 const wasm_ws_pending_send_buffer = Symbol.for("wasm ws_pending_send_buffer");
 const wasm_ws_pending_send_buffer_offset = Symbol.for("wasm ws_pending_send_buffer_offset");
@@ -21,6 +22,7 @@ const wasm_ws_pending_open_promise = Symbol.for("wasm ws_pending_open_promise");
 const wasm_ws_pending_close_promises = Symbol.for("wasm ws_pending_close_promises");
 const wasm_ws_pending_send_promises = Symbol.for("wasm ws_pending_send_promises");
 const wasm_ws_is_aborted = Symbol.for("wasm ws_is_aborted");
+const wasm_ws_on_closed = Symbol.for("wasm ws_on_closed");
 const wasm_ws_receive_status_ptr = Symbol.for("wasm ws_receive_status_ptr");
 let mono_wasm_web_socket_close_warning = false;
 const ws_send_buffer_blocking_threshold = 65536;
@@ -41,6 +43,7 @@ function verifyEnvironment() {
 export function ws_wasm_create(uri: string, sub_protocols: string[] | null, receive_status_ptr: VoidPtr, onClosed: (code: number, reason: string) => void): WebSocketExtension {
     verifyEnvironment();
     mono_assert(uri && typeof uri === "string", () => `ERR12: Invalid uri ${typeof uri}`);
+    mono_assert(typeof onClosed === "function", () => `ERR12: Invalid onClosed ${typeof onClosed}`);
 
     const ws = new globalThis.WebSocket(uri, sub_protocols || undefined) as WebSocketExtension;
     const { promise_control: open_promise_control } = createPromiseController<WebSocketExtension>();
@@ -51,6 +54,7 @@ export function ws_wasm_create(uri: string, sub_protocols: string[] | null, rece
     ws[wasm_ws_pending_send_promises] = [];
     ws[wasm_ws_pending_close_promises] = [];
     ws[wasm_ws_receive_status_ptr] = receive_status_ptr;
+    ws[wasm_ws_on_closed] = onClosed as any;
     ws.binaryType = "arraybuffer";
     const local_on_open = () => {
         if (ws[wasm_ws_is_aborted]) return;
@@ -65,10 +69,11 @@ export function ws_wasm_create(uri: string, sub_protocols: string[] | null, rece
     const local_on_close = (ev: CloseEvent) => {
         ws.removeEventListener("message", local_on_message);
         if (ws[wasm_ws_is_aborted]) return;
-        if (onClosed) onClosed(ev.code, ev.reason);
+
+        onClosed(ev.code, ev.reason);
 
         // this reject would not do anything if there was already "open" before it.
-        open_promise_control.reject(ev.reason);
+        open_promise_control.reject(new Error(ev.reason));
 
         for (const close_promise_control of ws[wasm_ws_pending_close_promises]) {
             close_promise_control.resolve();
@@ -82,9 +87,16 @@ export function ws_wasm_create(uri: string, sub_protocols: string[] | null, rece
             setI32(<any>receive_status_ptr + 8, 1);// end_of_message: true
             receive_promise_control.resolve();
         });
+
+        // cleanup the delegate proxy
+        ws[wasm_ws_on_closed].dispose();
     };
     const local_on_error = (ev: any) => {
-        open_promise_control.reject(ev.message || "WebSocket error");
+        if (ws[wasm_ws_is_aborted]) return;
+        ws.removeEventListener("message", local_on_message);
+        const error = new Error(ev.message || "WebSocket error");
+        mono_log_warn("WebSocket error", error);
+        reject_promises(ws, error);
     };
     ws.addEventListener("message", local_on_message);
     ws.addEventListener("open", local_on_open, { once: true });
@@ -177,23 +189,30 @@ export function ws_wasm_abort(ws: WebSocketExtension): void {
     mono_assert(!!ws, "ERR18: expected ws instance");
 
     ws[wasm_ws_is_aborted] = true;
+    reject_promises(ws, new Error("OperationCanceledException"));
+
+    // cleanup the delegate proxy
+    ws[wasm_ws_on_closed]?.dispose();
+
+    // this is different from Managed implementation
+    ws.close(1000, "Connection was aborted.");
+}
+
+function reject_promises(ws: WebSocketExtension, error: Error) {
     const open_promise_control = ws[wasm_ws_pending_open_promise];
     if (open_promise_control) {
-        open_promise_control.reject(new Error("OperationCanceledException"));
+        open_promise_control.reject(error);
     }
     for (const close_promise_control of ws[wasm_ws_pending_close_promises]) {
-        close_promise_control.reject(new Error("OperationCanceledException"));
+        close_promise_control.reject(error);
     }
     for (const send_promise_control of ws[wasm_ws_pending_send_promises]) {
-        send_promise_control.reject(new Error("OperationCanceledException"));
+        send_promise_control.reject(error);
     }
 
     ws[wasm_ws_pending_receive_promise_queue].drain(receive_promise_control => {
-        receive_promise_control.reject(new Error("OperationCanceledException"));
+        receive_promise_control.reject(error);
     });
-
-    // this is different from Managed implementation
-    ws.close(1000, "Connection was aborted.");
 }
 
 // send and return promise
@@ -224,7 +243,7 @@ function _mono_wasm_web_socket_send_and_wait(ws: WebSocketExtension, buffer_view
             if (readyState != WebSocket.OPEN && readyState != WebSocket.CLOSING) {
                 // only reject if the data were not sent
                 // bufferedAmount does not reset to zero once the connection closes
-                promise_control.reject(`InvalidState: ${readyState} The WebSocket is not connected.`);
+                promise_control.reject(new Error(`InvalidState: ${readyState} The WebSocket is not connected.`));
             }
             else if (!promise_control.isDone) {
                 globalThis.setTimeout(polling_check, nextDelay);
@@ -372,6 +391,7 @@ type WebSocketExtension = WebSocket & {
     [wasm_ws_pending_send_promises]: PromiseController<void>[]
     [wasm_ws_pending_close_promises]: PromiseController<void>[]
     [wasm_ws_is_aborted]: boolean
+    [wasm_ws_on_closed]: IDisposable
     [wasm_ws_receive_status_ptr]: VoidPtr
     [wasm_ws_pending_send_buffer_offset]: number
     [wasm_ws_pending_send_buffer_type]: number