From: Aleksey Kliger (λgeek) <alklig@microsoft.com>
Date: Sat, 28 Jan 2023 13:22:52 +0000 (-0500)
Subject: [wasm][mt] Don't call Module.removeRunDependency in workers (#81311)
X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~4361
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b6cd3003c05d9be521d6d1ffff9207255d177116;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

[wasm][mt] Don't call Module.removeRunDependency in workers (#81311)

Fixes a sporadic hang that depends on the order in which webworkers get initialized.

The way `removeRunDependency` works is that it decrements a counter (incremented by `addRunDependency`) and when the counter hits 0 it runs a callback that runs `run`.

The problem is that on workers there are no other dependencies, so `removeRunDependency("wasm_pre_init_essential")` ticks the counter down to 0 and calls `run`.

The second problem is that `run` on a worker short-circuits most of the logic and just calls `readyPromiseResolve`, `initRuntime` and `postMessage` and returns.

So the worker posts a message back to the main thread that it is loaded.

Then the main body of dotnet.js in the worker calls `run` a second time. And we send a second `postMessage`.

So the main thread receives 2 "loaded" message for each worker in the pool.

That messes up the counter in `instantiateWasmPThreadWorkerPool` which is waiting for every fresh worker to send just 1 message before allowing the main thread to continue.

As a result, by the time the main thread runs some user code, only half the workers have loaded.

Depending on the timing, that could mean that if the main thread attempts to start a new pthread, that pthread might be in a worker that hasn't loaded the runtime yet.  In that case, the pthread start function doesn't get called, and the main thread hangs in Mono's `create_thread()` helper waiting for the `start_info->registered` semaphore that will never be posted.
---

diff --git a/src/mono/wasm/runtime/startup.ts b/src/mono/wasm/runtime/startup.ts
index ea1a58d..71a51bc 100644
--- a/src/mono/wasm/runtime/startup.ts
+++ b/src/mono/wasm/runtime/startup.ts
@@ -127,7 +127,7 @@ function preInit(userPreInit: (() => void)[]) {
     Module.addRunDependency("mono_pre_init");
     const mark = startMeasure();
     try {
-        mono_wasm_pre_init_essential();
+        mono_wasm_pre_init_essential(false);
         if (runtimeHelpers.diagnosticTracing) console.debug("MONO_WASM: preInit");
         beforePreInit.promise_control.resolve();
         // all user Module.preInit callbacks
@@ -164,7 +164,7 @@ async function preInitWorkerAsync() {
     try {
         if (runtimeHelpers.diagnosticTracing) console.debug("MONO_WASM: preInitWorker");
         beforePreInit.promise_control.resolve();
-        mono_wasm_pre_init_essential();
+        mono_wasm_pre_init_essential(true);
         await init_polyfills_async();
         afterPreInit.promise_control.resolve();
         endMeasure(mark, MeasuredBlock.preInitWorker);
@@ -283,8 +283,9 @@ export function abort_startup(reason: any, should_exit: boolean): void {
 }
 
 // runs in both blazor and non-blazor
-function mono_wasm_pre_init_essential(): void {
-    Module.addRunDependency("mono_wasm_pre_init_essential");
+function mono_wasm_pre_init_essential(isWorker: boolean): void {
+    if (!isWorker)
+        Module.addRunDependency("mono_wasm_pre_init_essential");
 
     if (runtimeHelpers.diagnosticTracing) console.debug("MONO_WASM: mono_wasm_pre_init_essential");
 
@@ -294,7 +295,13 @@ function mono_wasm_pre_init_essential(): void {
     cwraps_mono_api(MONO);
     cwraps_binding_api(BINDING);
 
-    Module.removeRunDependency("mono_wasm_pre_init_essential");
+    // removeRunDependency triggers the dependenciesFulfilled callback (runCaller) in
+    // emscripten - on a worker since we don't have any other dependencies that causes run() to get
+    // called too soon; and then it will get called a second time when dotnet.js calls it directly.
+    // on a worker run() short-cirtcuits and just calls   readyPromiseResolve, initRuntime and postMessage.
+    // sending postMessage twice will break instantiateWasmPThreadWorkerPool on the main thread.
+    if (!isWorker)
+        Module.removeRunDependency("mono_wasm_pre_init_essential");
 }
 
 // runs in both blazor and non-blazor