Skip to content

Commit

Permalink
[browser][MT] Fix Promise cancelation (#99397)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelsavara authored Mar 8, 2024
1 parent 80c0df6 commit 8e23fec
Show file tree
Hide file tree
Showing 21 changed files with 73 additions and 48 deletions.
29 changes: 20 additions & 9 deletions src/mono/browser/runtime/cancelable-promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ export function wrap_as_cancelable<T>(inner: Promise<T>): ControllablePromise<T>
}

export function mono_wasm_cancel_promise(task_holder_gc_handle: GCHandle): void {
if (!loaderHelpers.is_runtime_running()) {
mono_log_debug("This promise can't be canceled, mono runtime already exited.");
return;
}
const holder = _lookup_js_owned_object(task_holder_gc_handle) as PromiseHolder;
mono_assert(!!holder, () => `Expected Promise for GCHandle ${task_holder_gc_handle}`);
holder.cancel();
Expand Down Expand Up @@ -75,6 +79,11 @@ export class PromiseHolder extends ManagedObject {

resolve(data: any) {
mono_assert(!this.isResolved, "resolve could be called only once");
mono_assert(!this.isDisposed, "resolve is already disposed.");
if (!loaderHelpers.is_runtime_running()) {
mono_log_debug("This promise resolution can't be propagated to managed code, mono runtime already exited.");
return;
}
if (WasmEnableThreads && !this.setIsResolving()) {
// we know that cancelation is in flight
// because we need to keep the GCHandle alive until until the cancelation arrives
Expand All @@ -89,11 +98,16 @@ export class PromiseHolder extends ManagedObject {
return;
}
this.isResolved = true;
this.complete_task(data, null);
this.complete_task_wrapper(data, null);
}

reject(reason: any) {
mono_assert(!this.isResolved, "reject could be called only once");
mono_assert(!this.isDisposed, "resolve is already disposed.");
if (!loaderHelpers.is_runtime_running()) {
mono_log_debug("This promise rejection can't be propagated to managed code, mono runtime already exited.");
return;
}
const isCancelation = reason && reason[promise_holder_symbol] === this;
if (WasmEnableThreads && !isCancelation && !this.setIsResolving()) {
// we know that cancelation is in flight
Expand All @@ -109,21 +123,22 @@ export class PromiseHolder extends ManagedObject {
return;
}
this.isResolved = true;
this.complete_task(null, reason);
this.complete_task_wrapper(null, reason);
}

cancel() {
mono_assert(!this.isResolved, "cancel could be called only once");
mono_assert(!this.isDisposed, "resolve is already disposed.");

if (this.isPostponed) {
// there was racing resolve/reject which was postponed, to retain valid GCHandle
// in this case we just finish the original resolve/reject
// and we need to use the postponed data/reason
this.isResolved = true;
if (this.reason !== undefined) {
this.complete_task(null, this.reason);
this.complete_task_wrapper(null, this.reason);
} else {
this.complete_task(this.data, null);
this.complete_task_wrapper(this.data, null);
}
} else {
// there is no racing resolve/reject, we can reject/cancel the promise
Expand All @@ -138,11 +153,7 @@ export class PromiseHolder extends ManagedObject {
}

// we can do this just once, because it will be dispose the GCHandle
complete_task(data: any, reason: any) {
if (!loaderHelpers.is_runtime_running()) {
mono_log_debug("This promise can't be propagated to managed code, mono runtime already exited.");
return;
}
complete_task_wrapper(data: any, reason: any) {
try {
mono_assert(!this.isPosted, "Promise is already posted to managed.");
this.isPosted = true;
Expand Down
7 changes: 7 additions & 0 deletions src/mono/browser/runtime/dotnet.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,13 @@ type APIType = {
* @returns exit code of the Main() method.
*/
runMainAndExit: (mainAssemblyName?: string, args?: string[]) => Promise<number>;
/**
* Exits the runtime.
* Note: after the runtime exits, it would reject all further calls to the API.
* @param code "process" exit code.
* @param reason could be a string or an Error object.
*/
exit: (code: number, reason?: any) => void;
/**
* Sets the environment variable for the "process"
* @param name
Expand Down
1 change: 1 addition & 0 deletions src/mono/browser/runtime/export-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export function export_api(): any {
const api: APIType = {
runMain: mono_run_main,
runMainAndExit: mono_run_main_and_exit,
exit: loaderHelpers.mono_exit,
setEnvironmentVariable: mono_wasm_setenv,
getAssemblyExports: mono_wasm_get_assembly_exports,
setModuleImports: mono_wasm_set_module_imports,
Expand Down
3 changes: 3 additions & 0 deletions src/mono/browser/runtime/gc-handles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ export function setup_managed_proxy(owner: any, gc_handle: GCHandle): void {

export function upgrade_managed_proxy_to_strong_ref(owner: any, gc_handle: GCHandle): void {
const sr = create_strong_ref(owner);
if (_use_finalization_registry) {
_js_owned_object_registry.unregister(owner);
}
_js_owned_object_table.set(gc_handle, sr);
}

Expand Down
1 change: 0 additions & 1 deletion src/mono/browser/runtime/interp-pgo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ export async function getCacheKey(prefix: string): Promise<string | null> {
delete inputs.forwardConsoleLogsToWS;
delete inputs.diagnosticTracing;
delete inputs.appendElementOnExit;
delete inputs.assertAfterExit;
delete inputs.interopCleanupOnExit;
delete inputs.dumpThreadsOnNonZeroExit;
delete inputs.logExitCode;
Expand Down
2 changes: 2 additions & 0 deletions src/mono/browser/runtime/invoke-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export function mono_wasm_invoke_jsimport(signature: JSFunctionSignature, args:

export function mono_wasm_invoke_jsimport_ST(function_handle: JSFnHandle, args: JSMarshalerArguments): void {
if (WasmEnableThreads) return;
loaderHelpers.assert_runtime_running();
const bound_fn = js_import_wrapper_by_fn_handle[<any>function_handle];
mono_assert(bound_fn, () => `Imported function handle expected ${function_handle}`);
bound_fn(args);
Expand Down Expand Up @@ -336,6 +337,7 @@ type BindingClosure = {
}

export function mono_wasm_invoke_js_function(bound_function_js_handle: JSHandle, args: JSMarshalerArguments): void {
loaderHelpers.assert_runtime_running();
const bound_fn = mono_wasm_get_jsobj_from_js_handle(bound_function_js_handle);
mono_assert(bound_fn && typeof (bound_fn) === "function" && bound_fn[bound_js_function_symbol], () => `Bound function handle expected ${bound_function_js_handle}`);
bound_fn(args);
Expand Down
4 changes: 1 addition & 3 deletions src/mono/browser/runtime/loader/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import WasmEnableThreads from "consts:wasmEnableThreads";

import { MainThreadingMode, type DotnetModuleInternal, type MonoConfigInternal, JSThreadBlockingMode, JSThreadInteropMode } from "../types/internal";
import type { DotnetModuleConfig, MonoConfig, ResourceGroups, ResourceList } from "../types";
import { ENVIRONMENT_IS_WEB, exportedRuntimeAPI, loaderHelpers, runtimeHelpers } from "./globals";
import { exportedRuntimeAPI, loaderHelpers, runtimeHelpers } from "./globals";
import { mono_log_error, mono_log_debug } from "./logging";
import { importLibraryInitializers, invokeLibraryInitializers } from "./libraryInitializers";
import { mono_exit } from "./exit";
Expand Down Expand Up @@ -178,8 +178,6 @@ export function normalizeConfig() {
}
}

loaderHelpers.assertAfterExit = config.assertAfterExit = config.assertAfterExit || !ENVIRONMENT_IS_WEB;

if (config.debugLevel === undefined && BuildConfiguration === "Debug") {
config.debugLevel = -1;
}
Expand Down
16 changes: 4 additions & 12 deletions src/mono/browser/runtime/loader/exit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,11 @@ export function is_runtime_running() {
}

export function assert_runtime_running() {
if (!is_exited()) {
if (WasmEnableThreads && ENVIRONMENT_IS_WORKER) {
mono_assert(runtimeHelpers.runtimeReady, "The WebWorker is not attached to the runtime. See https://github.com/dotnet/runtime/blob/main/src/mono/wasm/threads.md#JS-interop-on-dedicated-threads");
} else {
mono_assert(runtimeHelpers.runtimeReady, ".NET runtime didn't start yet. Please call dotnet.create() first.");
}
mono_assert(!is_exited(), () => `.NET runtime already exited with ${loaderHelpers.exitCode} ${loaderHelpers.exitReason}. You can use runtime.runMain() which doesn't exit the runtime.`);
if (WasmEnableThreads && ENVIRONMENT_IS_WORKER) {
mono_assert(runtimeHelpers.runtimeReady, "The WebWorker is not attached to the runtime. See https://github.com/dotnet/runtime/blob/main/src/mono/wasm/threads.md#JS-interop-on-dedicated-threads");
} else {
const message = `.NET runtime already exited with ${loaderHelpers.exitCode} ${loaderHelpers.exitReason}. You can use runtime.runMain() which doesn't exit the runtime.`;
if (loaderHelpers.assertAfterExit) {
mono_assert(false, message);
} else {
mono_log_warn(message);
}
mono_assert(runtimeHelpers.runtimeReady, ".NET runtime didn't start yet. Please call dotnet.create() first.");
}
}

Expand Down
1 change: 0 additions & 1 deletion src/mono/browser/runtime/loader/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ export function setLoaderGlobals(

maxParallelDownloads: 16,
enableDownloadRetry: true,
assertAfterExit: !ENVIRONMENT_IS_WEB,

_loaded_files: [],
loadedFiles: [],
Expand Down
13 changes: 0 additions & 13 deletions src/mono/browser/runtime/loader/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,19 +138,6 @@ export class HostBuilder implements DotnetHostBuilder {
}
}

// internal
withAssertAfterExit(): DotnetHostBuilder {
try {
deep_merge_config(monoConfig, {
assertAfterExit: true
});
return this;
} catch (err) {
mono_exit(1, err);
throw err;
}
}

// internal
// todo fallback later by debugLevel
withWaitingForDebugger(level: number): DotnetHostBuilder {
Expand Down
2 changes: 2 additions & 0 deletions src/mono/browser/runtime/managed-exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export function call_entry_point(main_assembly_name: string, program_args: strin

// the marshaled signature is: void LoadSatelliteAssembly(byte[] dll)
export function load_satellite_assembly(dll: Uint8Array): void {
loaderHelpers.assert_runtime_running();
const sp = Module.stackSave();
try {
const size = 3;
Expand All @@ -95,6 +96,7 @@ export function load_satellite_assembly(dll: Uint8Array): void {

// the marshaled signature is: void LoadLazyAssembly(byte[] dll, byte[] pdb)
export function load_lazy_assembly(dll: Uint8Array, pdb: Uint8Array | null): void {
loaderHelpers.assert_runtime_running();
const sp = Module.stackSave();
try {
const size = 4;
Expand Down
5 changes: 5 additions & 0 deletions src/mono/browser/runtime/marshal-to-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { get_marshaler_to_cs_by_type, jsinteropDoc, marshal_exception_to_cs } fr
import { localHeapViewF64, localHeapViewI32, localHeapViewU8 } from "./memory";
import { call_delegate } from "./managed-exports";
import { gc_locked } from "./gc-lock";
import { mono_log_debug } from "./logging";

export function initialize_marshalers_to_js(): void {
if (cs_to_js_marshalers.size == 0) {
Expand Down Expand Up @@ -337,6 +338,10 @@ function create_task_holder(res_converter?: MarshalerToJs) {
}

export function mono_wasm_resolve_or_reject_promise(args: JSMarshalerArguments): void {
if (!loaderHelpers.is_runtime_running()) {
mono_log_debug("This promise resolution/rejection can't be propagated to managed code, mono runtime already exited.");
return;
}
const exc = get_arg(args, 0);
const receiver_should_free = WasmEnableThreads && is_receiver_should_free(args);
try {
Expand Down
2 changes: 1 addition & 1 deletion src/mono/browser/runtime/memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ export function compareExchangeI32(offset: MemOffset, value: number, expected: n
}
return actual;
}
return globalThis.Atomics.compareExchange(localHeapViewI32(), <any>offset >>> 2, value, expected);
return globalThis.Atomics.compareExchange(localHeapViewI32(), <any>offset >>> 2, expected, value);
}

export function storeI32(offset: MemOffset, value: number): void {
Expand Down
7 changes: 7 additions & 0 deletions src/mono/browser/runtime/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,13 @@ export type APIType = {
* @returns exit code of the Main() method.
*/
runMainAndExit: (mainAssemblyName?: string, args?: string[]) => Promise<number>;
/**
* Exits the runtime.
* Note: after the runtime exits, it would reject all further calls to the API.
* @param code "process" exit code.
* @param reason could be a string or an Error object.
*/
exit: (code: number, reason?: any) => void;
/**
* Sets the environment variable for the "process"
* @param name
Expand Down
2 changes: 0 additions & 2 deletions src/mono/browser/runtime/types/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ export type MonoConfigInternal = MonoConfig & {
browserProfilerOptions?: BrowserProfilerOptions, // dictionary-style Object. If omitted, browser profiler will not be initialized.
waitForDebugger?: number,
appendElementOnExit?: boolean
assertAfterExit?: boolean // default true for shell/nodeJS
interopCleanupOnExit?: boolean
dumpThreadsOnNonZeroExit?: boolean
logExitCode?: boolean
Expand Down Expand Up @@ -123,7 +122,6 @@ export type LoaderHelpers = {

maxParallelDownloads: number;
enableDownloadRetry: boolean;
assertAfterExit: boolean;

exitCode: number | undefined;
exitReason: any;
Expand Down
1 change: 0 additions & 1 deletion src/mono/browser/test-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ function configureRuntime(dotnet, runArgs) {
.withExitCodeLogging()
.withElementOnExit()
.withInteropCleanupOnExit()
.withAssertAfterExit()
.withDumpThreadsOnNonZeroExit()
.withConfig({
loadAllSatelliteResources: true
Expand Down
1 change: 0 additions & 1 deletion src/mono/sample/wasm/browser-shutdown/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ try {
.withExitOnUnhandledError()
.withExitCodeLogging()
.withElementOnExit()
.withAssertAfterExit()
.withOnConfigLoaded(() => {
// you can test abort of the startup by opening http://localhost:8000/?throwError=true
const params = new URLSearchParams(location.search);
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,8 @@ In Blazor, you can customize the startup in your index.html
<script src="_framework/blazor.webassembly.js" autostart="false"></script>
<script>
Blazor.start({
configureRuntime: function (builder) {
builder.withConfig({
configureRuntime: function (dotnet) {
dotnet.withConfig({
browserProfilerOptions: {}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@
{
await DisposeHubConnection();
// exit the client
await JSRuntime.InvokeVoidAsync("eval", "import('./dotnet.js').then(module => { module.dotnet; module.exit(0); });");
Helper.TestOutputWriteLine($"SendExitSignal by CurrentManagedThreadId={Environment.CurrentManagedThreadId}");
await JSRuntime.InvokeVoidAsync("eval", "setTimeout(() => { getDotnetRuntime(0).exit(0); }, 50);");
}

private async Task DisposeHubConnection()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@
<a href="" class="reload">Reload</a>
<a class="dismiss">🗙</a>
</div>
<script src="_framework/blazor.webassembly.js"></script>
<script src="_framework/blazor.webassembly.js" autostart="false"></script>
<script>
Blazor.start({
configureRuntime: function (dotnet) {
dotnet.withExitCodeLogging();
}
});
</script>
</body>

</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"Logging": {
"LogLevel": {
"Default": "Information",
"Microsoft.AspNetCore": "Warning"
}
}
}

0 comments on commit 8e23fec

Please sign in to comment.