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

[wasm] crypto deadlock fix #73537

Merged
merged 10 commits into from
Aug 11, 2022
Merged
2 changes: 1 addition & 1 deletion src/mono/wasm/runtime/exports-linker.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import MonoWasmThreads from "consts:monoWasmThreads";
import { dotnet_browser_can_use_subtle_crypto_impl, dotnet_browser_simple_digest_hash, dotnet_browser_sign, dotnet_browser_encrypt_decrypt, dotnet_browser_derive_bits } from "./crypto-worker";
import { dotnet_browser_can_use_subtle_crypto_impl, dotnet_browser_simple_digest_hash, dotnet_browser_sign, dotnet_browser_encrypt_decrypt, dotnet_browser_derive_bits } from "./subtle-crypto";
import { mono_wasm_fire_debugger_agent_message, mono_wasm_debugger_log, mono_wasm_add_dbg_command_received, mono_wasm_set_entrypoint_breakpoint } from "./debug";
import { mono_wasm_release_cs_owned_object } from "./gc-handles";
import { mono_wasm_load_icu_data, mono_wasm_get_icudt_name } from "./icu";
Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasm/runtime/startup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { mono_wasm_init_aot_profiler, mono_wasm_init_coverage_profiler } from ".
import { mono_on_abort, set_exit_code } from "./run";
import { initialize_marshalers_to_cs } from "./marshal-to-cs";
import { initialize_marshalers_to_js } from "./marshal-to-js";
import { init_crypto } from "./crypto-worker";
import { init_crypto } from "./subtle-crypto";
import { init_polyfills_async } from "./polyfills";
import * as pthreads_worker from "./pthreads/worker";
import { createPromiseController } from "./promise-controller";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,22 +207,13 @@ class LibraryChannel {

public send_msg(msg: string): string {
try {
let state = Atomics.load(this.comm, this.STATE_IDX);
// FIXME: this console write is possibly serializing the access and prevents a deadlock
if (state !== this.STATE_IDLE) console.debug(`MONO_WASM_ENCRYPT_DECRYPT: send_msg, waiting for idle now, ${state}`);
state = this.wait_for_state(pstate => pstate == this.STATE_IDLE, "waiting");

this.wait_for_state_change_to(pstate => pstate == this.STATE_IDLE, "waiting");
this.send_request(msg);
return this.read_response();
} catch (err) {
this.reset(LibraryChannel._stringify_err(err));
this.reset(LibraryChannel.stringify_err(err));
throw err;
}
finally {
const state = Atomics.load(this.comm, this.STATE_IDX);
// FIXME: this console write is possibly serializing the access and prevents a deadlock
if (state !== this.STATE_IDLE) console.debug(`MONO_WASM_ENCRYPT_DECRYPT: state at end of send_msg: ${state}`);
}
}

public shutdown(): void {
Expand All @@ -231,9 +222,11 @@ class LibraryChannel {
if (state !== this.STATE_IDLE)
throw new Error(`OWNER: Invalid sync communication channel state: ${state}`);

this.using_lock(() => {
Atomics.store(this.comm, this.MSG_SIZE_IDX, 0);
this.change_state_locked(this.STATE_SHUTDOWN);
});
// Notify webworker
Atomics.store(this.comm, this.MSG_SIZE_IDX, 0);
this._change_state_locked(this.STATE_SHUTDOWN);
Atomics.notify(this.comm, this.STATE_IDX);
}

Expand All @@ -248,8 +241,11 @@ class LibraryChannel {
return;
}

Atomics.store(this.comm, this.MSG_SIZE_IDX, 0);
this._change_state_locked(this.STATE_RESET);
this.using_lock(() => {
Atomics.store(this.comm, this.MSG_SIZE_IDX, 0);
this.change_state_locked(this.STATE_RESET);
});
// Notify webworker
Atomics.notify(this.comm, this.STATE_IDX);
}

Expand All @@ -259,9 +255,7 @@ class LibraryChannel {
let msg_written = 0;

for (; ;) {
this.acquire_lock();

try {
this.using_lock(() => {
// Write the message and return how much was written.
const wrote = this.write_to_msg(msg, msg_written, msg_len);
msg_written += wrote;
Expand All @@ -272,19 +266,20 @@ class LibraryChannel {
// Indicate if this was the whole message or part of it.
state = msg_written === msg_len ? this.STATE_REQ : this.STATE_REQ_P;

// Notify webworker
this._change_state_locked(state);
} finally {
this.release_lock();
}

this.change_state_locked(state);
});
// Notify webworker
Atomics.notify(this.comm, this.STATE_IDX);

// The send message is complete.
if (state === this.STATE_REQ)
if (state === this.STATE_REQ) {
break;
}
else if (state !== this.STATE_REQ_P) {
throw new Error(`Unexpected state ${state}`);
}

this.wait_for_state(state => state == this.STATE_AWAIT, "send_request");
this.wait_for_state_change_to(state => state == this.STATE_AWAIT, "send_request");
}
}

Expand All @@ -302,56 +297,63 @@ class LibraryChannel {
private read_response(): string {
let response = "";
for (; ;) {
const state = this.wait_for_state(state => state == this.STATE_RESP || state == this.STATE_RESP_P, "read_response");
this.acquire_lock();

try {
this.wait_for_state_change_to(state => state == this.STATE_RESP || state == this.STATE_RESP_P, "read_response");
const done = this.using_lock(() => {
const size_to_read = Atomics.load(this.comm, this.MSG_SIZE_IDX);

// Append the latest part of the message.
response += this.read_from_msg(0, size_to_read);

// The response is complete.
const state = Atomics.load(this.comm, this.STATE_IDX);
if (state === this.STATE_RESP) {
Atomics.store(this.comm, this.MSG_SIZE_IDX, 0);
break;
return true;
} else if (state !== this.STATE_RESP_P) {
throw new Error(`Unexpected state ${state}`);
}

// Reset the size and transition to await state.
Atomics.store(this.comm, this.MSG_SIZE_IDX, 0);
this._change_state_locked(this.STATE_AWAIT);
} finally {
this.release_lock();
}
this.change_state_locked(this.STATE_AWAIT);
return false;
});

// Notify webworker
Atomics.notify(this.comm, this.STATE_IDX);

if (done) {
break;
}
}

// Reset the communication channel's state and let the
// webworker know we are done.
this._change_state_locked(this.STATE_IDLE);
this.using_lock(() => {
this.change_state_locked(this.STATE_IDLE);
});
Atomics.notify(this.comm, this.STATE_IDX);

return response;
}

private _change_state_locked(newState: number): void {
private change_state_locked(newState: number): void {
Atomics.store(this.comm, this.STATE_IDX, newState);
}

private wait_for_state(is_ready: (state: number) => boolean, msg: string): number {
private wait_for_state_change_to(is_ready: (state: number) => boolean, msg: string): void {
// Wait for webworker
// - Atomics.wait() is not permissible on the main thread.
for (; ;) {
const lock_state = Atomics.load(this.comm, this.LOCK_IDX);
if (lock_state !== this.LOCK_UNLOCKED)
continue;

const state = Atomics.load(this.comm, this.STATE_IDX);
if (state == this.STATE_REQ_FAILED)
throw new OperationFailedError(`Worker failed during ${msg} with state=${state}`);
const done = this.using_lock(() => {
const state = Atomics.load(this.comm, this.STATE_IDX);
if (state == this.STATE_REQ_FAILED)
throw new OperationFailedError(`Worker failed during ${msg} with state=${state}`);

if (is_ready(state))
return state;
if (is_ready(state))
return true;
});
if (done) return;
}
}

Expand All @@ -361,6 +363,15 @@ class LibraryChannel {
return String.fromCharCode.apply(null, slicedMessage);
}

private using_lock(callback: Function) {
try {
this.acquire_lock();
return callback();
} finally {
this.release_lock();
}
}

private acquire_lock() {
for (; ;) {
const lock_state = Atomics.compareExchange(this.comm, this.LOCK_IDX, this.LOCK_UNLOCKED, this.LOCK_OWNED);
Expand All @@ -381,7 +392,7 @@ class LibraryChannel {
}
}

private static _stringify_err(err: any) {
private static stringify_err(err: any) {
return (err instanceof Error && err.stack !== undefined) ? err.stack : err;
}

Expand Down
Loading