Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[browser][MT] Fix Promise cancelation #99397

Merged
merged 7 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
{
await DisposeHubConnection();
// exit the client
await JSRuntime.InvokeVoidAsync("eval", "import('./dotnet.js').then(module => { module.dotnet; module.exit(0); });");
Environment.Exit(0);
pavelsavara marked this conversation as resolved.
Show resolved Hide resolved
}

private async Task DisposeHubConnection()
Expand Down
Loading