Skip to content

refactor(web-client): read borrows #29

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.

Sign up for GitHub

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

Open
WiktorStarczewski wants to merge 1 commit into
base: next
Choose a base branch
from
Open

refactor(web-client): read borrows #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Enhancements

* [FEATURE][web] Concurrent `MidenClient` reads now run in parallel. Read-only WASM bindings (`getAccount`, `getAccounts`, `getAccountStorage`, ...) take shared borrows of the inner client, and the JS proxy's write-serialization chain was replaced with a readers-writer lock, so reads coalesce while writes still take the slot exclusively. ([#29](https://github.com/0xMiden/web-sdk/pull/29), original: [0xMiden/miden-client#2080](https://github.com/0xMiden/miden-client/pull/2080))
* [FEATURE][web,react] The 0.14.x line's mobile/MT proving surface is available on the 0.15 series (forward-ported from `main`): `TransactionProver.newCallbackProver(jsFn)` (route prove to a native iOS/Android plugin; wire format matches `RemoteTransactionProver`), `ClientOptions.useWorker?: boolean` / `MidenConfig.useWorker` (opt out of the Web Worker shim — required for callback provers, whose closure cannot cross the worker boundary), `MidenClient._withInnerWebClient(fn)` with the depth-tracked re-entrancy fix, the multi-threaded WASM build at the `@miden-sdk/miden-sdk/mt` + `/mt/lazy` (and `@miden-sdk/react/mt` + `/mt/lazy`) subpaths, and the `miden-mobile-prover` C-ABI crate — now built against the 0.15 protocol, so its `TransactionInputs`/`ProvenTransaction` wire format requires a matching 0.15 SDK (native binaries built from 0.14 do not interoperate). ([#149](https://github.com/0xMiden/web-sdk/pull/149), [#152](https://github.com/0xMiden/web-sdk/pull/152), [#134](https://github.com/0xMiden/web-sdk/pull/134))
* [FEATURE][web] Added `BlockHeader.feeFaucetId()` — the account ID of the fungible faucet whose assets pay transaction verification fees, read from the block's on-chain fee parameters. This is the 0.15 spelling of the 0.14 line's `BlockHeader.nativeAssetId()` (the underlying protocol field was renamed `native_asset_id` → `fee_faucet_id`); consumers discovering the fee/native asset per network should migrate to the new name.

Expand Down
36 changes: 17 additions & 19 deletions crates/web-client/js/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ export class MidenClient {
*
* The callback runs inside `_serializeWasmCall`, so the WASM RefCell is
* held for the duration of `fn`. Concurrent SDK calls (sync, other
* transactions, etc.) queue on the same chain and run after `fn`
* settles. Without this serialization, raw inner-client access would
* race the proxy's chain and trip wasm-bindgen's "recursive use of an
* object detected" panic.
* transactions, etc.) queue on the same readers-writer lock and run
* after `fn` settles. Without this serialization, raw inner-client
* access would race the lock and trip wasm-bindgen's "recursive use of
* an object detected" panic.
*
* Re-entrancy: while `fn` is running, the underlying client's
* `_withInnerLockDepth` counter is bumped so that `_serializeWasmCall`
Expand Down Expand Up @@ -308,28 +308,26 @@ export class MidenClient {
}

/**
* Resolves once every serialized WASM call that was already on the
* internal `_serializeWasmCall` chain when `waitForIdle()` was called
* (execute, submit, prove, apply, sync, or account creation) has
* settled. Use this from callers that need to perform a non-WASM-side
* action — e.g. clearing an in-memory auth key on wallet lock — after
* the kernel finishes, so its auth callback doesn't race with the key
* being cleared.
* Resolves once every WASM call queued on this client before
* `waitForIdle()` was called (execute, submit, prove, apply, sync,
* account creation, proxy-fallback reads) has settled. Use this from
* callers that need to perform a non-WASM-side action — e.g. clearing
* an in-memory auth key on wallet lock — after the kernel finishes, so
* its auth callback doesn't race with the key being cleared.
*
* Does NOT wait for calls enqueued after `waitForIdle()` returns —
* intentional, so a caller can drain and proceed without being blocked
* indefinitely by concurrent workload.
*
* Caveat for `syncState`: `syncStateWithTimeout` awaits the sync lock
* (`acquireSyncLock`, which uses Web Locks) BEFORE putting its WASM
* call onto the chain, so a `syncState` that is queued on the sync
* lock — but has not yet begun its WASM phase — is not visible to
* `waitForIdle` and will not be awaited. Other methods (`newWallet`,
* `executeTransaction`, etc.) route through the chain synchronously
* on call and are always observed.
* (`acquireSyncLock`, which uses Web Locks) BEFORE enqueuing its WASM
* call, so a `syncState` that is queued on the sync lock — but has
* not yet begun its WASM phase — is not visible to `waitForIdle` and
* will not be awaited. Other methods (`newWallet`, `executeTransaction`,
* etc.) route through the queue synchronously on call and are always
* observed.
*
* Safe to call at any time; returns immediately if nothing was in
* flight.
* Safe to call at any time; resolves as soon as the client is idle.
*
* @returns {Promise<void>}
*/
Expand Down
165 changes: 120 additions & 45 deletions crates/web-client/js/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,17 @@ const READ_METHODS = new Set([
"getTransactions",
"listSettingKeys",
"listTags",
"executeProgram",
]);

const MOCK_STORE_NAME = "mock_client_db";

// Suppress unused-variable warnings — these sets exist solely for the CI lint check.
void SYNC_METHODS;
// `SYNC_METHODS` and `READ_METHODS` are read by `createClientProxy` to
// route proxy-fallback calls: sync methods bypass the lock entirely and
// reads go through the shared-borrow path so they can run in parallel.
// `WRITE_METHODS` is consulted only by the CI lint (see
// scripts/check-method-classification.js); suppress unused-variable
// warnings for it.
void WRITE_METHODS;
void READ_METHODS;

const buildTypedArraysExport = (exportObject) => {
return Object.entries(exportObject).reduce(
Expand Down Expand Up @@ -252,6 +254,18 @@ export const getWasmOrThrow = async () => {
/**
* Create a Proxy that forwards missing properties to the underlying WASM
* WebClient.
*
* Async proxy-fallback methods are routed through the readers-writer lock
* on the enclosing `WebClient`:
* - `READ_METHODS` take `&self` on the Rust side, so wasm-bindgen uses
* shared borrows for them and they can run concurrently with other
* reads; they go through `_serializeWasmRead`.
* - Everything else takes `&mut self` and must run alone; those go
* through `_serializeWasmCall`.
*
* `SYNC_METHODS` opts out: they are synchronous in JS and wrapping them
* would change their return type to `Promise<T>`, which is a breaking
* change for consumers that use them as plain getters or builders.
*/
function createClientProxy(instance) {
return new Proxy(instance, {
Expand All @@ -262,7 +276,19 @@ function createClientProxy(instance) {
if (target.wasmWebClient && prop in target.wasmWebClient) {
const value = target.wasmWebClient[prop];
if (typeof value === "function") {
return value.bind(target.wasmWebClient);
if (typeof prop === "string" && SYNC_METHODS.has(prop)) {
return value.bind(target.wasmWebClient);
}
if (typeof prop === "string" && READ_METHODS.has(prop)) {
return (...args) =>
target._serializeWasmRead(() =>
value.apply(target.wasmWebClient, args)
);
}
return (...args) =>
target._serializeWasmCall(() =>
value.apply(target.wasmWebClient, args)
);
}
return value;
}
Expand Down Expand Up @@ -499,37 +525,76 @@ class WebClient {
this.wasmWebClient = null;
this.wasmWebClientPromise = null;

// Promise chain to serialize direct WASM calls that require exclusive
// (&mut self) access. Without this, concurrent calls on the same client
// would panic with "recursive use of an object detected" due to
// wasm-bindgen's internal RefCell.
this._wasmCallChain = Promise.resolve();
// Readers-writer lock state for direct WASM calls. Concurrent calls on
// the same client would otherwise panic with "recursive use of an object
// detected" (wasm-bindgen's internal RefCell): any two `&mut self` calls,
// or an `&mut self` call racing an `&self` call, collide on the borrow.
// Multiple `&self` calls (our reads) can coexist, so they run in parallel
// while writes still take the slot alone.
this._rwQueue = []; // FIFO of { type: 'read' | 'write', fn, resolve, reject }
this._rwRunning = 0;
this._rwRunningType = null; // 'read' | 'write' | null
// Depth counter for `_withInnerWebClient` re-entrancy. While > 0,
// `_serializeWasmCall` runs its callback inline instead of queueing
// it on the chain — see the comment on `_serializeWasmCall` for the
// safety contract.
// `_serializeWasmCall` and `_serializeWasmRead` run their callback
// inline instead of enqueueing it on the readers-writer queue — see
// the comment on `_serializeWasmCall` for the safety contract.
this._withInnerLockDepth = 0;
}

/**
* Serialize a WASM call that requires exclusive (&mut self) access.
* Concurrent calls are queued and executed one at a time.
* Enqueue a WASM call under the readers-writer lock.
* @param {'read' | 'write'} type
* @param {() => Promise<any>} fn
* @returns {Promise<any>}
*/
_enqueueWasmCall(type, fn) {
return new Promise((resolve, reject) => {
this._rwQueue.push({ type, fn, resolve, reject });
this._pumpWasmCalls();
});
}

_pumpWasmCalls() {
while (this._rwQueue.length > 0) {
const head = this._rwQueue[0];
const canStart =
this._rwRunningType === null ||
(this._rwRunningType === "read" && head.type === "read");
if (!canStart) break;
this._rwQueue.shift();
this._rwRunning++;
this._rwRunningType = head.type;
(async () => {
try {
head.resolve(await head.fn());
} catch (err) {
head.reject(err);
} finally {
this._rwRunning--;
if (this._rwRunning === 0) this._rwRunningType = null;
this._pumpWasmCalls();
}
})();
}
}

/**
* Serialize a WASM call that requires exclusive (`&mut self`) access.
* The call waits for every prior queued call (reads and writes) to finish
* and runs alone. New calls queued after it will wait for it to complete.
*
* Wraps both the direct (in-thread) path and the worker-dispatched path.
* On the worker path this is redundant with the worker's own message queue,
* but harmless (the chain resolves immediately on the main thread once the
* worker's postMessage returns). On the direct path it is load-bearing —
* without it, concurrent main-thread callers would panic with
* "recursive use of an object detected" (wasm-bindgen's internal RefCell).
* but harmless. On the direct path it is load-bearing.
*
* Re-entrancy: when invoked from inside a `_withInnerWebClient(fn)`
* callback — detected via `_withInnerLockDepth > 0` — `fn` runs inline
* (no chain enqueue). The outer `_withInnerWebClient` invocation
* already holds the chain via its own wrapping `_serializeWasmCall`,
* (no queue enqueue). The outer `_withInnerWebClient` invocation
* already holds the write slot via its own wrapping `_serializeWasmCall`,
* so enqueueing the inner call would deadlock (the inner queues
* behind the outer; the outer awaits the inner). The inline run is
* safe because the chain still serializes against external callers
* — they queue behind the outer call's chain slot, which only resolves
* safe because the queue still serializes against external callers
* — they queue behind the outer call's write slot, which only resolves
* after `fn` (including all inline re-entries) settles. Callers of
* `_withInnerWebClient` MUST hold an external mutex preventing
* concurrent access via other code paths on this same instance during
Expand All @@ -543,38 +608,48 @@ class WebClient {
if (this._withInnerLockDepth > 0) {
return Promise.resolve().then(fn);
}
const result = this._wasmCallChain.catch(() => {}).then(fn);
this._wasmCallChain = result.catch(() => {});
return result;
return this._enqueueWasmCall("write", fn);
}

/**
* Serialize a WASM call that only requires shared (`&self`) access.
* Concurrent reads run in parallel; a read waits for any in-flight or
* queued write to complete first.
*
* @param {() => Promise<any>} fn - The async function to execute.
* @returns {Promise<any>} The result of fn.
*/
_serializeWasmRead(fn) {
if (this._withInnerLockDepth > 0) {
return Promise.resolve().then(fn);
}
return this._enqueueWasmCall("read", fn);
}

/**
* Returns a promise that resolves once every serialized WASM call that
* was already on `_wasmCallChain` when `waitForIdle()` was called has
* settled. Use this from callers that need to perform a non-WASM-side
* action (e.g. clear an in-memory auth key) AFTER any in-flight
* execute / submit / sync has completed, so the WASM kernel's auth
* callback doesn't race with the key being cleared.
* Resolves once every WASM call queued on this client before `waitForIdle()`
* was called has settled. Use this from callers that need to perform a
* non-WASM-side action (e.g. clear an in-memory auth key) AFTER any
* in-flight execute / submit / sync has completed, so the WASM kernel's
* auth callback doesn't race with the key being cleared.
*
* Does NOT wait for calls enqueued after `waitForIdle()` returns —
* this is intentional, so a caller can drain and then proceed without
* being blocked indefinitely by a concurrent workload.
* Implemented by enqueuing a no-op write: it cannot run until every
* preceding read and write has finished, so when it runs the client is
* idle. Calls queued after `waitForIdle()` returns are not awaited.
*
* Caveat for `syncState`: `syncStateWithTimeout` awaits
* `acquireSyncLock` (Web Locks) BEFORE wrapping its WASM call in
* `_serializeWasmCall`, so a sync that is queued on the sync lock but
* has not yet reached its WASM phase is not on the chain and will not
* be awaited. Every other serialized method (`executeTransaction`,
* `newWallet`, `submitNewTransaction`, `proveTransaction`,
* `applyTransaction`, and the proxy-fallback reads) routes through
* the chain synchronously on call and is always observed.
* `acquireSyncLock` (Web Locks) BEFORE enqueueing its WASM call, so a
* sync that is queued on the sync lock but has not yet reached its WASM
* phase is not on the queue and will not be awaited. Every other
* serialized method (`executeTransaction`, `newWallet`,
* `submitNewTransaction`, `proveTransaction`, `applyTransaction`, and
* the proxy-fallback reads) routes through the queue synchronously on
* call and is always observed.
*
* @returns {Promise<void>}
*/
async waitForIdle() {
// Chain on `_wasmCallChain`; by the time this resolves, any in-flight
// serialized call has settled. Catch so the chain state doesn't leak.
await this._wasmCallChain.catch(() => {});
await this._enqueueWasmCall("write", async () => {}).catch(() => {});
}

// TODO: This will soon conflict with some changes in main.
Expand Down
10 changes: 5 additions & 5 deletions crates/web-client/js/types/api-types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,19 +1059,19 @@ export declare class MidenClient {
/** Returns the current sync height. */
getSyncHeight(): Promise<number>;
/**
* Resolves once every serialized WASM call that was already on the
* internal call chain when `waitForIdle()` was called (execute, submit,
* prove, apply, sync, or account creation) has settled. Use this from
* Resolves once every WASM call queued on this client before
* `waitForIdle()` was called (execute, submit, prove, apply, sync,
* account creation, proxy-fallback reads) has settled. Use this from
* callers that need to perform a non-WASM-side action — e.g. clearing
* an in-memory auth key on wallet lock — after the kernel finishes, so
* its auth callback doesn't race with the key being cleared. Does NOT
* wait for calls enqueued after `waitForIdle()` returns.
*
* Caveat for `sync`: a `syncState` blocked on its sync lock (Web
* Locks) has not yet reached the internal chain, so `waitForIdle`
* Locks) has not yet reached the internal queue, so `waitForIdle`
* does not await it. Other serialized methods are always observed.
*
* Returns immediately if nothing was in flight.
* Resolves as soon as the client is idle.
*/
waitForIdle(): Promise<void>;
/**
Expand Down
22 changes: 11 additions & 11 deletions crates/web-client/src/account.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use crate::{WebClient, js_error_with_context};
impl WebClient {
#[js_export(js_name = "getAccounts")]
pub async fn get_accounts(&self) -> Result<Vec<AccountHeader>, JsErr> {
let mut guard = self.get_mut_inner().await;
let client = guard.as_mut().ok_or_else(|| from_str_err("Client not initialized"))?;
let guard = self.get_inner().await;
let client = guard.as_ref().ok_or_else(|| from_str_err("Client not initialized"))?;
let result = client
.get_account_headers()
.await
Expand All @@ -35,8 +35,8 @@ impl WebClient {
/// This method loads the complete account state including vault, storage, and code.
#[js_export(js_name = "getAccount")]
pub async fn get_account(&self, account_id: &AccountId) -> Result<Option<Account>, JsErr> {
let mut guard = self.get_mut_inner().await;
let client = guard.as_mut().ok_or_else(|| from_str_err("Client not initialized"))?;
let guard = self.get_inner().await;
let client = guard.as_ref().ok_or_else(|| from_str_err("Client not initialized"))?;
client
.get_account(account_id.into())
.await
Expand All @@ -49,8 +49,8 @@ impl WebClient {
/// To check the balance for a single asset, use `accountReader` instead.
#[js_export(js_name = "getAccountVault")]
pub async fn get_account_vault(&self, account_id: &AccountId) -> Result<AssetVault, JsErr> {
let mut guard = self.get_mut_inner().await;
let client = guard.as_mut().ok_or_else(|| from_str_err("Client not initialized"))?;
let guard = self.get_inner().await;
let client = guard.as_ref().ok_or_else(|| from_str_err("Client not initialized"))?;
client
.get_account_vault(account_id.into())
.await
Expand All @@ -66,8 +66,8 @@ impl WebClient {
&self,
account_id: &AccountId,
) -> Result<AccountStorage, JsErr> {
let mut guard = self.get_mut_inner().await;
let client = guard.as_mut().ok_or_else(|| from_str_err("Client not initialized"))?;
let guard = self.get_inner().await;
let client = guard.as_ref().ok_or_else(|| from_str_err("Client not initialized"))?;
client
.get_account_storage(account_id.into())
.await
Expand All @@ -83,8 +83,8 @@ impl WebClient {
&self,
account_id: &AccountId,
) -> Result<Option<AccountCode>, JsErr> {
let mut guard = self.get_mut_inner().await;
let client = guard.as_mut().ok_or_else(|| from_str_err("Client not initialized"))?;
let guard = self.get_inner().await;
let client = guard.as_ref().ok_or_else(|| from_str_err("Client not initialized"))?;
client
.get_account_code(account_id.into())
.await
Expand All @@ -108,7 +108,7 @@ impl WebClient {
/// ```
#[js_export(js_name = "accountReader")]
pub async fn account_reader(&self, account_id: &AccountId) -> Result<AccountReader, JsErr> {
let guard = self.inner.lock().await;
let guard = self.get_inner().await;
let client = guard.as_ref().ok_or_else(|| from_str_err("Client not initialized"))?;
Ok(AccountReader::from(client.account_reader(account_id.into())))
}
Expand Down
Loading
Loading