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] stop doing grow(0) #101248

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
4 changes: 1 addition & 3 deletions src/mono/browser/runtime/cancelable-promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { _lookup_js_owned_object, teardown_managed_proxy, upgrade_managed_proxy_
import { createPromiseController, loaderHelpers, mono_assert } from "./globals";
import { ControllablePromise, GCHandle, MarshalerToCs } from "./types/internal";
import { ManagedObject } from "./marshal";
import { compareExchangeI32, forceThreadMemoryViewRefresh } from "./memory";
import { compareExchangeI32 } from "./memory";
import { mono_log_debug } from "./logging";
import { complete_task } from "./managed-exports";
import { marshal_cs_object_to_cs } from "./marshal-to-cs";
Expand Down Expand Up @@ -75,7 +75,6 @@ export class PromiseHolder extends ManagedObject {
if (!WasmEnableThreads || this.promiseHolderPtr === 0) {
return true;
}
forceThreadMemoryViewRefresh();
if (compareExchangeI32(this.promiseHolderPtr + PromiseHolderState.IsResolving, 1, 0) === 0) {
return true;
}
Expand Down Expand Up @@ -166,7 +165,6 @@ export class PromiseHolder extends ManagedObject {
try {
mono_assert(!this.isPosted, "Promise is already posted to managed.");
this.isPosted = true;
forceThreadMemoryViewRefresh();

// we can unregister the GC handle just on JS side
teardown_managed_proxy(this, this.gc_handle, /*skipManaged: */ true);
Expand Down
17 changes: 2 additions & 15 deletions src/mono/browser/runtime/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { toBase64StringImpl } from "./base64";
import cwraps from "./cwraps";
import { VoidPtr, CharPtr } from "./types/emscripten";
import { mono_log_warn } from "./logging";
import { forceThreadMemoryViewRefresh, localHeapViewU8 } from "./memory";
import { localHeapViewU8 } from "./memory";
import { utf8ToString } from "./strings";
const commands_received: any = new Map<number, CommandResponse>();
commands_received.remove = function (key: number): CommandResponse {
Expand Down Expand Up @@ -76,8 +76,6 @@ function mono_wasm_malloc_and_set_debug_buffer (command_parameters: string) {
}

export function mono_wasm_send_dbg_command_with_parms (id: number, command_set: number, command: number, command_parameters: string, length: number, valtype: number, newvalue: number): CommandResponseResult {
forceThreadMemoryViewRefresh();

mono_wasm_malloc_and_set_debug_buffer(command_parameters);
cwraps.mono_wasm_send_dbg_command_with_parms(id, command_set, command, _debugger_buffer, length, valtype, newvalue.toString());

Expand All @@ -88,8 +86,6 @@ export function mono_wasm_send_dbg_command_with_parms (id: number, command_set:
}

export function mono_wasm_send_dbg_command (id: number, command_set: number, command: number, command_parameters: string): CommandResponseResult {
forceThreadMemoryViewRefresh();

mono_wasm_malloc_and_set_debug_buffer(command_parameters);
cwraps.mono_wasm_send_dbg_command(id, command_set, command, _debugger_buffer, command_parameters.length);

Expand All @@ -110,16 +106,14 @@ export function mono_wasm_get_dbg_command_info (): CommandResponseResult {
}

export function mono_wasm_debugger_resume (): void {
forceThreadMemoryViewRefresh();
// noop
}

export function mono_wasm_detach_debugger (): void {
forceThreadMemoryViewRefresh();
cwraps.mono_wasm_set_is_debugger_attached(false);
}

export function mono_wasm_change_debugger_log_level (level: number): void {
forceThreadMemoryViewRefresh();
cwraps.mono_wasm_change_debugger_log_level(level);
}

Expand Down Expand Up @@ -155,7 +149,6 @@ export function mono_wasm_wait_for_debugger (): Promise<void> {
export function mono_wasm_debugger_attached (): void {
if (runtimeHelpers.waitForDebugger == -1)
runtimeHelpers.waitForDebugger = 1;
forceThreadMemoryViewRefresh();
cwraps.mono_wasm_set_is_debugger_attached(true);
}

Expand All @@ -168,8 +161,6 @@ export function mono_wasm_set_entrypoint_breakpoint (entrypoint_method_token: nu
console.assert(true, `Adding an entrypoint breakpoint ${_assembly_name_str} at method token ${_entrypoint_method_token}`);
// eslint-disable-next-line no-debugger
debugger;

forceThreadMemoryViewRefresh();
}

function _create_proxy_from_object_id (objectId: string, details: any) {
Expand Down Expand Up @@ -220,8 +211,6 @@ function _create_proxy_from_object_id (objectId: string, details: any) {
}

export function mono_wasm_call_function_on (request: CallRequest): CFOResponse {
forceThreadMemoryViewRefresh();

if (request.arguments != undefined && !Array.isArray(request.arguments))
throw new Error(`"arguments" should be an array, but was ${request.arguments}`);

Expand Down Expand Up @@ -341,7 +330,6 @@ type ValueAsJsonString = {
}

export function mono_wasm_get_details (objectId: string, args = {}): ValueAsJsonString {
forceThreadMemoryViewRefresh();
return _get_cfo_res_details(`dotnet:cfo_res:${objectId}`, args);
}

Expand All @@ -357,7 +345,6 @@ export function mono_wasm_release_object (objectId: string): void {
}

export function mono_wasm_debugger_log (level: number, message_ptr: CharPtr): void {
forceThreadMemoryViewRefresh();
const message = utf8ToString(message_ptr);

if (INTERNAL["logging"] && typeof INTERNAL.logging["debugger"] === "function") {
Expand Down
3 changes: 0 additions & 3 deletions src/mono/browser/runtime/invoke-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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 { forceThreadMemoryViewRefresh } from "./memory";
import { JSFunctionSignature, JSMarshalerArguments, BoundMarshalerToJs, JSFnHandle, BoundMarshalerToCs, JSHandle, MarshalerType, VoidPtrNull } from "./types/internal";
import { VoidPtr } from "./types/emscripten";
import { INTERNAL, Module, loaderHelpers, mono_assert, runtimeHelpers } from "./globals";
Expand Down Expand Up @@ -149,13 +148,11 @@ function bind_js_import (signature: JSFunctionSignature): Function {
}

function async_bound_fn (args: JSMarshalerArguments): void {
forceThreadMemoryViewRefresh();
bound_fn(args);
}
function sync_bound_fn (args: JSMarshalerArguments): void {
const previous = runtimeHelpers.isPendingSynchronousCall;
try {
forceThreadMemoryViewRefresh();
const caller_tid = get_caller_native_tid(args);
runtimeHelpers.isPendingSynchronousCall = runtimeHelpers.managedThreadTID === caller_tid;
bound_fn(args);
Expand Down
3 changes: 1 addition & 2 deletions src/mono/browser/runtime/marshal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import WasmEnableThreads from "consts:wasmEnableThreads";

import { js_owned_gc_handle_symbol, teardown_managed_proxy } from "./gc-handles";
import { Module, loaderHelpers, mono_assert, runtimeHelpers } from "./globals";
import { getF32, getF64, getI16, getI32, getI64Big, getU16, getU32, getU8, setF32, setF64, setI16, setI32, setI64Big, setU16, setU32, setU8, localHeapViewF64, localHeapViewI32, localHeapViewU8, _zero_region, forceThreadMemoryViewRefresh, setB8, getB8 } from "./memory";
import { getF32, getF64, getI16, getI32, getI64Big, getU16, getU32, getU8, setF32, setF64, setI16, setI32, setI64Big, setU16, setU32, setU8, localHeapViewF64, localHeapViewI32, localHeapViewU8, _zero_region, setB8, getB8 } from "./memory";
import { mono_wasm_new_external_root } from "./roots";
import { GCHandle, JSHandle, MonoObject, MonoString, GCHandleNull, JSMarshalerArguments, JSFunctionSignature, JSMarshalerType, JSMarshalerArgument, MarshalerToJs, MarshalerToCs, WasmRoot, MarshalerType, PThreadPtr, PThreadPtrNull, VoidPtrNull } from "./types/internal";
import { TypedArray, VoidPtr } from "./types/emscripten";
Expand Down Expand Up @@ -65,7 +65,6 @@ const enum JSBindingHeaderOffsets {
}

export function alloc_stack_frame (size: number): JSMarshalerArguments {
forceThreadMemoryViewRefresh();
const bytes = JavaScriptMarshalerArgSize * size;
const args = Module.stackAlloc(bytes) as any;
_zero_region(args, bytes);
Expand Down
26 changes: 0 additions & 26 deletions src/mono/browser/runtime/memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,29 +462,3 @@ export function isSharedArrayBuffer (buffer: any): buffer is SharedArrayBuffer {
// See also https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/toStringTag
return sharedArrayBufferDefined && buffer[Symbol.toStringTag] === "SharedArrayBuffer";
}

/*
Problem: When WebWorker is suspended in the browser, the other running threads could `grow` the linear memory in the meantime.
After the thread is un-suspended C code may to try to de-reference pointer which is beyond it's known view.
This is likely V8 bug. We don't have direct evidence, just failed debugger unit tests with MT runtime.
*/
export function forceThreadMemoryViewRefresh () {
// this condition should be eliminated by rollup on non-threading builds and it would become empty method.
if (!WasmEnableThreads) return;

const wasmMemory = runtimeHelpers.getMemory();

/*
Normally when wasm memory grows in v8, this size change is broadcast to other web workers via an 'interrupt', which works by setting a thread-local flag that needs to be checked.
It's possible that at this point in execution the flag has not been checked yet (because this worker was suspended by the debugger in an unknown location),
which means the size change has not taken effect in this worker.
wasmMemory.grow's implementation in v8 checks to see whether other workers have already grown the buffer,
and will update the current worker's knowledge of the buffer's size.
After that we should be able to safely updateMemoryViews and get a correctly sized view.
This only works because their implementation does not skip doing work even when you ask to grow by 0 pages.
*/
wasmMemory.grow(0);
if (wasmMemory.buffer !== Module.HEAPU8.buffer) {
runtimeHelpers.updateMemoryViews();
}
}
3 changes: 0 additions & 3 deletions src/mono/browser/runtime/pthreads/deputy-thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { monoThreadInfo, postMessageToMain, update_thread_info } from "./shared"
import { Module, loaderHelpers, runtimeHelpers } from "../globals";
import { start_runtime } from "../startup";
import { WorkerToMainMessageType } from "../types/internal";
import { forceThreadMemoryViewRefresh } from "../memory";

export function mono_wasm_start_deputy_thread_async () {
if (!WasmEnableThreads) return;
Expand All @@ -29,8 +28,6 @@ export function mono_wasm_start_deputy_thread_async () {
Module.runtimeKeepalivePush();
Module.safeSetTimeout(async () => {
try {
forceThreadMemoryViewRefresh();

await start_runtime();

postMessageToMain({
Expand Down
2 changes: 0 additions & 2 deletions src/mono/browser/runtime/pthreads/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { Module, loaderHelpers, runtimeHelpers } from "../globals";
import { set_thread_prefix } from "../logging";
import { monoMessageSymbol, PThreadPtrNull, WorkerToMainMessageType } from "../types/internal";
import { threads_c_functions as tcwraps } from "../cwraps";
import { forceThreadMemoryViewRefresh } from "../memory";

// A duplicate in loader/assets.ts
export const worker_empty_prefix = " - ";
Expand Down Expand Up @@ -76,7 +75,6 @@ export function exec_synchronization_context_pump (): void {
if (!loaderHelpers.is_runtime_running()) {
return;
}
forceThreadMemoryViewRefresh();
try {
tcwraps.mono_wasm_synchronization_context_pump();
} catch (ex) {
Expand Down
3 changes: 0 additions & 3 deletions src/mono/browser/runtime/pthreads/worker-thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { postRunWorker, preRunWorker } from "../startup";
import { mono_log_debug, mono_log_error } from "../logging";
import { CharPtr } from "../types/emscripten";
import { utf8ToString } from "../strings";
import { forceThreadMemoryViewRefresh } from "../memory";

// re-export some of the events types
export {
Expand Down Expand Up @@ -70,7 +69,6 @@ function monoDedicatedChannelMessageFromMainToWorker (event: MessageEvent<string

export function on_emscripten_thread_init (pthread_ptr: PThreadPtr) {
runtimeHelpers.currentThreadTID = monoThreadInfo.pthreadId = pthread_ptr;
forceThreadMemoryViewRefresh();
}

/// Called by emscripten when a pthread is setup to run on a worker. Can be called multiple times
Expand All @@ -79,7 +77,6 @@ export function on_emscripten_thread_init (pthread_ptr: PThreadPtr) {
export function mono_wasm_pthread_on_pthread_created (): void {
if (!WasmEnableThreads) return;
try {
forceThreadMemoryViewRefresh();
const pthread_id = mono_wasm_pthread_ptr();
mono_assert(pthread_id == monoThreadInfo.pthreadId, `needs to match (mono_wasm_pthread_ptr ${pthread_id}, threadId from thread info ${monoThreadInfo.pthreadId})`);

Expand Down
2 changes: 0 additions & 2 deletions src/mono/browser/runtime/scheduling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import WasmEnableThreads from "consts:wasmEnableThreads";

import cwraps from "./cwraps";
import { Module, loaderHelpers } from "./globals";
import { forceThreadMemoryViewRefresh } from "./memory";

let spread_timers_maximum = 0;
let pump_count = 0;
Expand Down Expand Up @@ -79,7 +78,6 @@ export function mono_wasm_schedule_timer (shortestDueTimeMs: number): void {
function mono_wasm_schedule_timer_tick () {
if (WasmEnableThreads) return;
Module.maybeExit();
forceThreadMemoryViewRefresh();
if (!loaderHelpers.is_runtime_running()) {
return;
}
Expand Down
6 changes: 1 addition & 5 deletions src/mono/browser/runtime/web-socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import WasmEnableThreads from "consts:wasmEnableThreads";
import { prevent_timer_throttling } from "./scheduling";
import { Queue } from "./queue";
import { ENVIRONMENT_IS_NODE, ENVIRONMENT_IS_SHELL, createPromiseController, loaderHelpers, mono_assert } from "./globals";
import { setI32, localHeapViewU8, forceThreadMemoryViewRefresh } from "./memory";
import { setI32, localHeapViewU8 } from "./memory";
import { VoidPtr } from "./types/emscripten";
import { PromiseController } from "./types/internal";
import { mono_log_warn } from "./logging";
Expand Down Expand Up @@ -78,7 +78,6 @@ export function ws_wasm_create (uri: string, sub_protocols: string[] | null, rec
try {
if (ws[wasm_ws_is_aborted]) return;
if (!loaderHelpers.is_runtime_running()) return;
forceThreadMemoryViewRefresh();
open_promise_control.resolve(ws);
prevent_timer_throttling();
} catch (error: any) {
Expand All @@ -89,7 +88,6 @@ export function ws_wasm_create (uri: string, sub_protocols: string[] | null, rec
try {
if (ws[wasm_ws_is_aborted]) return;
if (!loaderHelpers.is_runtime_running()) return;
forceThreadMemoryViewRefresh();
web_socket_on_message(ws, ev);
prevent_timer_throttling();
} catch (error: any) {
Expand All @@ -101,7 +99,6 @@ export function ws_wasm_create (uri: string, sub_protocols: string[] | null, rec
ws.removeEventListener("message", local_on_message);
if (ws[wasm_ws_is_aborted]) return;
if (!loaderHelpers.is_runtime_running()) return;
forceThreadMemoryViewRefresh();

ws[wasm_ws_close_received] = true;
ws["close_status"] = ev.code;
Expand Down Expand Up @@ -131,7 +128,6 @@ export function ws_wasm_create (uri: string, sub_protocols: string[] | null, rec
try {
if (ws[wasm_ws_is_aborted]) return;
if (!loaderHelpers.is_runtime_running()) return;
forceThreadMemoryViewRefresh();
ws.removeEventListener("message", local_on_message);
const message = ev.message
? "WebSocket error: " + ev.message
Expand Down
Loading