From 7a8a9e5c058c9a3561ec0e595f9dec54f3c6aca0 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 3 Aug 2026 20:34:08 +0300 Subject: [PATCH 1/2] core+qt: add opt-in asynchronous model registration Bridge registers a model by calling IBackend::registerModel, which is synchronous -- it must have the server-assigned ModelId before it returns. QtWebSocketBackend implements that via a nested QEventLoop parked until the reply arrives. On a WASM main thread Qt refuses to do this at all ("WaitForMoreEvents is not supported on the main thread without asyncify") and aborts the module, so a Qt/QML client compiled to WebAssembly cannot register a single model against a remote backend -- the very first registerModel call kills the page. Add IBackend::registerModelAsync(typeId, factory, contextKey, onRegistered, onError): an optional non-blocking counterpart defaulting to "unsupported" (returns false, calls neither callback) so every backend that hasn't opted in is unaffected. Bridge::registerHandler() (both overloads) now prefers this path when a backend offers one, adding the binding to _handlers and issuing the async call before taking _mtx (so a backend that -- unlike any documented here -- invoked a callback synchronously could not self-deadlock), falling back to the synchronous registerModelWithContext otherwise. The success callback guards against a switchBackend() racing ahead of a still-pending registration (same weak_ptr + liveness-token pattern installReconnectHandler already uses) so a stale reply can never overwrite the fresh id switchBackend's own re-registration already assigned. QtWebSocketBackend overrides registerModelAsync, matching execute()'s existing callId-based reply matching (the server already echoes callId on every reply kind, register included -- no protocol change needed). Gated by a new QtWebSocketBackendConfig::asyncRegistrationEnabled, defaulting to false: making the override unconditional broke the existing Qt test suite (measured directly -- 20 of 50 test cases failed) because any caller that fires an action immediately after constructing a BridgeHandler, assuming synchronous-enough-to-execute-next-line registration, now sees "handler not bound" instead. Defaulting the flag off preserves every existing embedder's behavior exactly; a WASM build opts in explicitly and adapts its call sites to wait for registration before firing an action. Scope: only the plain (non-shared) registration path -- a BridgeHandler's initial construction -- goes through registerModelAsync. registerModelShared/ attachModel (shared/keyed handlers) and the re-registration switchBackend()/ the reconnect handler perform after a backend swap remain synchronous; making those async too is a larger change to Bridge's locking model, left for a follow-up if it proves necessary. Closes #26 Signed-off-by: Yaraslau Tamashevich --- docs/spec/core/backend.md | 83 +++++- docs/spec/core/bridge.md | 11 +- include/morph/core/backend.hpp | 51 ++++ include/morph/core/bridge.hpp | 73 ++++- include/morph/qt/qt_websocket_backend.hpp | 82 +++++- src/qt/qt_websocket_backend.cpp | 108 ++++++-- tests/CMakeLists.txt | 1 + tests/qt/test_qt_websocket.cpp | 42 +++ tests/test_async_registration.cpp | 307 ++++++++++++++++++++++ 9 files changed, 710 insertions(+), 48 deletions(-) create mode 100644 tests/test_async_registration.cpp diff --git a/docs/spec/core/backend.md b/docs/spec/core/backend.md index 00837adf..d94dcf54 100644 --- a/docs/spec/core/backend.md +++ b/docs/spec/core/backend.md @@ -27,6 +27,7 @@ and react to backend changes. - [The dispatch struct — `ActionCall`](#the-dispatch-struct--actioncall) - [The abstract interface — `IBackend`](#the-abstract-interface--ibackend) +- [Asynchronous registration — `registerModelAsync`](#asynchronous-registration--registermodelasync) - [Error types](#error-types) - [`LocalBackend` — in-process execution](#localbackend--in-process-execution) - [`RemoteServer` — server-side message handler](#remoteserver--server-side-message-handler) @@ -68,12 +69,63 @@ holds a `unique_ptr` and delegates all model operations to it. |---|---| | `registerModel(typeId, factory)` | Registers a new model instance, returns its opaque `ModelId`. | | `registerModelWithContext(typeId, factory, contextKey)` | Same as `registerModel`, additionally passes a stable identity (e.g. account id). Default implementation drops `contextKey` and forwards to `registerModel` — correct for `LocalBackend` where the factory closure already captures identity. `SimulatedRemoteBackend` overrides to carry `contextKey` across the wire. | +| `registerModelAsync(typeId, factory, contextKey, onRegistered, onError)` | Optional non-blocking counterpart to `registerModelWithContext`. Returns `false` by default (no async path); `Bridge::registerHandler()` prefers this when it returns `true` and falls back to the synchronous call otherwise. See [Asynchronous registration](#asynchronous-registration--registermodelasync). | | `deregisterModel(mid)` | Removes the model identified by `mid`. | | `execute(mid, call, cbExec)` | Dispatches `call` against the model identified by `mid`. Returns a `Completion>`. | | `notifyBackendChanged()` | Called by `Bridge::switchBackend()` after all handlers are re-registered. | | `cancelPending(exc)` | Resolves every still-pending completion with `exc`. Called on the outgoing backend during `switchBackend()` and in `Bridge`'s destructor. After this call, any later `setValue`/`setException` on those states is a no-op. | | `setReconnectHandler(handler)` | Installs a callback invoked when the backend reconnects to its peer. Used by backends with transport (e.g. `QtWebSocketBackend`). Default implementation is a no-op. | +## Asynchronous registration — `registerModelAsync` + +`registerModel`/`registerModelWithContext` are synchronous: a backend whose +registration requires a round-trip can only implement that by blocking the +calling thread until the reply arrives — `QtWebSocketBackend` does this via a +nested `QEventLoop` in `sendSync`. On a WASM main thread, Qt refuses to spin a +nested loop at all (`WaitForMoreEvents is not supported on the main thread +without asyncify`), so that blocking call aborts the page — the very first +`registerModel` a WASM client makes. + +`IBackend::registerModelAsync(typeId, factory, contextKey, onRegistered, +onError)` is the optional non-blocking counterpart. A backend that offers one +sends the request and returns `true` immediately, then invokes exactly one of +`onRegistered(ModelId)` / `onError(message)` once the reply arrives, on the +backend's own thread — unless the backend is destroyed first, in which case +neither fires. The default implementation returns `false` without calling +either callback. + +`Bridge::registerHandler()` (both overloads — the default-factory template and +the pre-built-binding overload) prefers this path: it adds the binding to +`_handlers` and calls `registerModelAsync` *before* acquiring `Bridge::_mtx` +for the callback (a synchronous callback invocation would otherwise +self-deadlock re-acquiring the lock). If it returns `false`, `registerHandler` +falls back to the synchronous `registerModelWithContext`, exactly as before +this feature existed. If it returns `true`, the binding is returned **unbound** +(`currentId == 0`) — `executeVia` fails fast with "handler not bound" for any +call made before `onRegistered` fires, so a caller using the async path must +wait for registration (e.g. gate its UI on it) rather than fire an action +immediately after constructing the handler. + +**Staleness guard.** The success callback captures a `weak_ptr` +pinned to the backend the request was issued against, plus the Bridge's +`liveness()` token (the same pattern `installReconnectHandler` uses). Before +applying the received `ModelId`, it checks the liveness token (skip if the +Bridge is gone) and compares the pinned backend against `loadBackend()` (skip +if a `switchBackend()` already moved past this registration — that call's own +re-registration loop already gave the binding a fresh id on the new backend, +which a stale reply must not overwrite). + +**Scope.** Only the plain (non-shared) registration path uses this — a +`BridgeHandler`'s initial construction. `registerModelShared`/`attachModel` +(shared/keyed handlers) and the re-registration `switchBackend()`/the +reconnect handler perform after a backend swap remain synchronous; giving +those an async path too is a larger change to `Bridge`'s locking model, left +for a future issue if it proves necessary. + +`QtWebSocketBackend` is the one backend that currently overrides this, gated +by `QtWebSocketBackendConfig::asyncRegistrationEnabled` (default `false` — see +its own section below). + ## Error types Four exception types are thrown into in-flight `Completion`s: @@ -455,6 +507,11 @@ by `_pendingMtx`, because `cancelPending` can be called from `Bridge` / unblocks and reports failure instead of freezing the Qt thread forever; when the parked loop returns with an empty `_pendingReply`, `sendSync` throws `"disconnected"`. See concurrency_and_lifetimes.md. + + **`registerModelAsync` is the non-blocking alternative**, opt-in via + `QtWebSocketBackendConfig::asyncRegistrationEnabled` (default `false`, so + every existing embedder keeps `registerModel`'s synchronous behavior + unchanged). See [Asynchronous registration](#asynchronous-registration--registermodelasync). - `deregisterModel` — **fire-and-forget**, not synchronous: if `_connected`, it sends a `deregister` envelope and returns immediately without waiting for the ack; if disconnected, it does nothing. This deliberately avoids a nested @@ -472,16 +529,22 @@ by `_pendingMtx`, because `cancelPending` can be called from `Bridge` / **Reply framing / callId multiplexing.** `onTextMessage` decodes each incoming frame and routes it by `callId`: -- A **non-zero `callId`** is an async `execute` reply. The backend pops the - matching `PendingExecute` from `_pending`; `ok` → `deserialize(body)` into the - completion's value (deserialisation exceptions become the completion's error), - any other kind → `std::runtime_error(message)` into the completion's error. A - reply whose `callId` is **not** in `_pending` (e.g. a late reply for an +- A **non-zero `callId`** is checked against `_pending` first (an async + `execute` reply): the backend pops the matching `PendingExecute`; `ok` → + `deserialize(body)` into the completion's value (deserialisation exceptions + become the completion's error), any other kind → `std::runtime_error(message)` + into the completion's error. If not found there, `_pendingRegistrations` is + checked next (an async `registerModelAsync` reply — same `callId` + counter/namespace as `execute`, separate map because the reply shape differs): + `ok` → `onRegistered(ModelId{modelId})`, any other kind → `onError(message)`. + A `callId` matching **neither** map (e.g. a late reply for an already-cancelled call) is dropped silently. -- A **`callId == 0`** frame is a synchronous control reply (`register`); it is - stored in `_pendingReply` and quits the parked nested `QEventLoop`. A frame - that fails to decode is also routed to the parked sync waiter (as the raw - string) so the blocked `sendSync` unblocks with an error rather than hanging. +- A **`callId == 0`** frame is a synchronous control reply (`register` when + `asyncRegistrationEnabled` is `false`, or `attach`/`assign`/`instances`/ + `deregister`); it is stored in `_pendingReply` and quits the parked nested + `QEventLoop`. A frame that fails to decode is also routed to the parked sync + waiter (as the raw string) so the blocked `sendSync` unblocks with an error + rather than hanging. Because `execute` replies are matched on `callId`, concurrent in-flight execute calls are supported; `RemoteServer`/`QtWebSocketServer` echo the request `callId` @@ -969,12 +1032,14 @@ thread to marshal onto. | `initialReconnectDelay` | `std::chrono::milliseconds` | `500 ms` | | `maxReconnectDelay` | `std::chrono::milliseconds` | `30 s` | | `backoffMultiplier` | `double` | `2.0` | +| `asyncRegistrationEnabled` | `bool` | `false` — opts in to `registerModelAsync` (see [Asynchronous registration](#asynchronous-registration--registermodelasync)); `false` keeps every embedder on `registerModel`'s synchronous behavior. | ### `QtWebSocketBackend` (namespace `morph::qt`) | Method | Notes | |---|---| | `QtWebSocketBackend(serverUrl, dispatcher = defaultDispatcher(), registry = defaultRegistry(), tls = nullopt, cfg = Config{})` | Opens the socket to `serverUrl` in the constructor. `dispatcher`/`registry` params are accepted but unused (models live on the server). `tls` non-null → `wss://`. | +| `registerModelAsync(typeId, factory, contextKey, onRegistered, onError)` | Returns `false` immediately unless `cfg.asyncRegistrationEnabled` is `true`. Otherwise: assigns a fresh `callId` (the same counter `execute` uses), records the callbacks in `_pendingRegistrations[callId]`, sends `register` with that `callId`, and returns `true`. `onRegistered`/`onError` fire later from `onTextMessage` (or from `cancelPending` on a disconnect) — never synchronously from this call. | | `waitForConnected(timeoutMs = 5000)` | Pumps the Qt loop until connected or timeout; returns `_connected`. | | `negotiateProtocolVersion()` | Opt-in: sends `hello` synchronously (same nested-`QEventLoop` path as `registerModel`), classifies the reply via `wire::interpretHelloReply`. Throws on an explicit version rejection or a `sendSync` failure. | | `registerModel(typeId, factory)` | Synchronous via nested `QEventLoop`; `factory` ignored. Throws on `err` reply. | diff --git a/docs/spec/core/bridge.md b/docs/spec/core/bridge.md index 0850a25f..b2b25e31 100644 --- a/docs/spec/core/bridge.md +++ b/docs/spec/core/bridge.md @@ -86,7 +86,12 @@ re-register all live bindings on reconnection. `ModelFactory::create()` factory and registers it on the active backend. Returns the `shared_ptr`. An overload accepts a pre-built binding (for dependency injection, custom `contextKey`, or custom -factory captures). +factory captures). Both funnel through a shared `registerHandlerImpl`, which +prefers the backend's `IBackend::registerModelAsync` when it offers one (see +`backend.md`, "Asynchronous registration") and falls back to the synchronous +`registerModelWithContext` otherwise — the returned binding may therefore come +back **unbound** (`currentId == 0`) if the backend registered asynchronously +and the reply has not arrived yet. **`executeVia(binding, action, cbExec)`** dispatches one action. Takes a short snapshot of the backend `shared_ptr` under the dedicated @@ -468,8 +473,8 @@ make teardown order-independent.) |---|---|---| | ctor | `explicit Bridge(unique_ptr)` | Installs reconnect handler on the backend. | | dtor | `~Bridge()` | Clears the active backend's reconnect handler, then cancels all pending completions with `BridgeDestroyedError`. | -| `registerHandler` | `shared_ptr registerHandler()` | Default factory. | -| `registerHandler(binding)` | `void registerHandler(const shared_ptr&)` | Pre-built binding. | +| `registerHandler` | `shared_ptr registerHandler()` | Default factory. Prefers `IBackend::registerModelAsync`; see `backend.md`. | +| `registerHandler(binding)` | `void registerHandler(const shared_ptr&)` | Pre-built binding. Same async-preferring behavior. | | `switchBackend` | `void switchBackend(unique_ptr)` | Atomic: stages all re-registrations on the new backend, commits (publishes new ids + swaps) only if all succeed, else rolls back and rethrows leaving old backend + `currentId`s intact. Cancels old backend's pending ops with `BackendChangedError`. Holds both `_mtx` and `_attachMtx` for its duration. | | `deregisterHandler` | `void deregisterHandler(const shared_ptr&)` | Deregisters from active backend (if bound), resets `currentId` to 0, removes from tracking. | | `executeVia` | `Completion executeVia(const shared_ptr&, Action, IExecutor*)` | Lock-free dispatch. Attaches default session. On `LocalBackend`, rejects an action whose `ActionValidator::ready` returns `false` with `morph::model::ValidationError` via `onError`, before `Model::execute` runs. Records journal for loggable actions. Value-forwarding into the typed `Completion` is `try`/`catch`-guarded — a throwing result move/copy resolves the completion via `onError` instead of hanging or terminating. The bridge-touching side effects (`onResult`, `hasSubscribers()`/`publishResult`) are gated on the `_liveness` token, checked before either runs, so a completion resolving after `~Bridge()` skips them instead of touching the dangling `Bridge`. | diff --git a/include/morph/core/backend.hpp b/include/morph/core/backend.hpp index 39892f82..8b9d5ca5 100644 --- a/include/morph/core/backend.hpp +++ b/include/morph/core/backend.hpp @@ -104,6 +104,57 @@ struct IBackend { return registerModel(typeId, std::move(factory)); } + /// @brief Optional non-blocking counterpart to `registerModelWithContext`. + /// + /// `registerModelWithContext`/`registerModel` are synchronous: a backend + /// whose registration requires a round-trip (a socket backend) can only + /// implement that by blocking the calling thread until the reply arrives — + /// `QtWebSocketBackend` does this via a nested `QEventLoop`. On a WASM main + /// thread, Qt refuses to spin a nested loop at all + /// (`WaitForMoreEvents is not supported on the main thread without asyncify`), + /// so that blocking call aborts the page — the very first `registerModel` + /// a WASM client makes. + /// + /// Overriding this lets such a backend register without blocking: send the + /// request and return `true` immediately, then invoke exactly one of + /// @p onRegistered / @p onError once the reply arrives, on the backend's own + /// thread (unless the backend is destroyed first, in which case neither + /// fires). `Bridge::registerHandler()` prefers this path when it is + /// available and falls back to the synchronous `registerModelWithContext` + /// otherwise, so every backend that has not opted in (every backend as of + /// this writing, other than `QtWebSocketBackend`) is unaffected. + /// + /// The default implementation offers no async path and returns `false` + /// without calling either callback — the caller (`Bridge::registerHandler`) + /// falls back to `registerModelWithContext` in that case, matching every + /// caller's behavior before this method existed. + /// + /// @note Scope: only `Bridge::registerHandler()`'s plain (non-shared) + /// registration path — a `BridgeHandler`'s initial construction — + /// uses this. Shared/keyed registration (`registerModelShared`, + /// `attachModel`) and the re-registration `switchBackend()`/the + /// reconnect handler perform after a backend swap remain + /// synchronous; see docs/spec/core/backend.md. + /// @param typeId String type-id of the model to instantiate. + /// @param factory Callable that constructs the `IModelHolder` (local path only). + /// @param contextKey Stable identity of the new instance; empty if none. + /// @param onRegistered Invoked with the assigned `ModelId` on success. + /// @param onError Invoked with a diagnostic message on failure. + /// @return `true` if this backend accepted the request and will invoke + /// exactly one callback later; `false` if it has no async path + /// (neither callback is invoked in that case). + virtual bool registerModelAsync( + const std::string& typeId, std::function()> factory, + std::string_view contextKey, std::function onRegistered, + std::function onError) { + (void)typeId; + (void)factory; + (void)contextKey; + (void)onRegistered; + (void)onError; + return false; + } + /// @brief Registers or attaches to the shared instance holding @p primary. /// /// A *register-or-attach*: if an instance for `(typeId, primary)` is already diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index 988f84da..c6304bd6 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -228,10 +228,7 @@ class Bridge { auto binding = std::make_shared(); binding->typeId = std::string{::morph::model::ModelTraits::typeId()}; binding->modelFactory = [] { return ::morph::model::detail::ModelFactory::create(); }; - std::scoped_lock const lock{_mtx}; - binding->currentId.store( - loadBackend()->registerModelWithContext(binding->typeId, binding->modelFactory, binding->contextKey).v); - _handlers.push_back(binding); + registerHandlerImpl(binding); return binding; } @@ -241,12 +238,7 @@ class Bridge { /// dependencies that the type-erasing default factory cannot carry, or when /// `binding->contextKey` needs to be set before registration (see `HandlerBinding::contextKey`). /// @param binding Pre-constructed binding. Its `typeId` and `modelFactory` must be set. - void registerHandler(const std::shared_ptr& binding) { - std::scoped_lock const lock{_mtx}; - binding->currentId.store( - loadBackend()->registerModelWithContext(binding->typeId, binding->modelFactory, binding->contextKey).v); - _handlers.push_back(binding); - } + void registerHandler(const std::shared_ptr& binding) { registerHandlerImpl(binding); } /// @brief Creates a shared, initially **unattached** binding for `Model`. /// @@ -793,6 +785,67 @@ class Bridge { return _backend; } + /// @brief Shared body of both `registerHandler()` overloads: prefers the + /// backend's `registerModelAsync` path (see `IBackend::registerModelAsync`'s + /// doc comment for why — avoiding a nested-event-loop block that + /// aborts a WASM main thread) and falls back to the synchronous + /// `registerModelWithContext` when the backend offers no async path. + /// + /// @p binding is added to `_handlers` *before* the backend call — not + /// after, as the synchronous fallback below does internally — so a + /// concurrently-running `switchBackend()`/reconnect can already see and + /// re-register it even while this registration is still in flight (see + /// the async branch's comment for why that race is harmless). This also + /// means the backend call must not run under `_mtx`: a backend that (unlike + /// every backend documented here) invoked `onRegistered`/`onError` + /// synchronously from inside `registerModelAsync` would otherwise + /// self-deadlock re-acquiring `_mtx` in the callback below. + /// @param binding Binding to register; its `typeId`/`modelFactory`/`contextKey` must be set. + void registerHandlerImpl(const std::shared_ptr& binding) { + auto backend = loadBackend(); + { + std::scoped_lock const lock{_mtx}; + _handlers.push_back(binding); + } + + std::weak_ptr<::morph::backend::detail::IBackend> const weakBackend{backend}; + std::weak_ptr const weakLiveness{_liveness}; + std::weak_ptr const weakBinding{binding}; + bool const started = backend->registerModelAsync( + binding->typeId, binding->modelFactory, binding->contextKey, + [this, weakBackend, weakLiveness, weakBinding](::morph::exec::detail::ModelId newId) { + auto aliveToken = weakLiveness.lock(); + if (!aliveToken) { + return; // The Bridge is gone; do not touch `this`. + } + auto strongBinding = weakBinding.lock(); + if (!strongBinding) { + return; // The BridgeHandler (and its binding) is gone. + } + std::scoped_lock const lock{_mtx}; + auto pinned = weakBackend.lock(); + if (!pinned || pinned != loadBackend()) { + // A switchBackend() already moved past this registration + // (see this backend's own doc comment on the class) and + // its own re-registration loop already gave `binding` a + // fresh id on the *new* backend -- applying this stale + // one now would overwrite that with a dangling id from a + // backend nothing uses any more. + return; + } + strongBinding->currentId.store(newId.v); + }, + [typeId = binding->typeId](const std::string& message) { + ::morph::log::logError("[registerHandler] async registration of '" + typeId + + "' failed: " + message); + }); + + if (!started) { + binding->currentId.store( + backend->registerModelWithContext(binding->typeId, binding->modelFactory, binding->contextKey).v); + } + } + std::shared_ptr<::morph::backend::detail::IBackend> exchangeBackend( std::shared_ptr<::morph::backend::detail::IBackend> next) { std::scoped_lock const lock{_backendMtx}; diff --git a/include/morph/qt/qt_websocket_backend.hpp b/include/morph/qt/qt_websocket_backend.hpp index 3ce9a95e..09d36ed6 100644 --- a/include/morph/qt/qt_websocket_backend.hpp +++ b/include/morph/qt/qt_websocket_backend.hpp @@ -35,15 +35,39 @@ struct QtWebSocketBackendConfig { /// @brief Multiplier applied to the delay after each failed attempt. double backoffMultiplier = 2.0; + + /// @brief Opt in to `registerModelAsync` (see its doc comment on `IBackend`). + /// + /// Defaults to `false`: `Bridge::registerHandler()` then falls back to the + /// synchronous `registerModel`, exactly as before this feature existed — + /// every existing embedder (a desktop Qt client, this backend's own test + /// suite) keeps registering synchronously, immediately usable the line + /// after `BridgeHandler`'s constructor returns. + /// + /// Set `true` only for a build where that synchronous guarantee cannot + /// hold at all — a WASM main thread, where the nested `QEventLoop` + /// `registerModel` relies on aborts the page outright. Doing so is a + /// deliberate trade: the caller must then wait for registration to + /// complete (e.g. gate the UI on it) before firing an action through that + /// handler, since `executeVia` fails fast with "handler not bound" for an + /// unbound binding rather than queuing or blocking. + bool asyncRegistrationEnabled = false; }; /// @brief `IBackend` implementation that communicates with a `RemoteServer` over WebSocket. /// /// `registerModel()` is synchronous (blocks the calling thread via a nested -/// `QEventLoop` until the server replies). `deregisterModel()` is fire-and-forget -/// (it sends the message without waiting, avoiding a nested event loop during -/// destruction). `execute()` is asynchronous: it assigns a call-id, sends the -/// message, and resolves the returned `Completion` when the matching reply arrives. +/// `QEventLoop` until the server replies) -- unusable on a WASM main thread, +/// which Qt refuses to spin a nested loop on at all. `registerModelAsync()` +/// is the non-blocking alternative `Bridge::registerHandler()` prefers when +/// available (see `IBackend::registerModelAsync`'s doc comment): it assigns a +/// call-id, sends the message, returns immediately, and invokes exactly one +/// of its `onRegistered`/`onError` callbacks once the matching reply arrives +/// -- the same call-id-matching mechanism `execute()` already uses. +/// `deregisterModel()` is fire-and-forget (it sends the message without +/// waiting, avoiding a nested event loop during destruction). `execute()` is +/// asynchronous: it assigns a call-id, sends the message, and resolves the +/// returned `Completion` when the matching reply arrives. /// /// @par TLS /// Pass a `QSslConfiguration` to enable `wss://`. Build it with `tlsVerifyingConfig()` @@ -107,6 +131,34 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { const std::string& typeId, std::function()> factory) override; + /// @brief Sends a `register` message and returns without blocking; the + /// reply is matched later, asynchronously, by `callId`. + /// + /// The non-blocking counterpart to `registerModel`/`registerModelWithContext` + /// (see `IBackend::registerModelAsync`'s doc comment for why this exists): + /// `registerModel` blocks the calling thread in a nested `QEventLoop` via + /// `sendSync`, which a WASM main thread cannot do at all. This instead + /// assigns a fresh `callId` (the same counter `execute()` uses), sends the + /// `register` envelope, and returns `true` immediately; the reply is + /// matched via `_pendingRegistrations` when `onTextMessage` sees it (no + /// protocol change needed — the server already echoes `callId` on every + /// reply, `register` included). Exactly one of @p onRegistered / @p onError + /// fires, on the Qt event loop thread, once the reply arrives — or never, + /// if the socket disconnects first without ever reconnecting and + /// `cancelPending` is never called again for this id (a disconnect + /// *before* a reconnect calls `cancelPending`, which does invoke @p onError + /// — see `cancelPending`'s doc comment). + /// + /// @param typeId String type-id of the model to instantiate. + /// @param contextKey Stable identity of the new instance; travels in the wire envelope. + /// @param onRegistered Invoked with the server-assigned `ModelId` on success. + /// @param onError Invoked with a diagnostic message on failure or disconnect. + /// @return `true` always — this backend has an async path (`false` is never returned). + bool registerModelAsync(const std::string& typeId, + std::function()> factory, + std::string_view contextKey, std::function onRegistered, + std::function onError) override; + /// @brief Sends a shared (register-or-attach) `register` and blocks for the reply. /// /// An empty primary degrades to the private path. @@ -169,13 +221,16 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { /// @brief No-op — this backend holds no local model objects. void notifyBackendChanged() override {} - /// @brief Resolves every pending execute call's `Completion` with @p exc. + /// @brief Resolves every pending execute call's `Completion` with @p exc, + /// and fails every pending async registration's `onError`. /// /// Called by `Bridge::switchBackend()` on the outgoing backend, by `~Bridge`, /// and internally when the socket disconnects. Late replies arriving for - /// already-cancelled call ids are dropped silently. + /// already-cancelled call ids (execute or register) are dropped silently. /// - /// @param exc Exception delivered to every pending completion's error sink. + /// @param exc Exception delivered to every pending completion's error sink; + /// `exc.what()`-equivalent text is delivered to every pending + /// `registerModelAsync` call's `onError`. void cancelPending(const std::exception_ptr& exc) override; /// @brief Installs the handler `Bridge` uses to re-register handlers after a reconnect. @@ -218,6 +273,19 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { uint64_t _nextCallId{0}; std::unordered_map _pending; std::mutex _pendingMtx; + + /// @brief One in-flight `registerModelAsync` call, keyed by `callId`. + /// + /// Kept separate from `PendingExecute`/`_pending` (a different `callId` + /// namespace would be a protocol change; this shares the same namespace + /// and counter, just a different local map) because a register reply's + /// shape (`modelId`, no `deserialize` step) differs from an execute + /// reply's. + struct PendingRegistration { + std::function onRegistered; + std::function onError; + }; + std::unordered_map _pendingRegistrations; }; } // namespace morph::qt diff --git a/src/qt/qt_websocket_backend.cpp b/src/qt/qt_websocket_backend.cpp index 7a163a90..d74d8074 100644 --- a/src/qt/qt_websocket_backend.cpp +++ b/src/qt/qt_websocket_backend.cpp @@ -132,6 +132,32 @@ ::morph::exec::detail::ModelId QtWebSocketBackend::registerModel( throw std::runtime_error("register failed: " + reply.message); } +bool QtWebSocketBackend::registerModelAsync( + const std::string& typeId, std::function()> /*factory*/, + std::string_view contextKey, std::function onRegistered, + std::function onError) { + if (!_cfg.asyncRegistrationEnabled) { + // Opt-in only (see QtWebSocketBackendConfig::asyncRegistrationEnabled): + // returning false here makes Bridge::registerHandler() fall back to + // the synchronous registerModel(), preserving every existing + // embedder's behavior unless it explicitly asks for the async path. + return false; + } + if (!_connected) { + onError("disconnected"); + return true; + } + uint64_t const callId = ++_nextCallId; + { + std::scoped_lock const lock{_pendingMtx}; + _pendingRegistrations[callId] = PendingRegistration{std::move(onRegistered), std::move(onError)}; + } + auto env = ::morph::wire::makeRegister(typeId, std::string{contextKey}); + env.callId = callId; + _socket.sendTextMessage(QString::fromStdString(::morph::wire::encode(env))); + return true; +} + ::morph::wire::ProtocolNegotiationResult QtWebSocketBackend::negotiateProtocolVersion() { std::string replyJson; try { @@ -245,16 +271,31 @@ ::morph::async::Completion> QtWebSocketBackend::execute( } void QtWebSocketBackend::cancelPending(const std::exception_ptr& exc) { - std::unordered_map drained; + std::unordered_map drainedExecutes; + std::unordered_map drainedRegistrations; { std::scoped_lock lock{_pendingMtx}; - drained.swap(_pending); + drainedExecutes.swap(_pending); + drainedRegistrations.swap(_pendingRegistrations); } - for (auto& [_, pending] : drained) { + for (auto& [_, pending] : drainedExecutes) { if (pending.state) { pending.state->setException(exc); } } + std::string message = "disconnected"; + try { + std::rethrow_exception(exc); + } catch (const std::exception& concrete) { + message = concrete.what(); + } catch (...) { + // Non-std::exception thrown in: keep the "disconnected" fallback above. + } + for (auto& [_, pending] : drainedRegistrations) { + if (pending.onError) { + pending.onError(message); + } + } } void QtWebSocketBackend::setReconnectHandler(const std::function& handler) { _reconnectHandler = handler; } @@ -304,30 +345,59 @@ void QtWebSocketBackend::onTextMessage(const QString& message) { return; } - // Async execute replies carry a non-zero callId; sync replies (register / - // deregister) carry callId == 0 and resume the parked nested event loop. + // Async execute replies and async registerModelAsync replies both carry a + // non-zero callId (the two share one counter/namespace, but land in + // separate maps below since their reply shapes differ); sync replies + // (registerModel/deregister/etc's sendSync calls) carry callId == 0 and + // resume the parked nested event loop. if (env.callId != 0U) { - PendingExecute pending; + PendingExecute execPending; + bool foundExecute = false; { std::scoped_lock lock{_pendingMtx}; auto iter = _pending.find(env.callId); - if (iter == _pending.end()) { - return; + if (iter != _pending.end()) { + execPending = std::move(iter->second); + _pending.erase(iter); + foundExecute = true; + } + } + if (foundExecute) { + if (env.kind == "ok") { + try { + execPending.state->setValue(execPending.deserialize(env.body)); + } catch (...) { + execPending.state->setException(std::current_exception()); + } + } else if (env.message == "timeout") { + execPending.state->setException(std::make_exception_ptr(::morph::backend::TimeoutError{})); + } else { + execPending.state->setException(std::make_exception_ptr(std::runtime_error(env.message))); + } + return; + } + + PendingRegistration regPending; + bool foundRegistration = false; + { + std::scoped_lock lock{_pendingMtx}; + auto iter = _pendingRegistrations.find(env.callId); + if (iter != _pendingRegistrations.end()) { + regPending = std::move(iter->second); + _pendingRegistrations.erase(iter); + foundRegistration = true; } - pending = std::move(iter->second); - _pending.erase(iter); } - if (env.kind == "ok") { - try { - pending.state->setValue(pending.deserialize(env.body)); - } catch (...) { - pending.state->setException(std::current_exception()); + if (foundRegistration) { + if (env.kind == "ok") { + regPending.onRegistered(::morph::exec::detail::ModelId{env.modelId}); + } else { + regPending.onError(env.message); } - } else if (env.message == "timeout") { - pending.state->setException(std::make_exception_ptr(::morph::backend::TimeoutError{})); - } else { - pending.state->setException(std::make_exception_ptr(std::runtime_error(env.message))); } + // Neither map matched: an already-resolved or already-cancelled + // callId's late reply, dropped silently -- same as before this + // feature for an execute reply. return; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6ccaa781..ec2a833f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -29,6 +29,7 @@ add_executable(morph_tests test_dispatch_di.cpp test_handler_binding.cpp test_switch_backend.cpp + test_async_registration.cpp test_network_monitor.cpp test_offline_queue.cpp test_file_offline_queue.cpp diff --git a/tests/qt/test_qt_websocket.cpp b/tests/qt/test_qt_websocket.cpp index 951083b7..57ab8bc2 100644 --- a/tests/qt/test_qt_websocket.cpp +++ b/tests/qt/test_qt_websocket.cpp @@ -161,6 +161,48 @@ TEST_CASE("morph::qt::QtWebSocketBackend: action result delivered via then", "[q REQUIRE(result.load() == 99); } +TEST_CASE( + "morph::qt::QtWebSocketBackend: registerModelAsync (opt-in via Config::asyncRegistrationEnabled) registers " + "without blocking", + "[qt][ws][issue26]") { + ensureApp(); + morph::exec::ThreadPoolExecutor serverPool{2}; + auto server = std::make_shared(serverPool); + morph::qt::QtWebSocketServer wsServer{*server, 0}; + REQUIRE(wsServer.listen()); + + QUrl url{QString("ws://127.0.0.1:%1").arg(wsServer.port())}; + auto backendPtr = std::make_unique( + url, morph::model::detail::defaultDispatcher(), morph::model::detail::defaultRegistry(), std::nullopt, + morph::qt::QtWebSocketBackend::Config{.asyncRegistrationEnabled = true}); + REQUIRE(backendPtr->waitForConnected()); + + morph::qt::QtExecutor qtExec; + morph::bridge::Bridge bridge{std::move(backendPtr)}; + + auto binding = std::make_shared(); + binding->typeId = "WsEchoModel"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + bridge.registerHandler(binding); + + // Registration does not block: registerHandler() already returned above, + // yet the binding is still unbound -- this is the whole point of the + // async path (see IBackend::registerModelAsync's doc comment). A real + // WASM caller would gate its UI on this instead of firing an action + // immediately, since executeVia fails fast on an unbound binding. + CHECK(binding->currentId.load() == 0U); + + pumpUntil([&] { return binding->currentId.load() != 0U; }); + REQUIRE(binding->currentId.load() != 0U); + + morph::bridge::BridgeHandler handler{bridge, &qtExec, binding}; + std::atomic result{-1}; + handler.execute(WsEchoAction{99}).then([&](int val) { result.store(val); }).onError([](const std::exception_ptr&) { + }); + pumpUntil([&] { return result.load() != -1; }); + REQUIRE(result.load() == 99); +} + TEST_CASE("morph::qt::QtWebSocketBackend: exception delivered via onError", "[qt][ws]") { ensureApp(); morph::exec::ThreadPoolExecutor serverPool{2}; diff --git a/tests/test_async_registration.cpp b/tests/test_async_registration.cpp new file mode 100644 index 00000000..05a4d9c8 --- /dev/null +++ b/tests/test_async_registration.cpp @@ -0,0 +1,307 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Coverage for issue #26: Bridge::registerHandler() prefers +// IBackend::registerModelAsync when a backend offers one, falling back to the +// synchronous registerModelWithContext otherwise. AsyncRegisterBackend below +// is a minimal test double whose registerModelAsync defers its reply until +// the test explicitly completes it -- simulating a socket backend whose reply +// arrives later on its own thread (what QtWebSocketBackend's async path does +// against a real server), instead of blocking the calling thread via a nested +// event loop (what registerModel does today -- the pattern this issue is +// about, since Qt refuses to spin a nested loop on a WASM main thread at all). + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "test_support.hpp" + +namespace { + +struct ARCount { + int x = 0; +}; + +struct ARModel { + int execute(const ARCount& a) { return a.x; } +}; + +} // namespace + +template <> +struct morph::model::ActionTraits { + using Result = int; + static constexpr std::string_view typeId() { return "AR_Count"; } + static std::string toJson(const ARCount& a) { return R"({"x":)" + std::to_string(a.x) + "}"; } + static ARCount fromJson(std::string_view) { return {}; } + static std::string resultToJson(const int& r) { return std::to_string(r); } + static int resultFromJson(std::string_view s) { return std::stoi(std::string{s}); } +}; +template <> +struct morph::model::ModelTraits { + static constexpr std::string_view typeId() { return "AR_Model"; } +}; + +namespace { + +// Offers an async registration path that does not complete until the test +// calls completeNext()/failNext() -- simulating a backend whose registration +// reply arrives later, asynchronously, instead of blocking the caller. +class AsyncRegisterBackend : public morph::backend::detail::IBackend { +public: + morph::exec::detail::ModelId registerModel( + const std::string&, std::function()> factory) override { + std::scoped_lock const lock{_regMtx}; + auto holder = factory(); + auto const mid = morph::exec::detail::ModelId{_nextId++}; + _models[mid.v] = std::move(holder); + return mid; + } + void deregisterModel(morph::exec::detail::ModelId mid) override { + std::scoped_lock const lock{_regMtx}; + _models.erase(mid.v); + } + morph::async::Completion> execute(morph::exec::detail::ModelId mid, + morph::backend::detail::ActionCall call, + morph::exec::IExecutor* cbExec) override { + auto state = std::make_shared>>(); + morph::async::Completion> comp{state, cbExec}; + std::scoped_lock const lock{_regMtx}; + auto iter = _models.find(mid.v); + if (iter == _models.end()) { + state->setException(std::make_exception_ptr(std::runtime_error("no such model"))); + return comp; + } + state->setValue(call.localOp(*iter->second)); + return comp; + } + void notifyBackendChanged() override {} + void cancelPending(const std::exception_ptr&) override {} + void setReconnectHandler(const std::function&) override {} + + bool registerModelAsync(const std::string& typeId, + std::function()> factory, + std::string_view /*contextKey*/, + std::function onRegistered, + std::function onError) override { + std::scoped_lock const lock{_pendingMtx}; + _pending.push_back(Pending{typeId, std::move(factory), std::move(onRegistered), std::move(onError)}); + return true; + } + + // Test hooks: settle the oldest still-pending async registration. + void completeNext() { + Pending pending; + { + std::scoped_lock const lock{_pendingMtx}; + REQUIRE_FALSE(_pending.empty()); + pending = std::move(_pending.front()); + _pending.erase(_pending.begin()); + } + auto mid = registerModel(pending.typeId, pending.factory); + pending.onRegistered(mid); + } + void failNext(const std::string& message) { + Pending pending; + { + std::scoped_lock const lock{_pendingMtx}; + REQUIRE_FALSE(_pending.empty()); + pending = std::move(_pending.front()); + _pending.erase(_pending.begin()); + } + pending.onError(message); + } + [[nodiscard]] std::size_t pendingCount() const { + std::scoped_lock const lock{_pendingMtx}; + return _pending.size(); + } + +private: + struct Pending { + std::string typeId; + std::function()> factory; + std::function onRegistered; + std::function onError; + }; + mutable std::mutex _pendingMtx; + std::vector _pending; + + mutable std::mutex _regMtx; + std::unordered_map> _models; + uint64_t _nextId{100}; +}; + +// Shim so a Bridge (which takes ownership of a unique_ptr) can hold a backend +// the test also keeps a shared_ptr to -- making it co-owned / able to outlive +// the Bridge (see test_bridge_lifetime.cpp's identical BackendShim). Also lets +// a still-async-capable backend be installed via the unique_ptr-only +// switchBackend() overload the codebase currently has. +class AsyncBackendShim : public morph::backend::detail::IBackend { +public: + explicit AsyncBackendShim(std::shared_ptr target) : _target{std::move(target)} {} + morph::exec::detail::ModelId registerModel( + const std::string& typeId, + std::function()> factory) override { + return _target->registerModel(typeId, std::move(factory)); + } + void deregisterModel(morph::exec::detail::ModelId mid) override { _target->deregisterModel(mid); } + morph::async::Completion> execute(morph::exec::detail::ModelId mid, + morph::backend::detail::ActionCall call, + morph::exec::IExecutor* cbExec) override { + return _target->execute(mid, std::move(call), cbExec); + } + void notifyBackendChanged() override { _target->notifyBackendChanged(); } + void cancelPending(const std::exception_ptr& exc) override { _target->cancelPending(exc); } + void setReconnectHandler(const std::function& handler) override { _target->setReconnectHandler(handler); } + bool registerModelAsync(const std::string& typeId, + std::function()> factory, + std::string_view contextKey, std::function onRegistered, + std::function onError) override { + return _target->registerModelAsync(typeId, std::move(factory), contextKey, std::move(onRegistered), + std::move(onError)); + } + +private: + std::shared_ptr _target; +}; + +} // namespace + +using SyncExec = morph::testing::InlineExecutor; + +TEST_CASE("Bridge::registerHandler: uses the async path when the backend offers one; binding starts unbound", + "[bridge][registration][issue26]") { + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + + auto binding = std::make_shared(); + binding->typeId = "AR_Model"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + bridge.registerHandler(binding); + + // Still unbound: the async reply has not arrived yet -- proves registerHandler + // did not block waiting for it (the whole point of this feature). + CHECK(binding->currentId.load() == 0U); + REQUIRE(rawBackend->pendingCount() == 1); + + rawBackend->completeNext(); + CHECK(binding->currentId.load() != 0U); +} + +TEST_CASE("Bridge::registerHandler: async execute works once the deferred registration completes", + "[bridge][registration][issue26]") { + SyncExec cbExec; + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + REQUIRE(rawBackend->pendingCount() == 1); + rawBackend->completeNext(); + + std::atomic result{-1}; + handler.execute(ARCount{.x = 7}).then([&](int v) { result.store(v); }).onError([](const std::exception_ptr&) {}); + REQUIRE(morph::testing::waitUntil([&] { return result.load() != -1; })); + CHECK(result.load() == 7); +} + +TEST_CASE("Bridge::registerHandler: onError leaves the binding unbound (no crash, logged)", + "[bridge][registration][issue26]") { + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + + auto binding = std::make_shared(); + binding->typeId = "AR_Model"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + bridge.registerHandler(binding); + + REQUIRE(rawBackend->pendingCount() == 1); + rawBackend->failNext("simulated registration failure"); + CHECK(binding->currentId.load() == 0U); +} + +TEST_CASE("Bridge::registerHandler: a stale async reply after switchBackend() does not clobber the new id", + "[bridge][registration][issue26]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + + auto asyncBackendA = std::make_shared(); + bridge.switchBackend(std::make_unique(asyncBackendA)); + + auto binding = std::make_shared(); + binding->typeId = "AR_Model"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + bridge.registerHandler(binding); + REQUIRE(asyncBackendA->pendingCount() == 1); + CHECK(binding->currentId.load() == 0U); + + // Switch to a second backend WHILE the registration on asyncBackendA is + // still pending. switchBackend's re-registration loop doesn't consult + // currentId -- it re-registers every tracked binding unconditionally -- + // so this binding gets a real id on the new backend synchronously, right + // here, with the original async registration still outstanding. + morph::exec::ThreadPoolExecutor pool2{2}; + bridge.switchBackend(std::make_unique(pool2)); + auto const idAfterSwitch = binding->currentId.load(); + CHECK(idAfterSwitch != 0U); + + // The original (now-stale) async reply from asyncBackendA finally + // arrives. It must be ignored, not overwrite the id switchBackend already + // assigned on the new, active backend. + asyncBackendA->completeNext(); + CHECK(binding->currentId.load() == idAfterSwitch); +} + +TEST_CASE("Bridge::registerHandler: an async reply arriving after ~Bridge() is a safe no-op", + "[bridge][registration][issue26]") { + morph::exec::ThreadPoolExecutor pool{2}; + auto bridge = std::make_unique(std::make_unique(pool)); + + auto asyncBackend = std::make_shared(); + bridge->switchBackend(std::make_unique(asyncBackend)); // co-owned: outlives the Bridge below + + auto binding = std::make_shared(); + binding->typeId = "AR_Model"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + bridge->registerHandler(binding); + REQUIRE(asyncBackend->pendingCount() == 1); + + bridge.reset(); // ~Bridge() runs; asyncBackend and binding both outlive it. + + // Must not crash or touch the dangling Bridge; the liveness guard makes + // this a no-op, so the binding -- which the test still holds -- stays + // exactly as it was at destruction (unbound). + REQUIRE_NOTHROW(asyncBackend->completeNext()); + CHECK(binding->currentId.load() == 0U); +} + +TEST_CASE("Bridge::registerHandler: falls back to the synchronous path for a backend with no async support", + "[bridge][registration][issue26]") { + // LocalBackend does not override registerModelAsync, so the default + // (returns false) applies and registerHandler falls back to + // registerModelWithContext -- binding is bound immediately, exactly as + // before this feature existed. + morph::exec::ThreadPoolExecutor pool{2}; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + + auto binding = std::make_shared(); + binding->typeId = "AR_Model"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + bridge.registerHandler(binding); + + CHECK(binding->currentId.load() != 0U); +} From d448c93adad34360e249a0755f301bf01fe66ad7 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 3 Aug 2026 21:54:07 +0300 Subject: [PATCH 2/2] qt: document registerModelAsync's unused factory parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doxygen's WARN_AS_ERROR build fails on any undocumented parameter; QtWebSocketBackend::registerModelAsync's factory param (unused — this backend has no local model to construct) was missing its @param tag. Signed-off-by: Yaraslau Tamashevich --- include/morph/qt/qt_websocket_backend.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/morph/qt/qt_websocket_backend.hpp b/include/morph/qt/qt_websocket_backend.hpp index 09d36ed6..0d9e4c29 100644 --- a/include/morph/qt/qt_websocket_backend.hpp +++ b/include/morph/qt/qt_websocket_backend.hpp @@ -150,6 +150,8 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { /// — see `cancelPending`'s doc comment). /// /// @param typeId String type-id of the model to instantiate. + /// @param factory Unused — this backend holds no local model to construct; + /// the server instantiates the model from `typeId`. /// @param contextKey Stable identity of the new instance; travels in the wire envelope. /// @param onRegistered Invoked with the server-assigned `ModelId` on success. /// @param onError Invoked with a diagnostic message on failure or disconnect.