diff --git a/CMakeLists.txt b/CMakeLists.txt index 958f841c..1fc6cc16 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,6 +10,18 @@ set(CMAKE_CXX_STANDARD 23) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) +# Global, build-wide: the Lightweight ORM (fetched into examples/common's +# ladder testkit and examples/bank) includes from SqlStatement.hpp +# with no NOMINMAX/WIN32_LEAN_AND_MEAN guard of its own, so its `min`/`max` +# macros leak into every translation unit that (transitively) includes it and +# break any later std::min/std::max/std::numeric_limits::min() call in the +# same TU (e.g. examples/common/clock.hpp). Defined here, before any +# subdirectory is added, so it reaches every target that could end up in such +# a TU, not just the ones that call apply_warnings(). +if(WIN32) + add_compile_definitions(NOMINMAX WIN32_LEAN_AND_MEAN) +endif() + option(MORPH_BUILD_TESTS "Build tests" ON) option(MORPH_BUILD_EXAMPLES "Build examples" ON) option(MORPH_BUILD_BANK_EXAMPLE "Build the SQLite/Lightweight bank example (heavy deps)" OFF) @@ -160,6 +172,7 @@ target_sources(morph include/morph/core/executor.hpp include/morph/core/strand.hpp include/morph/core/completion.hpp + include/morph/core/timeout_scheduler.hpp include/morph/core/model.hpp include/morph/core/registry.hpp include/morph/core/backend.hpp diff --git a/cmake/compiler_options.cmake b/cmake/compiler_options.cmake index d9f9b1c8..a8a9de7e 100644 --- a/cmake/compiler_options.cmake +++ b/cmake/compiler_options.cmake @@ -1,3 +1,22 @@ +include(CheckCXXCompilerFlag) + +# Probed once per configure (CMake caches each check_cxx_compiler_flag() result +# in CMakeCache.txt keyed by the result variable, regardless of how many +# targets call apply_warnings()) rather than unconditionally listed below: +# these three are recent enough additions to Clang's -Weverything set that +# Emscripten's bundled clang (pinned to an older release than the Linux/ +# Windows Clang this project otherwise builds with — see +# .github/workflows/wasm-ladder.yml's EMSDK_VERSION) rejects them outright +# under -Werror with "unknown warning option", turning a *suppression* flag +# into the very error it exists to silence. A version-number cutoff would be +# equally correct but more fragile (would need updating every time either +# toolchain's version changes); probing the actual compiler is the standard, +# self-maintaining way to make -Weverything portable across Clang releases. +check_cxx_compiler_flag(-Wno-nrvo MORPH_CLANG_HAS_WNO_NRVO) +check_cxx_compiler_flag(-Wno-unsafe-buffer-usage-in-libc-call MORPH_CLANG_HAS_WNO_UNSAFE_BUFFER_USAGE_IN_LIBC_CALL) +check_cxx_compiler_flag(-Wno-c2y-extensions MORPH_CLANG_HAS_WNO_C2Y_EXTENSIONS) +check_cxx_compiler_flag(-Wno-missing-designated-field-initializers MORPH_CLANG_HAS_WNO_MISSING_DESIGNATED_FIELD_INITIALIZERS) + function(apply_warnings target) target_compile_options(${target} PRIVATE # ── MSVC ────────────────────────────────────────────────────────────── @@ -35,6 +54,14 @@ function(apply_warnings target) $<$: -Wall -Wextra + # GCC's -Wextra implies -Wmissing-field-initializers, which (unlike + # Clang's narrower -Wmissing-designated-field-initializers, already + # suppressed above for the identical reason) fires on every field a + # designated initializer leaves unset -- flagging the same + # deliberately-partial DTO/config-style construction + # (morph::session::Context{.principal = ...} and its many + # siblings) as a defect, one diagnostic per omitted field. + -Wno-missing-field-initializers -Wpedantic -Wshadow -Wnon-virtual-dtor @@ -66,24 +93,51 @@ function(apply_warnings target) -Wno-pre-c++17-compat-pedantic -Wno-pre-c++20-compat -Wno-pre-c++20-compat-pedantic + # -Wc++20-compat is the sibling of -Wpre-c++20-compat for a + # narrower set of syntax (consteval, implicit `typename` in alias + # templates) that only some Clang builds separate out from the + # pre-c++20-compat umbrella above — same "we target C++23" + # rationale, added once the WASM leg's Emscripten-bundled clang + # (older than the Linux/Windows clang this project otherwise + # builds with) was the first to actually split it out and fire it + # on model_key.hpp/quantity.hpp/forms.hpp/bridge.hpp. + -Wno-c++20-compat # (b) Inherent to a header-only, templated library. -Wno-weak-vtables # vtable emitted per TU for inline-virtual classes -Wno-ctad-maybe-unsupported # CTAD on types without explicit deduction guides -Wno-padded # struct tail/inter-member padding -Wno-exit-time-destructors # function-local statics with non-trivial dtors -Wno-global-constructors # non-trivial namespace-scope initializers + # Emscripten's sysroot stdio.h defines `#define stderr (stderr)` + # (a legal, intentional self-referential object-like macro used + # to make `stderr` a valid preprocessor token while still + # resolving to the libc symbol) -- logger.hpp's + # std::println(stderr, ...) call trips -Wdisabled-macro-expansion + # on that expansion. Not fixable in logger.hpp itself: the macro + # is the *platform's*, not this codebase's, and every other + # target's libc either doesn't define stderr as a macro at all or + # doesn't self-reference it this way. + -Wno-disabled-macro-expansion # (c) Stylistic / opinionated noise, not defects. -Wno-missing-noreturn - -Wno-nrvo # not eliding a trivial-type copy on return + # Deliberately-partial designated initialization of DTO/config- + # style aggregates (morph::session::Context{.principal = ...} + # and its many siblings across the ladder rungs) is this + # codebase's normal way to construct one with everything else + # left at its member default -- not an oversight this warning + # should flag. Probed like the other recent-Clang-only flags + # above: not every Clang release has this diagnostic yet. + $<$:-Wno-missing-designated-field-initializers> + $<$:-Wno-nrvo> # not eliding a trivial-type copy on return -Wno-shadow-uncaptured-local # lambda param shadowing an uncaptured local -Wno-documentation-unknown-command -Wno-unsafe-buffer-usage # flags all pointer arithmetic; needs a hardened API - -Wno-unsafe-buffer-usage-in-libc-call + $<$:-Wno-unsafe-buffer-usage-in-libc-call> -Wno-float-equal # exact == is intentional in the value/rational tests # (d) Conflicts with a warning we deliberately keep. -Wno-covered-switch-default # collides with -Wswitch-enum + -Wswitch-default # (e) Third-party test macros. - -Wno-c2y-extensions # Catch2 TEST_CASE expands __COUNTER__ + $<$:-Wno-c2y-extensions> # Catch2 TEST_CASE expands __COUNTER__ -Wno-unused-member-function # Catch2/test-fixture helper members -Wno-unneeded-member-function # (f) Clang 22 (Homebrew, macOS libc++) added thread-safety-analysis diff --git a/docs/spec/core/backend.md b/docs/spec/core/backend.md index 614f977c..9908323b 100644 --- a/docs/spec/core/backend.md +++ b/docs/spec/core/backend.md @@ -186,11 +186,10 @@ 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. +`BridgeHandler`'s initial construction. The re-registration `switchBackend()` +and the reconnect handler perform after a backend swap remains synchronous; +giving that 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 @@ -209,9 +208,44 @@ queue is drained by `cancelPending`, which still invokes each queued request's `onError` exactly once, exactly like an in-flight (already-sent) registration would. See `QtWebSocketBackend`'s own section below. +### Shared/keyed registration — `registerModelSharedAsync` / `attachModelAsync` + +`registerModelShared` and `attachModel` have the same problem for the same +reason, reached by a different route: a keyed screen's first payload-keyed +`execute()` attaches, and on a wire backend that attach blocks in `sendSync`, +which aborts a WASM main thread. Both therefore have an optional non-blocking +counterpart with `registerModelAsync`'s exact shape and contract — +`false` by default, `true` plus exactly one later callback when a backend opts +in: + +| Virtual | Synchronous counterpart | Preferred by | +|---|---|---| +| `registerModelSharedAsync(typeId, factory, identity, onRegistered, onError)` | `registerModelShared` | `Bridge::ensureBoundAsync` | +| `attachModelAsync(typeId, factory, identity, current, onRegistered, onError)` | `attachModel` | `Bridge::attachHandlerAsync` | + +`QtWebSocketBackend` implements both behind the same +`asyncRegistrationEnabled` flag, reusing the same `callId`-keyed pending map +(reply routing is verb-agnostic — a `register`, a shared `register`, and an +`attach` all reply the same way). An empty `identity.primary` degrades to +`registerModelAsync`, mirroring the synchronous methods' degrade-to-private +behaviour. + +Unlike the synchronous `attachModel`'s default implementation, +`attachModelAsync` does **not** release `current` itself: an overriding backend +is behind a wire protocol whose single `attach` request re-points server-side, +leaving nothing to deregister — the same division of responsibility +`QtWebSocketBackend::attachModel` already follows for a non-empty primary. + +`BridgeHandler::execute()`'s public signature and contract are unchanged; see +[shared_instances.md](shared_instances.md), "Async register-or-attach and +attach", for the caller-visible story and the `_attachMtx` locking rule these +two `Bridge` methods must obey. + ## Error types -Four exception types are thrown into in-flight `Completion`s: +Five exception types are thrown into in-flight `Completion`s. The first four are +raised by a backend; `ClientTimeoutError` is raised by `Bridge` itself, but is +declared alongside them so callers catch every dispatch failure from one header: | Type | Trigger | Purpose | |---|---|---| @@ -219,6 +253,7 @@ Four exception types are thrown into in-flight `Completion`s: | `BridgeDestroyedError` | `Bridge` is destroyed | In-flight completions are cancelled because the bridge is gone. | | `DisconnectedError` | Transport drops mid-call (e.g. WebSocket disconnect) | Framework retries the call on reconnect if the backend supports it; otherwise the GUI's `.onError(...)` runs. | | `TimeoutError` | Server-side `LimitPolicy::executeTimeout` elapses | Distinguishes a bounded-wait timeout from any other `err` reply, so callers can retry or surface a specific "request timed out" message. | +| `ClientTimeoutError` | Client-side `Bridge::setExecuteDeadline` elapses with *no* reply of any kind | Bounds the caller's wait when nothing comes back at all (a dropped frame, a hung server). Unlike `TimeoutError` it carries no evidence the server ever saw the request — see [`completion.md`](completion.md), "Client-side execute deadline". | ## `LocalBackend` — in-process execution @@ -430,11 +465,15 @@ A server-side execute timeout surfaces to a caller as `morph::backend::TimeoutEr than a generic `std::runtime_error`, on both `SimulatedRemoteBackend` and `QtWebSocketBackend`. -The background timer that enforces `executeTimeout` is `detail::TimeoutScheduler` — -a single dedicated thread per `RemoteServer` (mirroring `NetworkMonitor`'s +The background timer that enforces `executeTimeout` is +`morph::async::detail::TimeoutScheduler` (`include/morph/core/timeout_scheduler.hpp`) +— a single dedicated thread per `RemoteServer` (mirroring `NetworkMonitor`'s condition-variable wait loop), lazily started by `setLimitPolicy` the first time `executeTimeout` is configured, so a server that never uses the feature pays no -extra thread. +extra thread. The class lives in `morph::async::detail` rather than +`morph::backend::detail` because `Bridge` uses the same primitive for the +*client*-side `setExecuteDeadline` — see [`completion.md`](completion.md), +"Client-side execute deadline". ### Connection scopes @@ -1202,6 +1241,7 @@ thread to marshal onto. | `BridgeDestroyedError` | `std::runtime_error` | `"bridge destroyed before completion resolved"` | | `DisconnectedError` | `std::runtime_error` | `"transport disconnected before completion resolved"` | | `TimeoutError` | `std::runtime_error` | `"execute timed out on the server"` | +| `ClientTimeoutError` | `std::runtime_error` | `"execute timed out waiting for any reply"` | ### `LocalBackend` @@ -1361,7 +1401,7 @@ not a behavior change to the existing loopback-only default. | Reconnect handler skipped on first connect | Fired only when `_everConnected` was already true | The initial handler registration is driven by `BridgeHandler` constructors; firing the reconnect handler on the very first connect would double-register. | | No reconnect for never-connected sockets | `disconnected` schedules a retry only if `_everConnected` | A socket that never reached the server (bad URL / refused) fails fast via `waitForConnected` returning false, rather than backing off forever. | | Server reply marshalled to the Qt thread | `QMetaObject::invokeMethod(..., QueuedConnection)` with a `QPointer` | `RemoteServer::handle` produces the reply on a pool thread, but `QWebSocket::sendTextMessage` must run on the Qt thread; the weak `QPointer` drops the reply cleanly if the client disconnected meanwhile. | -| `executeTimeout` implementation | A dedicated, lazily-started background thread (`detail::TimeoutScheduler`) per `RemoteServer`, not a per-call thread | `IExecutor` has no delayed-post primitive and `RemoteServer` is transport-agnostic (cannot assume Qt's `QTimer`). One thread amortizes across every timed call; it is only started the first time `executeTimeout` is actually configured, so a server that never uses the feature pays no cost. | +| `executeTimeout` implementation | A dedicated, lazily-started background thread (`morph::async::detail::TimeoutScheduler`) per `RemoteServer`, not a per-call thread | `IExecutor` has no delayed-post primitive and `RemoteServer` is transport-agnostic (cannot assume Qt's `QTimer`). One thread amortizes across every timed call; it is only started the first time `executeTimeout` is actually configured, so a server that never uses the feature pays no cost. | | `messagesPerSecond` algorithm | Per-connection token bucket, capacity = rate, continuous refill, drop (not close) on empty | Simplest correct rate limiter; allows a legitimate one-second burst without penalizing an otherwise well-behaved client. Dropping (vs. closing) keeps a transient burst from taking down the connection — pair with `LimitPolicy::executeTimeout` if bounded caller-side waiting is also needed. | | Graceful shutdown drains via a shared in-flight counter, not a new `IExecutor::waitIdle` | `RemoteServer` counts its own accepted-but-unreplied executes rather than adding a general drain API to `IExecutor`/`StrandExecutor` | The drain condition morph can define precisely — "every accepted execute has replied" — lives at the server layer, where the work is counted; executor.md's "no graceful drain / `waitIdle`" limitation is deliberately left as-is for raw executor users. | | Backend-change-awareness captured at registration | `IModelHolder::isBackendChangeAware()` (compile-time answer per model type) + `LocalBackend::_changeAware`, maintained by `registerModel`/`deregisterModel` | Replaces a per-`notifyBackendChanged`-call `dynamic_cast` sweep over every live model with a virtual query done once at registration, and a lookup restricted to the models that actually opted in. No RTTI dependency; cost is O(change-aware models) instead of O(all models) under `_regMtx`. No change to the model-facing contract (`IBackendChangedSink`, `BackendChangedMixin`) or to when/where `onBackendChanged()` runs. | diff --git a/docs/spec/core/bridge.md b/docs/spec/core/bridge.md index ba312e44..54cb7061 100644 --- a/docs/spec/core/bridge.md +++ b/docs/spec/core/bridge.md @@ -258,6 +258,19 @@ envelopes. The constructor does the same with the (typically empty) initial session. See [session.md](../session/session.md#how-a-context-originates-and-flows) and [backend.md](backend.md#session-propagation-to-control-envelopes). +**`setExecuteDeadline(deadline)`** / **`executeDeadline()`** installs an +opt-in, client-side wall-clock bound on how long any subsequent `executeVia()` +waits for a reply. Defaults to `std::chrono::milliseconds{0}` (disabled — the +pre-existing behavior, and no extra thread). When enabled, each dispatch races +the real reply against a `morph::async::detail::TimeoutScheduler` timer that +resolves the pending `Completion` with `morph::backend::ClientTimeoutError`; +whichever settles first wins, and the loser is discarded by +`CompletionState`'s first-result-wins rule. The on-time reply path disarms the +timer as the first statement of its completion callback. Thread-safe, its own +mutex (`_executeDeadlineMtx`). Full semantics — including how +`ClientTimeoutError` differs from the server-reported `TimeoutError` — in +[completion.md](completion.md#client-side-execute-deadline). + **`setPrincipal(principal)`** / **`currentPrincipal()`** installs and reads back a `morph::session::Principal` — the verified identity + roles, readable *outside* a dispatch (unlike `session::current()`, which only exists during @@ -519,6 +532,34 @@ slow remote round-trip on one handler's attach never blocks another handler's construction, destruction, or a `switchBackend()` call on the same `Bridge`). +`attachHandlerAsync`/`ensureBoundAsync` — the non-blocking counterparts +`BridgeHandler::execute()` routes its keyed dispatches through — take +`_attachMtx` over the same scope their synchronous twins do, but **release it +before invoking their `onDone` callback**, on every path including the +synchronous fallback. That is a hard requirement, not a style choice: +`onDone` is where the action itself is dispatched, and a result-keyed dispatch +promotes its binding via `assignHandlerPrimary`, which re-takes `_attachMtx`. +It is the same rule `registerHandlerImpl` already follows for `_mtx`. See +[shared_instances.md](shared_instances.md), "Async register-or-attach and +attach". + +The guarantee is unconditional, including for a backend that completes its +`attachModelAsync`/`registerModelSharedAsync` callback **inline** — from inside +the dispatch call itself, while the dispatching frame still holds `_attachMtx` +(`QtWebSocketBackend` does exactly this on its `!_connected` error branch). +Such a callback does not act: it parks its outcome in a +`detail::AsyncDispatchHandoff` and returns, and the dispatching frame applies +the outcome once its own dispatch call has returned — publishing under the lock +it already holds, then releasing it, then calling `onDone`. A tiny mutex inside +the handoff makes the window race-free even against a backend that replies from +another thread while its dispatch call is still on this stack, and keeps +`onDone` invoked exactly once on every interleaving. Because the inline case can +never reach the callback body, `attachHandlerAsync`'s out-of-frame success +callback is free to re-acquire `_attachMtx` for the two `std::string` fields it +publishes (`HandlerBinding::contextKey`/`primary`, which every other reader +takes that lock for); `ensureBoundAsync`'s publishes only the atomic +`currentId` and needs no lock at all. + `subscribe`/`unsubscribe` mutate the bridge's subscription registry under `_subMtx`. Callbacks never run under that mutex: `publishResult` snapshots the matching sinks under the lock and invokes them outside it, marshalled to the @@ -576,9 +617,11 @@ make teardown order-independent.) | `registerHandler(binding)` | `void registerHandler(const shared_ptr&)` | Pre-built binding. Same async-preferring behavior. | | `switchBackend` | `void switchBackend(unique_ptr)` / `void switchBackend(shared_ptr)` | Pushes the current default session onto the new backend via `setSession` before staging. 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. The `unique_ptr` overload is a template on the concrete backend type and delegates to the `shared_ptr` one — see below. | | `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 a journal `LogEntry` for loggable actions on both success (`Outcome::Succeeded`) and a throwing `Model::execute` (`Outcome::Failed`, rethrown unchanged). 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`, and the `pendingCalls()` decrement) are gated on the `_liveness` token, checked before any runs, so a completion resolving after `~Bridge()` skips them instead of touching the dangling `Bridge`. Increments `pendingCalls()` once per call before dispatch (never for the synchronous "handler not bound" early return); decrements it exactly once, from whichever of the two mutually-exclusive resolution continuations actually fires. | +| `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 a journal `LogEntry` for loggable actions on both success (`Outcome::Succeeded`) and a throwing `Model::execute` (`Outcome::Failed`, rethrown unchanged). 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`, the `pendingCalls()` decrement, and the execute-deadline disarm) are gated on the `_liveness` token, checked before any runs, so a completion resolving after `~Bridge()` skips them instead of touching the dangling `Bridge`. Increments `pendingCalls()` once per call before dispatch (never for the synchronous "handler not bound" early return); decrements it exactly once, from whichever of the two mutually-exclusive resolution continuations actually fires. Arms the client-side execute deadline when one is installed (see `setExecuteDeadline`); the fast-fail "handler not bound" path returns before that and arms nothing. | | `setDefaultSession` | `void setDefaultSession(session::Context)` | Installs default session context; also pushes it to the active backend via `IBackend::setSession` so control envelopes (register/attach/assign/deregister) carry it too, not only `execute`. | | `defaultSession` | `session::Context defaultSession() const` | Returns snapshot of default session. | +| `setExecuteDeadline` | `void setExecuteDeadline(std::chrono::milliseconds)` | Opt-in client-side execute deadline; `0` (the default) disables it. Lazily creates the backing `TimeoutScheduler` thread on first enable. | +| `executeDeadline` | `std::chrono::milliseconds executeDeadline() const` | Returns the installed deadline; `0` when disabled. | | `setPrincipal` | `void setPrincipal(session::Principal)` | Installs the verified `Principal`, readable outside a dispatch. Pass `Principal{}` to clear (sign-out). | | `currentPrincipal` | `session::Principal currentPrincipal() const` | Returns a snapshot of the installed `Principal`; default-constructed if none was ever set. | | `pendingCalls` | `[[nodiscard]] size_t pendingCalls() const noexcept` | Count of `executeVia()` dispatches not yet resolved. Relaxed atomic load; see the `Bridge` section above. | diff --git a/docs/spec/core/completion.md b/docs/spec/core/completion.md index 53540e0f..d1a0d6c3 100644 --- a/docs/spec/core/completion.md +++ b/docs/spec/core/completion.md @@ -17,6 +17,7 @@ than vanishing (see [Failure modes](#failure-modes)). - [Move-only handle — `Completion`](#move-only-handle--completiont) - [Thread safety](#thread-safety) - [Failure modes](#failure-modes) +- [Client-side execute deadline](#client-side-execute-deadline) - [Empty state](#empty-state) - [API reference](#api-reference) - [Design decisions](#design-decisions) @@ -220,6 +221,91 @@ throw — they are silent by construction. `value` — since `attachThen`'s fire-now path reads `*value` directly, not from the (already-emptied) handler vector. +## Client-side execute deadline + +Nothing in `Completion` itself imposes a time limit: a state that no producer +ever settles simply stays pending forever, and its handle's callbacks never +fire. For an in-process `LocalBackend` that is unreachable, but across a wire a +request can genuinely disappear — a frame silently discarded by +`QtWebSocketServerConfig::messagesPerSecond`'s rate limiter, a connection that +dropped between send and reply, or a server that hangs. In every one of those +cases *no reply of any kind* comes back, so no layer below the caller has +anything to resolve the `Completion` with. + +`Bridge::setExecuteDeadline(std::chrono::milliseconds)` closes that hole. + +**Opt-in, default disabled.** The deadline defaults to +`std::chrono::milliseconds{0}`, which means "no deadline" and reproduces the +pre-existing behavior exactly — a `Bridge` that never calls the setter behaves +as it always did, and spawns no extra thread. The current value is readable via +`Bridge::executeDeadline()`. + +**Single-threaded WebAssembly.** `TimeoutScheduler` has a second build, +selected by `#if defined(__EMSCRIPTEN__) && !defined(__EMSCRIPTEN_PTHREADS__)`, +that uses the browser's own `setTimeout` (`emscripten_async_call`) instead of a +thread and fires its callbacks on the main thread — the same thread the Qt +event loop and every `QtExecutor`-posted completion callback already run on. +This is not a degradation switch: deadlines still fire, with the same +first-result-wins race and the same `ClientTimeoutError`. It exists because a +`wasm_singlethread` Qt build (what `.github/workflows/wasm-ladder.yml` installs +and what `cmake/morph_add_rung.cmake` builds against, with no `-pthread`) links +Emscripten's non-pthread `pthread_create` stub, so constructing a `std::thread` +throws `std::system_error` at runtime — which would have made +`setExecuteDeadline` unusable from a browser tab, and with it +`examples/common/gui/event_poller.hpp`, whose constructor calls it +unconditionally. Two behavioural differences, both documented in +`timeout_scheduler.hpp`'s own `@file` comment: callbacks are never concurrent +with the caller, and `cancel()` releases the callback immediately but leaves the +underlying browser timer to elapse harmlessly rather than clearing it. **This +build has never been compiled or run in this repository** — no Emscripten +toolchain is available here; its only verification is the `ladder-wasm` CI +compile gate. + +**Mechanics.** Every `executeVia()` call made while a non-zero deadline is +installed arms a timer on a `Bridge`-owned +`morph::async::detail::TimeoutScheduler` (a single background thread — or, in a +single-threaded WASM build, a browser timer; see above — created lazily on the +first call that enables a deadline and torn down with the `Bridge`; the same +class `RemoteServer` uses for its server-side `LimitPolicy::executeTimeout`). The timer's callback captures only the typed +`CompletionState` — never the `Bridge` — and resolves it with +`morph::backend::ClientTimeoutError`. The real reply and the timer therefore +race, and **whichever settles the state first wins**, because `setValue` / +`setException` are no-ops once the state is `ready` (see +[Failure modes](#failure-modes) and the *first-result-wins* row in +[Design decisions](#design-decisions)). A real reply that arrives after the +deadline already fired is silently discarded — it is an ordinary late write to +an already-resolved state, not an error condition. Conversely, a reply that +arrives first disarms the timer as the *first* statement of the completion +callback, before any `onResult` / `publishResult` fan-out work, so a slow +subscriber cannot open a window for the timer to fire against a result already +in hand. + +The deadline is armed only for real dispatches. `executeVia()`'s fast-fail path +for an unbound handler resolves its `Completion` synchronously before the timer +block is reached, so no timer is created for it. + +The disarm is guarded on the same `Bridge` liveness token the rest of the +completion callback uses: the callback can in principle run after `~Bridge()` +(the backend may be co-owned and outlive the `Bridge`). Skipping the disarm in +that case is harmless — `~TimeoutScheduler` drops still-pending entries without +firing them. + +**`ClientTimeoutError` vs. `TimeoutError`.** Both live in `morph::backend` and +both derive from `std::runtime_error`, but they report different facts: + +| Type | Raised by | Means | +|---|---|---| +| `TimeoutError` | The **server**, as an explicit `err "timeout"` reply when `LimitPolicy::executeTimeout` elapses | The request *was* received and the action *is* running (morph never interrupts an in-flight `Model::execute`); the server chose to stop making the caller wait. | +| `ClientTimeoutError` | The **client**, when `Bridge::setExecuteDeadline`'s duration elapses | Nothing came back at all. Whether the server ever received the request, is still processing it, or replied over a connection that had already dropped is **unknown**. | + +The practical consequence for callers: `TimeoutError` confirms the action is +in flight server-side, so a blind retry risks a duplicate. `ClientTimeoutError` +confirms nothing, so a retry must be idempotent (or reconciled) either way. + +A deadline bounds the *caller's wait*, never the work. It does not cancel the +request — see [Limitations](#limitations), "No cancellation". The server-side +counterpart is documented in [`backend.md`](backend.md) under `LimitPolicy`. + ## Empty state A default-constructed `Completion` has a null `_state` pointer. `then()` and @@ -283,6 +369,10 @@ future/promise or a monadic async type. Its scope is narrow by design: promise/awaiter machinery. Consumption is callback-only. - **No cancellation.** There is no handle to cancel an outstanding operation; once started, it runs to completion (or is abandoned). + `Bridge::setExecuteDeadline` (see + [Client-side execute deadline](#client-side-execute-deadline)) is not an + exception to this: it bounds how long the *caller* waits by resolving the + state early, and does nothing to the work still in flight underneath. - **Single consumer handle, but multiple handlers per outcome.** The `Completion` handle itself is move-only — only one owner at a time — but each state's `onOk`/`onErr` are vectors, so repeated `then()`/`onError()` @@ -315,10 +405,13 @@ state; the log is emitted only when the state itself is finally destroyed with a is the executor on which every callback is posted. - [`logger.md`](logger.md) — `morph::log::logError`, the error-handling sink used by orphan detection when an error is abandoned. +- [`backend.md`](backend.md) — backends resolve the pending `Completion` when a + response arrives; also `morph::backend::LimitPolicy::executeTimeout`, the + *server-side* counterpart to + [the client-side execute deadline](#client-side-execute-deadline), and + `TimeoutError` / `ClientTimeoutError`. - [`error_handling.md`](../error_handling.md) — the framework-wide error-propagation story; the orphan-logging contract detailed in this file is summarised there alongside the executor and backend error paths. - [`bridge.md`](bridge.md) — `BridgeHandler` produces `Completion` from - `execute()` and posts callbacks on the GUI executor. -- [`backend.md`](backend.md) — backends resolve the pending `Completion` when a - response arrives. \ No newline at end of file + `execute()` and posts callbacks on the GUI executor. \ No newline at end of file diff --git a/docs/spec/core/registry.md b/docs/spec/core/registry.md index f815ba6f..164940a8 100644 --- a/docs/spec/core/registry.md +++ b/docs/spec/core/registry.md @@ -12,6 +12,7 @@ without knowing their concrete types. - [Customisation traits](#customisation-traits) - [ModelTraits](#modeltraits) - [ActionTraits](#actiontraits) + - [Control bytes in action and result bodies](#control-bytes-in-action-and-result-bodies) - [Validation and logging policy](#validation-and-logging-policy) - [ActionValidator](#actionvalidator) - [ValidationError](#validationerror) @@ -173,6 +174,32 @@ struct ActionTraits; // forward — specialize or use BRIDGE_REGISTER_ACTION All four JSON functions throw `detail::ParseError` (a `std::runtime_error` subclass) on glaze encode/decode failure. +#### Control bytes in action and result bodies + +`toJson`/`resultToJson` write with `detail::EscapingWriteOpts`, a `glz::opts` +refinement that turns on glaze's `escape_control_characters` — the same +treatment, and for the same two reasons, that `wire::encode` already applies +to the envelope (see wire.md, "Control bytes in string fields"). With the +option off, an ASCII control byte (U+0000–U+001F) in any caller-supplied +string field of an action or result: + +- **produces invalid output** — RFC 8259 requires those code points to be + escaped, and glaze's own reader enforces it, so the peer's `fromJson` throws + a `ParseError` on a body its own peer just wrote; and +- **can be silently corrupted** — with a `\` or `"` earlier in the same + string, glaze's chunked fast path writes such a byte out as two `0x00` + bytes, and the result still decodes. + +Action bodies are pure caller data (a paste's content, a chat message, a +filename), so this is at least as exposed as the envelope was. Escaping is +lossless in both directions; the read side needs no counterpart, since glaze's +reader already accepts `\uXXXX`. + +`morph::model::detail::EscapingWriteOpts` deliberately duplicates +`morph::wire::detail::EscapingWriteOpts` rather than reusing it: the action +codec belongs to the model layer and must not acquire a dependency on the +transport layer's header to share a four-line option struct. + ## Validation and logging policy ### `ActionValidator` @@ -583,7 +610,9 @@ Expands to: `static constexpr std::string_view typeId()` (no `noexcept`, unlike `ModelTraits::typeId()`), a `static constexpr Loggable loggable`, and four JSON codec functions (each throwing `detail::ParseError` on failure): `toJson`/ - `resultToJson` use `glz::write_json`; `fromJson`/`resultFromJson` use + `resultToJson` use `glz::write` (see + ["Control bytes in action and result bodies"](#control-bytes-in-action-and-result-bodies)); + `fromJson`/`resultFromJson` use `glz::read` — the same forward-compatibility convention `wire::decode` uses (see wire.md, "Action-evolution policy") — so an older-compiled action struct silently diff --git a/docs/spec/core/shared_instances.md b/docs/spec/core/shared_instances.md index 74f2c836..98221eeb 100644 --- a/docs/spec/core/shared_instances.md +++ b/docs/spec/core/shared_instances.md @@ -11,6 +11,7 @@ - [The instance directory](#the-instance-directory) - [Enumerating live instances](#enumerating-live-instances) - [Wire protocol changes](#wire-protocol-changes) +- [Async register-or-attach and attach](#async-register-or-attach-and-attach) - [Ownership and authorization](#ownership-and-authorization) - [Lifetime and the A7 connection-scope change](#lifetime-and-the-a7-connection-scope-change) - [API reference](#api-reference) @@ -271,6 +272,65 @@ its primary so journal entries carry the entity key — but conflating them woul silently change behaviour for anyone already setting `contextKey` for journal purposes, which the framework's opt-in discipline forbids. +## Async register-or-attach and attach + +No wire change: the three requests above are unchanged. What changed is that a +backend may now answer them *without blocking the caller*, through two opt-in +`IBackend` virtuals that mirror `registerModelAsync`'s established shape +(see [backend.md](backend.md), "Asynchronous registration"): + +| Virtual | Synchronous counterpart | Preferred by | +|---|---|---| +| `registerModelSharedAsync(typeId, factory, identity, onRegistered, onError)` | `registerModelShared` | `Bridge::ensureBoundAsync` | +| `attachModelAsync(typeId, factory, identity, current, onRegistered, onError)` | `attachModel` | `Bridge::attachHandlerAsync` | + +Both default to returning `false` without calling either callback; a backend +that opts in sends the request, returns `true` immediately, and later invokes +exactly one of `onRegistered(ModelId)` / `onError(message)` on its own thread. +`QtWebSocketBackend` implements both, gated behind the *same* +`QtWebSocketBackendConfig::asyncRegistrationEnabled` flag `registerModelAsync` +already uses — there is no second knob. Their replies route through the +existing `callId`-keyed pending-registration map, which is verb-agnostic: +`register` (shared or not) and `attach` all reply `ok` with a `modelId`, or +`err`. An empty `identity.primary` degrades to the private async path +(`registerModelAsync`), mirroring the synchronous methods' own +degrade-to-private behaviour rather than inventing new semantics. + +**Why this exists.** `registerModelShared`/`attachModel` are synchronous, so on +a wire backend they block in a nested `QEventLoop`, which a WASM main thread +cannot spin at all. Before this, the *first* payload-keyed action a WASM client +executed — the very shape a keyed screen is built on — aborted the page. See +`examples/LADDER.md`, "Framework prerequisites" #1, for the rung-3 (`polls`) +scenario that motivated closing this. + +**What callers see.** Nothing, by design. `BridgeHandler::execute()`'s +signature and its documented contract are unchanged, including the promise that +a payload- or result-keyed action's attach/promote step never throws out of the +call but resolves the returned `Completion`'s `.onError(...)` instead. Only +*how* that promise is kept changed: `execute()` now routes its keyed dispatch +through `Bridge::attachHandlerAsync` / `Bridge::ensureBoundAsync`, which use the +async virtuals when the backend has them and otherwise run the identical +synchronous attach inline and call back before returning. A backend that has not +opted in behaves byte-for-byte as it did before. The one observable difference on +a backend that *has* opted in is that the dispatch happens after the attach's +reply arrives rather than on the calling stack — which is the point. + +**`attach()` stays synchronous.** The standalone `handler.attach(key)` is a +`void` call with no `Completion` to route a failure through, so it still throws +and still blocks. That is deliberate, and its own doc comment already named the +escape hatch: *a caller that wants the failure delivered asynchronously should +attach via a payload-keyed action's `execute()` instead.* This section is what +makes that escape hatch real. Giving `attach()` itself an async form would mean +changing its return type, which is a separate, breaking decision. + +**Locking.** `Bridge::attachHandlerAsync`/`ensureBoundAsync` hold `_attachMtx` +across the guard check, the async dispatch, and the synchronous fallback's own +state mutation — but never across the `onDone` callback. This is load-bearing, +not stylistic: what `execute()` does from inside `onDone` is dispatch the +action, and a result-keyed dispatch promotes its binding through +`assignHandlerPrimary`, which takes `_attachMtx` itself. It is the same rule +`registerHandlerImpl` already follows for `_mtx`. + ## Ownership and authorization `RemoteServer` records an `ownerPrincipal` for each instance at register time @@ -341,9 +401,11 @@ strictly reduces pressure on it. | `BRIDGE_KEY_FROM(A, &A::field)` | macro | Declares that a further action `A` also carries the key. | | `BRIDGE_MODEL_KEY_FROM_RESULT(M, A, &R::field)` | macro | As `BRIDGE_MODEL_KEY`, but the key comes from `A`'s *result*. | | `BRIDGE_KEY_FROM_RESULT(A, &R::field)` | macro | A further creating action whose result establishes the key. | -| `handler.attach(key)` | `void` | Attaches (or re-points) without executing an action. | +| `handler.attach(key)` | `void` | Attaches (or re-points) without executing an action. Synchronous and throwing, by design — see [Async register-or-attach and attach](#async-register-or-attach-and-attach). | | `handler.primary()` | `std::optional` | The handler's current primary; empty if unattached. | | `handler.instances()` | `Completion>` | Snapshot of live shared keys for this model type. | +| `handler.execute(keyedAction)` | `Completion` | Unchanged signature and contract. Its attach (payload-keyed) or bind-and-promote (result-keyed) step takes the backend's async path when one exists, so the call no longer blocks on a round-trip — visible only as *not aborting a WASM main thread*. See [Async register-or-attach and attach](#async-register-or-attach-and-attach). | +| `IBackend::registerModelSharedAsync` / `attachModelAsync` | `bool` | Opt-in non-blocking counterparts to `registerModelShared`/`attachModel`; `false` by default, and callers then fall back to the synchronous method unchanged. | ## Design decisions diff --git a/docs/spec/testing_strategy.md b/docs/spec/testing_strategy.md index de8a158d..2d7c7fd7 100644 --- a/docs/spec/testing_strategy.md +++ b/docs/spec/testing_strategy.md @@ -125,9 +125,22 @@ regression cases under `tests/fuzz/findings/`: log-bound text, and a raw `0x1B` in it would carry an ANSI escape into the reader's terminal. + That fix, in turn, covered the *envelope* only. An execute envelope's `body` + is not written by `wire::encode` at all — it is produced separately by + `ActionTraits::toJson` / `resultToJson` (registry.hpp's + `BRIDGE_REGISTER_ACTION` macro), which wrote with plain `glz::write_json` and + so reproduced the identical gap for every string field of every action and + result. Action bodies are pure caller data (a paste's content, a chat + message, a filename), so this is at least as exposed as the envelope was. + Found from the other end — by the application ladder's rung 1 (pastebin) + replaying `tests/fuzz/findings/` as *paste content*, which is the round trip + its README's "hostile content" requirement asks for — and fixed with the same + instrument one layer down, `model::detail::EscapingWriteOpts` (see + docs/spec/core/registry.md, "Control bytes in action and result bodies"). + These fixes are covered by dedicated regression tests in -`tests/test_wire_hardening.cpp` ("Bug C"/"Bug D"/"Bug E"/"Bug F") in addition to -the `fuzz_*_replay` findings above. +`tests/test_wire_hardening.cpp` ("Bug C"/"Bug D"/"Bug E"/"Bug F"/"Bug G") in +addition to the `fuzz_*_replay` findings above. ## Soak tests (`tests/soak/`) diff --git a/docs/spec/util/quantity_type.md b/docs/spec/util/quantity_type.md index 5480ebe7..19adec20 100644 --- a/docs/spec/util/quantity_type.md +++ b/docs/spec/util/quantity_type.md @@ -824,7 +824,8 @@ yields empty. | Symbol | Kind | Notes | |---|---|---| | `NamedQuantity` | class template | `Quantity` (declared precision defaulted) that names itself `Name` on construction; slices losslessly to a plain `Quantity`. `Name` is a `detail::FixedString` NTTP. Constructors: default (empty), `optional`, and from a plain `Quantity` — each names the value after building it; plus `static fromDouble(double)`. The name lives in the shared history, not as extra data. | -| `std::formatter>` | specialisation | Renders value + unit (`5.2kW`, `N/A%`). No `operator<<`. | +| `toString(Quantity)` | free function | Renders value + unit (`5.2kW`, `N/A%`) — the identical text `std::format("{}", q)` produces, via the identical logic; the `std::formatter` specialisation below delegates to it. Exists solely so a caller can render a `Quantity` without going through `std::format`'s own trait machinery: Emscripten's bundled libc++ has a known gap recognising a `std::formatter` partial specialisation parameterised over an `auto` non-type template parameter, so `std::format("{}", quantity)` fails to compile there outright, even though the specialisation is valid and works on every other toolchain this project targets. `pastebin::gui::readsText`/`bookmarks::gui::countText`/`polls::gui::countText` (the three QML-bridge call sites that render a `Quantity`) call it directly for this reason. | +| `std::formatter>` | specialisation | Renders value + unit (`5.2kW`, `N/A%`) by delegating to `toString`. No `operator<<`. | | `std::formatter>` | specialisation | Forwards to the `Quantity` formatter. | On the wire, `glz::meta` reduces the instance to its nullable @@ -847,7 +848,7 @@ and `unitAlternatives()`. | History structure | **Shared, unit-erased DAG (`ASTUnit` / `ASTNode` / `Context`)** | `ASTNode` (step + optional name + `shared_ptr` children) linked into a DAG; each `Quantity` holds a `Context` (a `shared_ptr` root). Cheap copies; reused subexpressions deduped by node identity; shareable across units. | | Placeholders | **Reuse, not leaf-vs-computed, mints a `cN`** | A value used once inlines (leaf → its number, computed → its expression); only a value reused across the expression earns one shared placeholder, so shared work is written once. | | Precision | **Actual = max of engaged operands; declared from `UnitTraits`** | Max-propagation keeps a result no less precise than its widest input; the declared tag stays a field property (`fromDouble` origin, `atDeclaredPrecision` to reset). | -| Formatting | **`std::formatter` only, delegating to the shared `formatRationalDecimal` renderer** | Single formatting path; no `operator<<`; the runtime `DecimalPlaces` tag is the sole authority on printed decimals. | +| Formatting | **`std::formatter`, delegating to `toString`, which delegates to the shared `formatRationalDecimal` renderer** | One rendering implementation (`toString`); `std::formatter` is a thin wrapper over it, not a second implementation. `toString` itself exists as a direct call target only because `std::format`'s compile-time formattability check fails to see this formatter's `auto`-NTTP specialisation on Emscripten's bundled libc++ — see the symbol table above. No `operator<<`; the runtime `DecimalPlaces` tag is the sole authority on printed decimals. | | Wire | **Payload only** | Units and history never travel; the wire stays a nullable `Rational`. | | Empty ordering | **Throws, not a compile error** | Emptiness is a runtime `optional` state, so ordering an empty operand throws `std::logic_error` (a testable defined diagnostic); `==` stays total. | | Conversion | **`UnitRelation` entries + auto-generated constrained-template `convert`; `UnitTraits::convert` static override** | Application declares exact peer-to-peer ratios in `UnitTraits::relations`; framework auto-generates a constrained `convert(From, To&)` template and records the provenance step. A `UnitTraits::convert` static wins (`if constexpr`) for non-ratio conversions (C↔F, currency) — a static, not an ADL free function, because the unit is a non-type template arg so ADL can't reach the enum's namespace. Chaining composes ratio edges over the relation graph. | diff --git a/include/morph/core/backend.hpp b/include/morph/core/backend.hpp index 65a731a0..833c1704 100644 --- a/include/morph/core/backend.hpp +++ b/include/morph/core/backend.hpp @@ -131,10 +131,12 @@ struct IBackend { /// /// @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. + /// uses this. Shared/keyed registration has its own opt-in async + /// pair, `registerModelSharedAsync`/`attachModelAsync` below, + /// preferred by `Bridge::ensureBoundAsync`/`attachHandlerAsync`. + /// The re-registration `switchBackend()`/the reconnect handler + /// perform after a backend swap remains 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. @@ -155,6 +157,46 @@ struct IBackend { return false; } + /// @brief Optional non-blocking counterpart to `registerModelShared`. + /// + /// Same rationale and shape as `registerModelAsync` (see its doc comment + /// immediately above): `registerModelShared`'s synchronous default + /// implementations block the calling thread until a reply arrives, which + /// aborts a WASM main thread the moment a shared/keyed handler makes its + /// first attach. A backend that overrides this sends the request and + /// returns `true` immediately, then invokes 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). + /// + /// The default implementation offers no async path and returns `false` + /// without calling either callback — the caller (`Bridge::ensureBoundAsync`) + /// falls back to the synchronous `registerModelShared` in that case, + /// matching every caller's behavior before this method existed. + /// + /// @param typeId String type-id of the model. + /// @param factory Callable that constructs the `IModelHolder` (local path only). + /// @param identity Entity key for the action log plus the directory primary key. + /// @param onRegistered Invoked with the assigned/attached `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. + // NOLINTBEGIN(performance-unnecessary-value-param) — by-value matches + // registerModelAsync's signature exactly; overriding backends move the + // callbacks into their pending-reply map. + virtual bool registerModelSharedAsync( + const std::string& typeId, std::function()> factory, + InstanceIdentity identity, std::function onRegistered, + std::function onError) { + (void)typeId; + (void)factory; + (void)identity; + (void)onRegistered; + (void)onError; + return false; + } + // NOLINTEND(performance-unnecessary-value-param) + /// @brief Registers or attaches to the shared instance holding @p primary. /// /// A *register-or-attach*: if an instance for `(typeId, primary)` is already @@ -218,6 +260,44 @@ struct IBackend { return next; } + /// @brief Optional non-blocking counterpart to `attachModel`. + /// + /// Same rationale and shape as `registerModelSharedAsync` immediately + /// above (itself mirroring `registerModelAsync`) — see that doc comment + /// for the full opt-in/fallback contract. + /// + /// @note Unlike the synchronous `attachModel` default above, this method + /// does *not* release @p current itself: an overriding backend is + /// behind a wire protocol, whose single `attach` request re-points + /// server-side and therefore leaves nothing to deregister — exactly + /// the division of responsibility `QtWebSocketBackend::attachModel` + /// already follows for a non-empty `identity.primary`. @p current is + /// passed so that request can name what it is re-pointing from. + /// + /// @param typeId String type-id of the model. + /// @param factory Callable that constructs the `IModelHolder` (local path only). + /// @param identity Entity key for the action log plus the directory primary key. + /// @param current Instance currently held, or `ModelId{0}` if none. + /// @param onRegistered Invoked with the `ModelId` now attached to, 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. + // NOLINTBEGIN(performance-unnecessary-value-param) — see registerModelSharedAsync above. + virtual bool attachModelAsync(const std::string& typeId, + std::function()> factory, + InstanceIdentity identity, ::morph::exec::detail::ModelId current, + std::function onRegistered, + std::function onError) { + (void)typeId; + (void)factory; + (void)identity; + (void)current; + (void)onRegistered; + (void)onError; + return false; + } + // NOLINTEND(performance-unnecessary-value-param) + /// @brief Enters an already-live instance into the directory under @p primary. /// /// The *promotion* half of keyed instances, and what makes a result-sourced @@ -459,6 +539,22 @@ struct TimeoutError : std::runtime_error { TimeoutError() : std::runtime_error{"execute timed out on the server"} {} }; +/// @brief Thrown to a pending `Completion` when `Bridge::setExecuteDeadline`'s +/// duration elapses before any reply arrives — a frame silently +/// dropped by `QtWebSocketServerConfig::messagesPerSecond`, or a +/// genuinely hung server, either way. +/// +/// Distinct from `TimeoutError`: that type means the *server* explicitly +/// replied that it hit `LimitPolicy::executeTimeout` while the action was +/// still running. `ClientTimeoutError` means the client gave up waiting — +/// no reply of any kind arrived, so whether the server ever received the +/// request, is still processing it, or replied to a connection that had +/// already dropped is unknown. See `docs/spec/core/completion.md`. +struct ClientTimeoutError : std::runtime_error { + /// @brief Constructs the error with a canned diagnostic message. + ClientTimeoutError() : std::runtime_error{"execute timed out waiting for any reply"} {} +}; + /// @brief In-process backend that executes model actions on a thread pool strand. /// /// Each model instance gets its own strand so actions are serialised per-model diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index c3c06035..6f392a73 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -4,7 +4,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -24,6 +26,7 @@ #include "completion.hpp" #include "model_key.hpp" #include "registry.hpp" +#include "timeout_scheduler.hpp" namespace morph::bridge { @@ -220,6 +223,93 @@ struct HandlerBinding { std::vector, std::function>> registrationWaiters; }; +/// @brief Outcome a backend's inline completion parked for its dispatcher. +struct ParkedOutcome { + /// @brief `true` when the parked outcome is a success. + bool succeeded = false; + /// @brief Instance id the success callback reported. + ::morph::exec::detail::ModelId modelId{}; + /// @brief Diagnostic the error callback reported; null on success. + std::exception_ptr failure; +}; + +/// @brief Handoff slot between an async attach/bind dispatch and its callback. +/// +/// `Bridge::attachHandlerAsync`/`ensureBoundAsync` dispatch to the backend while +/// holding `_attachMtx`, and both promise to release it before invoking their +/// `onDone`. A backend whose `attachModelAsync`/`registerModelSharedAsync` +/// completed its callback *inline* — synchronously, before the dispatch call +/// returned, as `QtWebSocketBackend` does on its `!_connected` error branch — +/// would otherwise break that promise from inside the dispatch frame, with the +/// lock still held. +/// +/// So instead of acting, such a callback parks its outcome here and returns; the +/// dispatching frame picks it up after the dispatch call returns, publishes it +/// under the lock it already holds, releases the lock, and only then reports. +/// `mtx` makes the handover race-free even for a backend that replies from +/// another thread *while* its own dispatch call is still on this stack, and the +/// `fired` flag keeps `onDone` invoked exactly once on every interleaving. +struct AsyncDispatchHandoff { + /// @brief Guards every other field; never held across `onDone` or `_attachMtx`. + std::mutex mtx; + /// @brief `true` while the backend's dispatch call is still on the caller's stack. + bool inFrame = true; + /// @brief Set once either callback has claimed the outcome. + bool fired = false; + /// @brief `true` when the parked outcome is a success, `false` for a failure. + bool succeeded = false; + /// @brief Instance id the success callback reported. + ::morph::exec::detail::ModelId modelId{}; + /// @brief Diagnostic the error callback reported; null on success. + std::exception_ptr failure; +}; + +/// @brief Records a backend callback's outcome in @p handoff. +/// +/// Called first thing by both completion callbacks of an async attach/bind +/// dispatch. +/// +/// @param handoff Handoff slot created by the dispatching frame. +/// @param succeeded `true` for the success callback, `false` for the error one. +/// @param modelId Instance id, for the success callback; ignored otherwise. +/// @param failure Diagnostic, for the error callback; null otherwise. +/// @return `true` if the caller must **not** act — either because the dispatch +/// call is still on the dispatcher's stack (which owns the outcome from +/// here on) or because another callback already claimed this dispatch. +/// `false` if the caller owns the outcome and should deliver it itself. +inline bool parkIfInFrame(AsyncDispatchHandoff& handoff, bool succeeded, ::morph::exec::detail::ModelId modelId, + std::exception_ptr failure) { + std::scoped_lock const guard{handoff.mtx}; + if (handoff.fired) { + // A backend is contractually allowed exactly one callback per dispatch; + // swallow a second one rather than reporting twice. + return true; + } + handoff.fired = true; + handoff.succeeded = succeeded; + handoff.modelId = modelId; + handoff.failure = std::move(failure); + return handoff.inFrame; +} + +/// @brief Closes the inline window and takes whatever a callback parked. +/// +/// Called by the dispatching frame immediately after the backend's dispatch call +/// returns. After this, a callback that has not yet run delivers its own outcome. +/// +/// @param handoff Handoff slot created by the dispatching frame. +/// @return The parked outcome if the backend completed inline (or concurrently, +/// before this frame closed the window); `std::nullopt` if the frame won +/// the race and the reply, if any, is still to come. +inline std::optional claimHandoff(AsyncDispatchHandoff& handoff) { + std::scoped_lock const guard{handoff.mtx}; + handoff.inFrame = false; + if (!handoff.fired) { + return std::nullopt; + } + return ParkedOutcome{.succeeded = handoff.succeeded, .modelId = handoff.modelId, .failure = handoff.failure}; +} + } // namespace detail /// @brief Central dispatcher that routes typed actions to an `IBackend`. @@ -356,6 +446,190 @@ class Bridge { binding->currentId.store(newId.v); } + /// @brief Async counterpart to `attachHandler`: prefers the backend's + /// `attachModelAsync` when available, invoking @p onDone once + /// attached (or failed) instead of blocking. + /// + /// Falls back to the synchronous `attachHandler` body (and calls @p onDone + /// immediately, from this thread) when the backend offers no async + /// path — so a caller that always goes through this method behaves + /// identically to calling `attachHandler` directly, on every backend + /// that has not opted in to `attachModelAsync`. + /// + /// @par Locking + /// `_attachMtx` is held around the guard check, the async branch's + /// *dispatch*, and the synchronous branch's own state mutation — matching + /// `attachHandler`'s existing lock scope — but is **released before + /// @p onDone is ever invoked**, on every path, unconditionally. That is not + /// a nicety: what `execute()` does from inside @p onDone is dispatch the + /// action, and a result-keyed dispatch promotes its binding through + /// `assignHandlerPrimary`, which takes `_attachMtx` itself. Invoking + /// @p onDone under the lock therefore self-deadlocks the moment the + /// completion is delivered on the calling thread — which is exactly what + /// the synchronous fallback below does, and what an inline executor does + /// for every callback. This is `registerHandlerImpl`'s existing rule ("the + /// backend call must not run under `_mtx`") applied to `_attachMtx`. + /// + /// The guarantee holds even for a backend that completes its callback + /// *inline*, from inside `attachModelAsync` itself, while this frame still + /// holds the lock: such a callback parks its outcome in a + /// `detail::AsyncDispatchHandoff` and returns without acting, and this frame + /// applies it after the dispatch call has returned and the lock is gone. + /// See that struct's doc comment. + /// + /// An out-of-frame success callback re-acquires `_attachMtx` for the two + /// `std::string` fields it publishes (`contextKey`/`primary`, which + /// `HandlerBinding` documents as readable only under that lock) and drops it + /// again before calling @p onDone. That re-acquisition is safe precisely + /// because the inline case never reaches it. + /// + /// @par Known gap + /// Two calls for the same key issued before the first one's reply arrives + /// are **not** deduplicated: the guard below reads `binding->primary`/ + /// `currentId`, neither of which is updated until the reply lands, so both + /// calls pass it and both dispatch an `attach`. This is a real behaviour + /// difference from the synchronous `attachHandler` it replaces, not merely + /// something inherent to asynchrony — `attachHandler` held `_attachMtx` + /// across the whole blocking round trip, which serialised concurrent + /// callers for free. It needs no second thread to hit: two `execute()` + /// calls in one event-loop turn are enough. The server answers both with + /// the same `ModelId` but counts two attachments, so one attach reference + /// leaks; the leak is bounded, not unbounded — the connection scope + /// releases every reference it holds when it closes. Closing this properly + /// needs in-flight tracking on the binding (coalescing the second caller + /// onto the first dispatch's completion); tracked as a follow-up, not fixed + /// here. Same gap, same reasoning, on `ensureBoundAsync`. + /// + /// @tparam Model Concrete model type. + /// @param binding Shared binding, as returned by `registerSharedHandler()`. + /// @param primary Canonical string encoding of the primary key to attach to. + /// @param onDone Invoked with `nullptr` on success, or a non-null + /// `exception_ptr` on failure — always exactly once, + /// synchronously if the fallback path is taken. + template + void attachHandlerAsync(const std::shared_ptr& binding, std::string primary, + const std::function& onDone) { + std::unique_lock lock{_attachMtx}; + if (binding->primary == primary && binding->currentId.load() != 0U) { + lock.unlock(); + onDone(nullptr); + return; + } + auto const previous = ::morph::exec::detail::ModelId{binding->currentId.load()}; + auto backend = loadBackend(); + auto primaryCopy = primary; + std::weak_ptr<::morph::backend::detail::IBackend> const weakBackend{backend}; + std::weak_ptr const weakLiveness{_liveness}; + std::weak_ptr const weakBinding{binding}; + auto handoff = std::make_shared(); + bool started = false; + try { + started = backend->attachModelAsync( + binding->typeId, binding->modelFactory, {.contextKey = primaryCopy, .primary = primaryCopy}, + previous, + [this, weakBackend, weakLiveness, weakBinding, primaryCopy, + onDone, handoff](::morph::exec::detail::ModelId newId) { + if (detail::parkIfInFrame(*handoff, true, newId, nullptr)) { + return; // Completed inline: the dispatching frame will finish this. + } + auto aliveToken = weakLiveness.lock(); + if (!aliveToken) { + return; // The Bridge is gone; publishing this id would be pointless. + } + auto strongBinding = weakBinding.lock(); + if (!strongBinding) { + return; // The BridgeHandler (and its binding) is gone. + } + std::exception_ptr failure; + { + // contextKey/primary are plain std::strings that five + // other sites read under `_attachMtx`; publishing them + // without it would be a data race, not just a stale + // read. + std::scoped_lock const guard{_attachMtx}; + auto pinned = weakBackend.lock(); + if (!pinned || pinned != loadBackend()) { + // A switchBackend() already moved past this attach + // (see registerHandlerImpl's identical guard) and + // its own re-registration loop already handled + // `binding` on the *new* backend -- applying this + // stale reply now would overwrite that with a + // dangling id from a backend nothing uses any + // more. Unlike registerHandlerImpl's fire-and- + // forget re-registration, a real execute() call is + // synchronously waiting on `onDone` here, so the + // stale reply must still be reported -- silently + // dropping it would hang that caller forever. + failure = std::make_exception_ptr(std::runtime_error( + "attach reply arrived from a backend switchBackend() already replaced")); + } else { + try { + strongBinding->contextKey = primaryCopy; + strongBinding->primary = primaryCopy; + strongBinding->currentId.store(newId.v); + } catch (...) { + failure = std::current_exception(); + } + } + } + onDone(failure); // Outside the lock -- see @par Locking. + }, + [onDone, handoff](const std::string& message) { + auto failure = std::make_exception_ptr(std::runtime_error(message)); + if (detail::parkIfInFrame(*handoff, false, {}, failure)) { + return; + } + onDone(failure); + }); + } catch (...) { + // The backend's own dispatch call can throw synchronously (e.g. + // QtWebSocketBackend::attachModelAsync's wire::encode() failing + // before send) -- report it like any other failure instead of + // letting it escape execute()'s documented never-throws contract. + lock.unlock(); + onDone(std::current_exception()); + return; + } + if (auto parked = detail::claimHandoff(*handoff)) { + // The backend answered on this very stack, with `_attachMtx` still + // held -- switchBackend() cannot have run concurrently (it takes + // the same lock), so no staleness check is needed here. Publish + // under the lock we already own, then release it and report -- + // @p onDone never runs inside the dispatch frame. + std::exception_ptr failure = parked->failure; + if (parked->succeeded) { + try { + binding->contextKey = primaryCopy; + binding->primary = std::move(primaryCopy); + binding->currentId.store(parked->modelId.v); + } catch (...) { + failure = std::current_exception(); + } + } + lock.unlock(); + onDone(failure); + return; + } + if (started) { + return; + } + // No async path on this backend: run the identical synchronous attach + // `attachHandler` would have run, under the same lock, then report the + // outcome only once the lock is gone (see @par Locking above). + std::exception_ptr failure; + try { + auto newId = backend->attachModel(binding->typeId, binding->modelFactory, + {.contextKey = primary, .primary = primary}, previous); + binding->contextKey = primary; + binding->primary = std::move(primary); + binding->currentId.store(newId.v); + } catch (...) { + failure = std::current_exception(); + } + lock.unlock(); + onDone(failure); + } + /// @brief Gives @p binding an anonymous instance if it does not have one yet. /// /// Used before a result-keyed action: such an action generates the key it @@ -373,6 +647,115 @@ class Bridge { binding->currentId.store(newId.v); } + /// @brief Async counterpart to `ensureBound`. See `attachHandlerAsync`'s + /// doc comment for the fallback and locking contract, including the + /// inline-completion handling and the in-flight dedup gap, both of + /// which apply here identically (two result-keyed `execute()` calls + /// on the same still-unbound handler each bind their own anonymous + /// instance; the first is then stranded until the connection scope + /// closes). + /// + /// The one difference: this method's success callback publishes only + /// `currentId`, which is a `std::atomic`, so — unlike `attachHandlerAsync`'s + /// — it needs no `_attachMtx` of its own to do it. + /// @param binding Shared binding to bind. + /// @param onDone Invoked exactly once: `nullptr` on success, or a + /// non-null `exception_ptr` on failure. + void ensureBoundAsync(const std::shared_ptr& binding, + const std::function& onDone) { + std::unique_lock lock{_attachMtx}; + if (binding->currentId.load() != 0U) { + lock.unlock(); + onDone(nullptr); + return; + } + auto backend = loadBackend(); + std::weak_ptr<::morph::backend::detail::IBackend> const weakBackend{backend}; + std::weak_ptr const weakLiveness{_liveness}; + std::weak_ptr const weakBinding{binding}; + auto handoff = std::make_shared(); + bool started = false; + try { + started = backend->registerModelSharedAsync( + binding->typeId, binding->modelFactory, {.contextKey = binding->contextKey, .primary = {}}, + [this, weakBackend, weakLiveness, weakBinding, onDone, + handoff](::morph::exec::detail::ModelId newId) { + if (detail::parkIfInFrame(*handoff, true, newId, nullptr)) { + return; // Completed inline: the dispatching frame will finish this. + } + auto aliveToken = weakLiveness.lock(); + if (!aliveToken) { + return; // The Bridge is gone; publishing this id would be pointless. + } + auto strongBinding = weakBinding.lock(); + if (!strongBinding) { + return; // The BridgeHandler (and its binding) is gone. + } + std::exception_ptr failure; + { + // Brief `_attachMtx` window purely to serialise this + // check against a concurrent switchBackend() (which + // takes the same lock) -- `currentId` itself is an + // atomic and needs no lock to store. + std::scoped_lock const guard{_attachMtx}; + auto pinned = weakBackend.lock(); + if (!pinned || pinned != loadBackend()) { + // See attachHandlerAsync's identical guard: a + // stale reply from a backend switchBackend() + // already replaced must still resolve `onDone` + // (a real execute() call is waiting), not be + // silently dropped. + failure = std::make_exception_ptr(std::runtime_error( + "attach reply arrived from a backend switchBackend() already replaced")); + } else { + strongBinding->currentId.store(newId.v); + } + } + onDone(failure); + }, + [onDone, handoff](const std::string& message) { + auto failure = std::make_exception_ptr(std::runtime_error(message)); + if (detail::parkIfInFrame(*handoff, false, {}, failure)) { + return; + } + onDone(failure); + }); + } catch (...) { + // See attachHandlerAsync's identical guard: the backend's own + // dispatch call can throw synchronously before send. + lock.unlock(); + onDone(std::current_exception()); + return; + } + if (auto parked = detail::claimHandoff(*handoff)) { + // Completed on this stack, under `_attachMtx`: publish here, then + // release the lock before reporting (see `attachHandlerAsync`). + if (parked->succeeded) { + binding->currentId.store(parked->modelId.v); + } + lock.unlock(); + onDone(parked->failure); + return; + } + if (started) { + return; + } + // No async path on this backend: run the identical synchronous + // registration `ensureBound` would have run, under the same lock, then + // report the outcome only once the lock is gone (see + // `attachHandlerAsync`'s "@par Locking"). + std::exception_ptr failure; + try { + auto newId = backend->registerModelShared(binding->typeId, binding->modelFactory, + {.contextKey = binding->contextKey, .primary = {}}); + binding->currentId.store(newId.v); + } catch (...) { + failure = std::current_exception(); + } + lock.unlock(); + onDone(failure); + } + /// @brief Files @p binding's current instance under @p primary, in place. /// /// The instance keeps everything the creating action just did — nothing is @@ -688,6 +1071,48 @@ class Bridge { } } + /// @brief Sets (or disables) the client-side execute deadline. + /// + /// Every `executeVia()` call after this point races the real reply against + /// @p deadline; whichever settles first wins (`CompletionState::setValue`/ + /// `setException` are idempotent — see `completion.hpp`). If @p deadline + /// elapses first, the pending `Completion` fails with + /// `::morph::backend::ClientTimeoutError`; the real reply, if it arrives + /// later, is silently discarded exactly like any other late write to an + /// already-resolved `CompletionState`. + /// + /// The clock starts inside `executeVia()`, immediately before the backend's + /// own `execute()` is dispatched, so @p deadline covers the whole round trip + /// — serialisation, transport, server-side work, and the reply's journey + /// back — not just the time spent waiting after dispatch. + /// + /// Disabled (`std::chrono::milliseconds{0}`, the default) reproduces + /// today's exact behavior: a dropped frame or a hung server leaves the + /// `Completion` pending forever, same as before this method existed. + /// + /// The backing `TimeoutScheduler` (and its one background thread) is + /// created lazily on the first call that enables a deadline, so a `Bridge` + /// that never opts in spawns no extra thread. Once created it lives until + /// `~Bridge()`; setting the deadline back to `0` stops new calls from + /// arming it but does not tear the thread down. Thread-safe. + /// + /// @param deadline Maximum time to wait for any reply. `0` disables the + /// deadline. + void setExecuteDeadline(std::chrono::milliseconds deadline) { + std::scoped_lock const lock{_executeDeadlineMtx}; + _executeDeadline = deadline; + if (_executeDeadline.count() > 0 && !_timeoutScheduler) { + _timeoutScheduler = std::make_shared<::morph::async::detail::TimeoutScheduler>(); + } + } + + /// @brief Returns the currently installed client-side execute deadline. + /// @return The deadline; `std::chrono::milliseconds{0}` when disabled. + [[nodiscard]] std::chrono::milliseconds executeDeadline() const { + std::scoped_lock const lock{_executeDeadlineMtx}; + return _executeDeadline; + } + /// @brief Returns a copy of the currently installed default session. Thread-safe. /// @return Snapshot of the default `Context`. [[nodiscard]] ::morph::session::Context defaultSession() const { @@ -946,6 +1371,38 @@ class Bridge { // each of which decrements exactly once (setValue/setException are // first-result-wins, so only one of the two ever actually fires). _pendingCalls.fetch_add(1, std::memory_order_relaxed); + // Arm the client-side deadline (setExecuteDeadline) only for real + // dispatches -- the fast-failed "handler not bound" completion above is + // already resolved and needs no timer. Reading the deadline and arming + // it happen under one lock so a concurrent setExecuteDeadline() cannot + // interleave between the two. + std::optional<::morph::async::detail::TimeoutScheduler::Handle> deadlineHandle; + // A private shared_ptr copy, obtained under the same lock as the + // schedule() call -- not `this->_timeoutScheduler`, and not gated on + // `alive` -- so the two `.then()`/`.onError()` callbacks below can + // call `cancel()` on a scheduler that is provably still alive, + // regardless of whether ~Bridge() has run or is running concurrently + // on another thread. `_executeDeadlineMtx` alone does not establish + // that: ~Bridge()'s own body never acquires it, so a plain + // `!alive.expired()` check followed by `_timeoutScheduler->cancel()` + // a few instructions later is a check-then-use race against + // ~Bridge()'s implicit member destruction (which joins + // TimeoutScheduler's thread). Holding a shared_ptr for the + // callback's own lifetime turns that into a non-issue by + // construction: while any copy of it is alive, ~TimeoutScheduler() + // cannot run at all. + std::shared_ptr<::morph::async::detail::TimeoutScheduler> schedulerRef; + { + std::scoped_lock const lock{_executeDeadlineMtx}; + if (_executeDeadline.count() > 0 && _timeoutScheduler) { + schedulerRef = _timeoutScheduler; + // The callback captures `typedState` alone -- never `this` -- so + // it stays safe to fire even while ~Bridge() is running. + deadlineHandle = schedulerRef->schedule(_executeDeadline, [typedState] { + typedState->setException(std::make_exception_ptr(::morph::backend::ClientTimeoutError{})); + }); + } + } ::morph::backend::detail::ActionCall call; call.modelTypeId = std::string{::morph::model::ModelTraits::typeId()}; call.actionTypeId = std::string{::morph::model::ActionTraits::typeId()}; @@ -1051,8 +1508,33 @@ class Bridge { } auto anyCompletion = backend->execute(::morph::exec::detail::ModelId{raw}, std::move(call), cbExec); anyCompletion - .then([typedState, onResult = std::move(onResult), this, raw, + .then([typedState, onResult = std::move(onResult), this, raw, deadlineHandle, schedulerRef, alive = liveness()](const std::shared_ptr& vAny) { + // Disarm the client-side deadline first, before any of the + // forwarding work below: a slow onResult/publishResult callback + // must not give the timer a window to fire concurrently and + // resolve this completion with ClientTimeoutError while the real + // result is already in hand. Uses the `schedulerRef` copy + // captured above, not `this->_timeoutScheduler` -- see that + // capture's own comment for why: this callback can in principle + // run after ~Bridge(), and `schedulerRef` (not `alive`) is what + // makes `cancel()` safe in that case, by keeping the scheduler + // alive for exactly as long as this callback needs it, not by + // racing a liveness check against ~Bridge()'s teardown. Leaving + // the entry armed if it were never cancelled would be harmless + // (~TimeoutScheduler drops pending entries without firing them), + // but a thrown cancel() must not prevent the real result from + // resolving the completion below either. + if (deadlineHandle && schedulerRef) { + try { + schedulerRef->cancel(*deadlineHandle); + } catch (...) { + // Best-effort: a failed cancel leaves the deadline's own + // entry to fire later and find nothing (setValue/ + // setException below are idempotent), which is exactly + // what an uncancelled entry already does. + } + } // Guard the value-forwarding: if R's move/copy throws (or the cast // is somehow wrong), route the exception to the typed completion's // error sink instead of letting it escape the callback executor — @@ -1106,7 +1588,22 @@ class Bridge { typedState->setException(std::current_exception()); } }) - .onError([typedState, this, alive = liveness()](const std::exception_ptr& err) { + .onError([typedState, this, deadlineHandle, schedulerRef, + alive = liveness()](const std::exception_ptr& err) { + // Same disarm-first reasoning (and the same schedulerRef-based + // safety, not a liveness-then-use race) as the success branch + // above: a real error reply settles the completion, so the + // deadline must not also fire. + if (deadlineHandle && schedulerRef) { + try { + schedulerRef->cancel(*deadlineHandle); + } catch (...) { + // Best-effort: a failed cancel leaves the deadline's own + // entry to fire later and find nothing (setValue/ + // setException below are idempotent), which is exactly + // what an uncancelled entry already does. + } + } // The other of the two mutually-exclusive resolution paths -- // see the .then continuation above. Same liveness guard: this // touches `this` and must not run once the Bridge might be gone. @@ -1325,6 +1822,16 @@ class Bridge { ::morph::session::Context _defaultSession; mutable std::mutex _principalMtx; ::morph::session::Principal _principal; + // Client-side execute deadline (see setExecuteDeadline). Both the duration + // and the lazily-created scheduler live under one mutex, so a concurrent + // setExecuteDeadline() can never let executeVia() observe a non-zero + // deadline before the scheduler backing it exists. Declared ahead of + // `_liveness` (the last member, and therefore the first destroyed) so the + // liveness token an in-flight completion callback checks before touching + // these has already expired by the time they are torn down. + mutable std::mutex _executeDeadlineMtx; + std::chrono::milliseconds _executeDeadline{0}; + std::shared_ptr<::morph::async::detail::TimeoutScheduler> _timeoutScheduler; // Instance subscriptions. Held against the binding rather than a fixed // instance id so a re-pointed handler keeps its subscriptions; matched at // publish time by comparing the binding's current instance. @@ -1452,6 +1959,16 @@ class BridgeHandler { template ::morph::async::Completion::Result> execute(Action action) { using R = ::morph::model::ActionTraits::Result; + // Three mutually exclusive routes, chained rather than sequential: an + // action is payload-keyed or result-keyed or neither (`PayloadKeyed` + // and `ResultKeyed` differ only in `fromResult`, so no action can + // satisfy both), and an unkeyed action — or any action at all on a + // `NoSharing` handler — always lands in the final `else`. Chaining the + // `if constexpr`s (rather than leaving the payload-keyed branch to + // fall through to that `else`, as it did while the attach step was + // synchronous) is what lets the keyed branches own their dispatch: the + // attach now completes asynchronously, so the dispatch it precedes has + // to happen from inside its completion callback, not on this stack. if constexpr (kShared && ::morph::model::detail::PayloadKeyed) { // The action names its instance: attach (or re-point) before // dispatching, so the call lands on the instance it asked for. A @@ -1459,13 +1976,41 @@ class BridgeHandler { // transport error, unauthorized) must surface through the // returned Completion's onError, exactly like every other // dispatch failure — not as a synchronous throw out of execute(). + // + // The attach goes through Bridge::attachHandlerAsync, which uses + // the backend's `attachModelAsync` when it has one and otherwise + // runs the identical synchronous attach inline and calls back + // before returning — so a backend that has not opted in behaves + // exactly as it did before this path existed. + auto state = std::make_shared<::morph::async::detail::CompletionState>(); + ::morph::async::Completion pending{state, _guiExec}; + auto* const bridgePtr = &_bridge; + auto binding = _binding; + // Key extraction is user code (ActionKeyTraits + keyToString), so it + // is inside the same no-throw-out-of-execute() promise the attach + // itself makes: a throw here resolves the Completion, it does not + // escape. + std::string key; try { - _bridge.template attachHandler(_binding, ::morph::model::ActionKeyTraits::key(action)); + key = ::morph::model::ActionKeyTraits::key(action); } catch (...) { - return failedCompletion(std::current_exception()); + state->setException(std::current_exception()); + return pending; } - } - if constexpr (kShared && ::morph::model::detail::ResultKeyed) { + auto sharedAction = std::make_shared(std::move(action)); + bridgePtr->template attachHandlerAsync( + binding, std::move(key), + [bridgePtr, binding, sharedAction, state, guiExec = _guiExec](std::exception_ptr err) { + if (err) { + state->setException(err); + return; + } + bridgePtr->template executeVia(binding, std::move(*sharedAction), guiExec) + .then([state](R value) { state->setValue(std::move(value)); }) + .onError([state](std::exception_ptr exc) { state->setException(exc); }); + }); + return pending; + } else if constexpr (kShared && ::morph::model::detail::ResultKeyed) { // The action *creates* the instance and its result carries the // generated key, exactly as a database insert returns its primary // key. Adopt it before any user callback observes the result, so a @@ -1475,18 +2020,30 @@ class BridgeHandler { // exists. Give the handler an anonymous instance to run on, then // promote *that* instance once the reply names it — re-pointing to a // fresh one instead would strand whatever the create just did. - try { - _bridge.ensureBound(_binding); - } catch (...) { - return failedCompletion(std::current_exception()); - } + // Same async/fallback contract as the payload-keyed branch above, + // via Bridge::ensureBoundAsync. + auto state = std::make_shared<::morph::async::detail::CompletionState>(); + ::morph::async::Completion pending{state, _guiExec}; auto* const bridgePtr = &_bridge; auto binding = _binding; - return _bridge.template executeVia( - _binding, std::move(action), _guiExec, [bridgePtr, binding](const R& result) { - bridgePtr->template assignHandlerPrimary( - binding, ::morph::model::ActionKeyTraits::template keyOfResult(result)); + auto sharedAction = std::make_shared(std::move(action)); + bridgePtr->ensureBoundAsync( + binding, [bridgePtr, binding, sharedAction, state, guiExec = _guiExec](std::exception_ptr err) { + if (err) { + state->setException(err); + return; + } + bridgePtr + ->template executeVia( + binding, std::move(*sharedAction), guiExec, + [bridgePtr, binding](const R& result) { + bridgePtr->template assignHandlerPrimary( + binding, ::morph::model::ActionKeyTraits::template keyOfResult(result)); + }) + .then([state](R value) { state->setValue(std::move(value)); }) + .onError([state](std::exception_ptr exc) { state->setException(exc); }); }); + return pending; } else { return _bridge.template executeVia(_binding, std::move(action), _guiExec); } @@ -1664,23 +2221,6 @@ class BridgeHandler { [[nodiscard]] const std::shared_ptr& binding() const { return _binding; } private: - /// @brief Builds an already-failed `Completion`, resolved via `.onError(...)`. - /// - /// Used to turn a synchronous exception from the attach/promote step of - /// `execute()` into the same asynchronous failure shape every other - /// dispatch error takes, instead of letting it escape `execute()` as a - /// thrown exception. - /// @tparam R Result type of the action that failed to attach/promote. - /// @param exc Exception to deliver through `.onError(...)`. - /// @return A `Completion` already resolved with @p exc. - template - ::morph::async::Completion failedCompletion(std::exception_ptr exc) { - auto state = std::make_shared<::morph::async::detail::CompletionState>(); - ::morph::async::Completion comp{state, _guiExec}; - state->setException(std::move(exc)); - return comp; - } - Bridge& _bridge; std::weak_ptr _bridgeAlive; // expires when _bridge is destroyed ::morph::exec::IExecutor* _guiExec; diff --git a/include/morph/core/registry.hpp b/include/morph/core/registry.hpp index 51ea3be0..85dec65c 100644 --- a/include/morph/core/registry.hpp +++ b/include/morph/core/registry.hpp @@ -191,6 +191,32 @@ struct ParseError : std::runtime_error { using std::runtime_error::runtime_error; }; +/// @brief Write options for an action's/result's own JSON body: identical to +/// `morph::wire::detail::EscapingWriteOpts`, applied one layer down. +/// +/// The envelope codec already escapes ASCII control bytes (see +/// `docs/spec/core/wire.md`, "Control bytes in string fields"); the action and +/// result bodies it carries are serialized here, separately, and need exactly +/// the same treatment for exactly the same two reasons. With the option off, +/// a raw `0x00`–`0x1F` in any caller-supplied string field of an action makes +/// the body invalid JSON that the peer's own reader rejects — and, when the +/// same string also contains an escaped character, glaze's chunked fast path +/// silently rewrites such a byte as two `0x00`s, destroying the payload in a +/// way that still decodes. Action bodies are pure caller data (a paste's +/// content, a message, a filename), so this is if anything more exposed than +/// the envelope was. +/// +/// Deliberately duplicated rather than reused from `morph::wire`: the action +/// codec belongs to the model layer and must not acquire a dependency on the +/// transport layer's header just to share a four-line option struct. +/// +/// Applies to writing only — glaze's reader already accepts `\\uXXXX`. +struct EscapingWriteOpts : glz::opts { + /// @brief Emit control bytes as `\\uXXXX` rather than raw. + // NOLINTNEXTLINE(readability-identifier-naming) — the name is glaze's, not ours; the option is matched by name. + bool escape_control_characters = true; +}; + // Forward declarations so instance() methods inside the classes can reference them. inline class ActionDispatcher& defaultDispatcher(); inline class ModelRegistryFactory& defaultRegistry(); @@ -698,7 +724,11 @@ bool registerActionExecutorOnce(std::string_view modelId, std::string_view actio static constexpr ::morph::model::Loggable loggable = (LOGGABLE); \ static std::string toJson(const A& action) { \ std::string out; \ - if (auto errCode = glz::write_json(action, out)) { \ + /* EscapingWriteOpts, not write_json: a raw control byte in any */ \ + /* caller-supplied string field would otherwise produce a body the */ \ + /* peer's reader rejects, or be silently mangled by glaze's chunked */ \ + /* fast path — see its doc comment in registry.hpp. */ \ + if (auto errCode = glz::write<::morph::model::detail::EscapingWriteOpts{}>(action, out)) { \ throw morph::model::detail::ParseError{glz::format_error(errCode, out)}; \ } \ return out; \ @@ -716,7 +746,10 @@ bool registerActionExecutorOnce(std::string_view modelId, std::string_view actio } \ static std::string resultToJson(const Result& result) { \ std::string out; \ - if (auto errCode = glz::write_json(result, out)) { \ + /* EscapingWriteOpts: see toJson() above — a result body carries */ \ + /* caller data back (a paste's content, a fetched record) and needs */ \ + /* the identical treatment. */ \ + if (auto errCode = glz::write<::morph::model::detail::EscapingWriteOpts{}>(result, out)) { \ throw morph::model::detail::ParseError{glz::format_error(errCode, out)}; \ } \ return out; \ diff --git a/include/morph/core/remote.hpp b/include/morph/core/remote.hpp index b283c080..31c05432 100644 --- a/include/morph/core/remote.hpp +++ b/include/morph/core/remote.hpp @@ -8,14 +8,12 @@ #include #include #include -#include #include #include #include #include #include #include -#include #include #include #include @@ -26,6 +24,7 @@ #include "backend.hpp" #include "logger.hpp" #include "observability.hpp" +#include "timeout_scheduler.hpp" #include "wire.hpp" namespace morph::backend { @@ -57,115 +56,6 @@ struct LimitPolicy { namespace detail { -/// @brief Background scheduler that invokes a callback once after a delay, unless cancelled first. -/// -/// `RemoteServer` is transport-agnostic and its `IExecutor` has no delayed-post -/// primitive, so a single dedicated thread per instance tracks pending -/// deadlines and fires callbacks when they elapse. Used to enforce -/// `LimitPolicy::executeTimeout` — see `docs/spec/core/backend.md`. -class TimeoutScheduler { -public: - /// @brief Opaque identifier for one scheduled callback. - using Handle = std::uint64_t; - - /// @brief Starts the background thread. - TimeoutScheduler() : _thread{[this] { run(); }} {} - - /// @brief Stops the background thread and joins it. - ~TimeoutScheduler() { - { - std::scoped_lock const lock{_mtx}; - _stop = true; - } - _cv.notify_all(); - _thread.join(); - } - - TimeoutScheduler(const TimeoutScheduler&) = delete; - TimeoutScheduler& operator=(const TimeoutScheduler&) = delete; - TimeoutScheduler(TimeoutScheduler&&) = delete; - TimeoutScheduler& operator=(TimeoutScheduler&&) = delete; - - /// @brief Schedules @p callback to run after @p delay on the scheduler's - /// background thread, unless cancelled first via `cancel()`. - /// @param delay Time to wait before firing. - /// @param callback Invoked on the scheduler thread if not cancelled in time. - /// Exceptions it throws are logged and swallowed. - /// @return Handle usable with `cancel()`. - Handle schedule(std::chrono::milliseconds delay, std::function callback) { - auto const deadline = std::chrono::steady_clock::now() + delay; - std::scoped_lock const lock{_mtx}; - Handle const handle = ++_nextHandle; - auto iter = _entries.emplace(deadline, Entry{handle, std::move(callback)}); - _index[handle] = iter; - _cv.notify_all(); - return handle; - } - - /// @brief Cancels a previously scheduled callback immediately. - /// - /// If @p handle has not fired yet, its entry (and anything its callback - /// captured) is erased right away — the caller does not have to wait for - /// the original deadline for that memory to be released. A no-op if - /// @p handle already fired or was already cancelled. - /// @param handle Handle returned by a prior `schedule()` call. - void cancel(Handle handle) { - std::scoped_lock const lock{_mtx}; - auto found = _index.find(handle); - if (found == _index.end()) { - return; - } - _entries.erase(found->second); - _index.erase(found); - } - -private: - struct Entry { - Handle handle; - std::function callback; - }; - - void run() { - std::unique_lock lock{_mtx}; - while (!_stop) { - if (_entries.empty()) { - _cv.wait(lock); - continue; - } - auto const nextDeadline = _entries.begin()->first; - _cv.wait_until(lock, nextDeadline); - if (_stop) { - break; - } - auto now = std::chrono::steady_clock::now(); - while (!_entries.empty() && _entries.begin()->first <= now) { - auto iter = _entries.begin(); - Entry entry = std::move(iter->second); - _index.erase(entry.handle); - _entries.erase(iter); - lock.unlock(); - try { - entry.callback(); - } catch (const std::exception& exc) { - ::morph::log::logError("[timeout-scheduler] callback threw: " + std::string{exc.what()}); - } catch (...) { - ::morph::log::logError("[timeout-scheduler] callback threw unknown exception"); - } - lock.lock(); - now = std::chrono::steady_clock::now(); - } - } - } - - std::mutex _mtx; - std::condition_variable _cv; - std::multimap _entries; - std::unordered_map::iterator> _index; - Handle _nextHandle{0}; - bool _stop{false}; - std::thread _thread; -}; - /// @brief Keyed 64-bit bijection that turns a monotonic counter into an /// unguessable, non-sequential id. /// @@ -487,7 +377,7 @@ class RemoteServer : public std::enable_shared_from_this { std::scoped_lock const lock{_limitsMtx}; _limits = policy; if (_limits.executeTimeout.count() > 0 && !_timeoutScheduler) { - _timeoutScheduler = std::make_unique(); + _timeoutScheduler = std::make_unique<::morph::async::detail::TimeoutScheduler>(); } } @@ -1290,7 +1180,7 @@ class RemoteServer : public std::enable_shared_from_this { } }; - detail::TimeoutScheduler::Handle timeoutHandle{}; + ::morph::async::detail::TimeoutScheduler::Handle timeoutHandle{}; if (limits.executeTimeout.count() > 0) { std::scoped_lock const lock{_limitsMtx}; if (_timeoutScheduler) { @@ -1452,7 +1342,7 @@ class RemoteServer : public std::enable_shared_from_this { // firing first. Shared by the executeInFlight metric, health()'s inFlight // field, and drainedWithin(): one counter, never double-counted. std::atomic _inFlightExecutes{0}; - std::unique_ptr _timeoutScheduler; + std::unique_ptr<::morph::async::detail::TimeoutScheduler> _timeoutScheduler; // Set once by beginShutdown() and never cleared — there is no // un-shutdown. Checked at the top of dispatchMessage() for register and // execute envelopes only; deregister and any other kind are unaffected. diff --git a/include/morph/core/timeout_scheduler.hpp b/include/morph/core/timeout_scheduler.hpp new file mode 100644 index 00000000..35cef6d9 --- /dev/null +++ b/include/morph/core/timeout_scheduler.hpp @@ -0,0 +1,312 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once +#include +#include +#include +#include +#include +#include + +/// @file +/// `TimeoutScheduler` — "run this callback once, in N milliseconds, unless +/// cancelled first" — in two builds of the same public API. +/// +/// @par Why two builds +/// The ordinary build owns a dedicated `std::thread`. A **single-threaded +/// Emscripten** build cannot: Qt for WebAssembly is installed here as +/// `wasm_singlethread` (`.github/workflows/wasm-ladder.yml`) and +/// `cmake/morph_add_rung.cmake` passes no `-pthread`, so Emscripten's +/// non-pthread `pthread_create` stub fails and `std::thread`'s constructor +/// throws `std::system_error` ("thread constructor failed") — from inside +/// whatever completion callback happened to enable the deadline. Every WASM +/// client in this repository is Qt-event-loop driven and would hit this the +/// moment it called `Bridge::setExecuteDeadline` (which +/// `examples/common/gui/event_poller.hpp`'s constructor does +/// unconditionally, on every poll open). +/// +/// So under `__EMSCRIPTEN__` without `__EMSCRIPTEN_PTHREADS__` this class is +/// built on `emscripten_async_call` — the browser's own `setTimeout` — and +/// fires its callbacks on the single main thread, i.e. on the same thread the +/// Qt event loop and every `QtExecutor`-posted completion callback already +/// run on. Deadlines still fire; nothing is silently disabled. +/// +/// @par What differs between the two builds +/// - **Callback thread.** Threaded build: a private background thread, so a +/// callback must be prepared to run concurrently with the caller (the one +/// real callback in this codebase, `executeVia`'s, only touches a +/// `CompletionState`, which is itself mutex-guarded). Browser build: the +/// main thread, never concurrently with anything. +/// - **Cancellation.** Threaded build: the entry, its callback and everything +/// the callback captured are erased immediately. Browser build: identical +/// for the callback and its captures (the map entry is erased at once), but +/// the underlying browser timer is not itself cleared — it still fires at +/// its original deadline and finds nothing to do. Only a small ticket +/// allocation outlives `cancel()`, until that point. +/// - **Destruction.** Threaded build: the destructor joins its thread, so no +/// callback can be in flight afterwards. Browser build: nothing to join; +/// pending browser timers observe an expired `std::weak_ptr` to the +/// scheduler's state and return without invoking anything. +/// +/// @warning The browser build has never been compiled or run in this +/// repository — no Emscripten toolchain is available where it was written. +/// Its only verification is the `ladder-wasm` CI compile gate. Stated plainly +/// here rather than smoothed over, exactly like `examples/TESTING.md`'s note +/// on the WASM clients themselves. + +#if defined(__EMSCRIPTEN__) && !defined(__EMSCRIPTEN_PTHREADS__) +#define MORPH_TIMEOUT_SCHEDULER_BROWSER_TIMERS 1 +#include + +#include +#include +#include +#else +#include +#include +#include +#include +#endif + +#include "logger.hpp" + +namespace morph::async::detail { + +#ifndef MORPH_TIMEOUT_SCHEDULER_BROWSER_TIMERS + +/// @brief Background scheduler that invokes a callback once after a delay, unless cancelled first. +/// +/// Neither `Bridge` nor `RemoteServer` is bound to a specific `IExecutor` +/// with a delayed-post primitive, so a single dedicated thread per instance +/// tracks pending deadlines and fires callbacks when they elapse. Used by +/// `RemoteServer` to enforce `LimitPolicy::executeTimeout` (server-side — +/// see `docs/spec/core/backend.md`) and by `Bridge::setExecuteDeadline` +/// (client-side — see `docs/spec/core/completion.md`). See this file's `@file` +/// comment for the single-threaded-WASM build of the same API. +class TimeoutScheduler { +public: + /// @brief Opaque identifier for one scheduled callback. + using Handle = std::uint64_t; + + /// @brief Starts the background thread. + TimeoutScheduler() : _thread{[this] { run(); }} {} + + /// @brief Stops the background thread and joins it. + ~TimeoutScheduler() { + { + std::scoped_lock const lock{_mtx}; + _stop = true; + } + _cv.notify_all(); + _thread.join(); + } + + TimeoutScheduler(const TimeoutScheduler&) = delete; + TimeoutScheduler& operator=(const TimeoutScheduler&) = delete; + TimeoutScheduler(TimeoutScheduler&&) = delete; + TimeoutScheduler& operator=(TimeoutScheduler&&) = delete; + + /// @brief Schedules @p callback to run after @p delay on the scheduler's + /// background thread, unless cancelled first via `cancel()`. + /// @param delay Time to wait before firing. + /// @param callback Invoked on the scheduler thread if not cancelled in time. + /// Exceptions it throws are logged and swallowed. + /// @return Handle usable with `cancel()`. + Handle schedule(std::chrono::milliseconds delay, std::function callback) { + auto const deadline = std::chrono::steady_clock::now() + delay; + std::scoped_lock const lock{_mtx}; + Handle const handle = ++_nextHandle; + auto iter = _entries.emplace(deadline, Entry{handle, std::move(callback)}); + _index[handle] = iter; + _cv.notify_all(); + return handle; + } + + /// @brief Cancels a previously scheduled callback immediately. + /// + /// If @p handle has not fired yet, its entry (and anything its callback + /// captured) is erased right away — the caller does not have to wait for + /// the original deadline for that memory to be released. A no-op if + /// @p handle already fired or was already cancelled. + /// @param handle Handle returned by a prior `schedule()` call. + void cancel(Handle handle) { + std::scoped_lock const lock{_mtx}; + auto found = _index.find(handle); + if (found == _index.end()) { + return; + } + _entries.erase(found->second); + _index.erase(found); + } + +private: + struct Entry { + Handle handle; + std::function callback; + }; + + void run() { + std::unique_lock lock{_mtx}; + while (!_stop) { + if (_entries.empty()) { + _cv.wait(lock); + continue; + } + auto const nextDeadline = _entries.begin()->first; + _cv.wait_until(lock, nextDeadline); + if (_stop) { + break; + } + auto now = std::chrono::steady_clock::now(); + while (!_entries.empty() && _entries.begin()->first <= now) { + auto iter = _entries.begin(); + Entry entry = std::move(iter->second); + _index.erase(entry.handle); + _entries.erase(iter); + lock.unlock(); + try { + entry.callback(); + } catch (const std::exception& exc) { + ::morph::log::logError("[timeout-scheduler] callback threw: " + std::string{exc.what()}); + } catch (...) { + ::morph::log::logError("[timeout-scheduler] callback threw unknown exception"); + } + lock.lock(); + now = std::chrono::steady_clock::now(); + } + } + } + + std::mutex _mtx; + std::condition_variable _cv; + std::multimap _entries; + std::unordered_map::iterator> _index; + Handle _nextHandle{0}; + bool _stop{false}; + std::thread _thread; +}; + +#else + +/// @brief Single-threaded-Emscripten build of the same API, backed by the +/// browser's `setTimeout` (`emscripten_async_call`) instead of a +/// thread. See this file's `@file` comment for why it exists and +/// exactly how its behaviour differs. +class TimeoutScheduler { +public: + /// @brief Opaque identifier for one scheduled callback. + using Handle = std::uint64_t; + + /// @brief Creates the scheduler. Starts no thread — there is none to start. + TimeoutScheduler() = default; + + /// @brief Drops every still-pending callback without firing it. + /// + /// Browser timers already queued outlive this object; each holds only a + /// `std::weak_ptr` to `_state` and returns immediately once it expires, + /// which is precisely at this destructor. Matches the threaded build's + /// "`~TimeoutScheduler` drops pending entries without firing them". + ~TimeoutScheduler() = default; + + TimeoutScheduler(const TimeoutScheduler&) = delete; + TimeoutScheduler& operator=(const TimeoutScheduler&) = delete; + TimeoutScheduler(TimeoutScheduler&&) = delete; + TimeoutScheduler& operator=(TimeoutScheduler&&) = delete; + + /// @brief Schedules @p callback to run after @p delay on the main + /// (browser) thread, unless cancelled first via `cancel()`. + /// @param delay Time to wait before firing. + /// @param callback Invoked on the main thread if not cancelled in time. + /// Exceptions it throws are logged and swallowed. + /// @return Handle usable with `cancel()`. + Handle schedule(std::chrono::milliseconds delay, std::function callback) { + Handle const handle = ++_state->nextHandle; + _state->pending.emplace(handle, std::move(callback)); + // Owned by the browser timer, deleted by `fire` below whether or not + // the entry is still live by then. A raw `new` rather than a + // `unique_ptr` because the ownership genuinely crosses a C callback + // boundary that cannot carry a smart pointer. + auto* ticket = new Ticket{_state, handle}; + ::emscripten_async_call(&TimeoutScheduler::fire, ticket, clampMillis(delay)); + return handle; + } + + /// @brief Cancels a previously scheduled callback immediately. + /// + /// If @p handle has not fired yet, its callback (and anything that + /// callback captured) is released right away, exactly like the threaded + /// build. The browser timer itself is left to elapse and find nothing — + /// see the `@file` comment. A no-op if @p handle already fired or was + /// already cancelled. + /// @param handle Handle returned by a prior `schedule()` call. + void cancel(Handle handle) { _state->pending.erase(handle); } + +private: + struct State { + std::unordered_map> pending; + Handle nextHandle{0}; + }; + + struct Ticket { + std::weak_ptr state; + Handle handle; + }; + + /// @brief @p delay as the `int` milliseconds `emscripten_async_call` + /// takes, saturating rather than wrapping (a `std::chrono` + /// duration can hold far more than an `int` can). + /// + /// @note This is a real, documented behavioural asymmetry from the + /// threaded build, which honours the full `std::chrono::milliseconds` + /// range unconditionally: a delay beyond `INT_MAX` ms (~24.85 days) fires + /// at ~24.85 days here instead of at its true, much later requested time. + /// `emscripten_async_call`'s `int` parameter is a hard platform + /// constraint with no larger-range alternative to fall back to, so this + /// is accepted rather than worked around. No caller in this codebase + /// currently requests a deadline anywhere near that range. + /// @param delay The requested delay. + /// @return A non-negative millisecond count that fits in an `int`. + [[nodiscard]] static int clampMillis(std::chrono::milliseconds delay) noexcept { + auto const count = delay.count(); + if (count <= 0) { + return 0; + } + if (count > static_cast(std::numeric_limits::max())) { + return std::numeric_limits::max(); + } + return static_cast(count); + } + + /// @brief The C callback the browser timer invokes. + /// @param arg The `Ticket*` handed to `emscripten_async_call`; always + /// deleted here, whether or not its entry is still live. + static void fire(void* arg) { + std::unique_ptr const ticket{static_cast(arg)}; + auto state = ticket->state.lock(); + if (!state) { + return; + } + auto found = state->pending.find(ticket->handle); + if (found == state->pending.end()) { + return; // cancelled before this timer elapsed + } + std::function callback = std::move(found->second); + state->pending.erase(found); + try { + callback(); + } catch (const std::exception& exc) { + ::morph::log::logError("[timeout-scheduler] callback threw: " + std::string{exc.what()}); + } catch (...) { + ::morph::log::logError("[timeout-scheduler] callback threw unknown exception"); + } + } + + /// @brief Held by `shared_ptr` so a browser timer that outlives this + /// object detects that fact instead of writing to freed storage — + /// the same weak-token pattern as `morph::bridge::Bridge::_liveness`. + std::shared_ptr _state{std::make_shared()}; +}; + +#endif + +} // namespace morph::async::detail diff --git a/include/morph/forms/forms.hpp b/include/morph/forms/forms.hpp index 2feaedb3..c8905981 100644 --- a/include/morph/forms/forms.hpp +++ b/include/morph/forms/forms.hpp @@ -621,8 +621,8 @@ using LiteralString = ::morph::detail::FixedString; template inline constexpr bool isLiteralString = false; -/// @brief `isLiteralString` specialization recognising `LiteralString`. -/// @tparam N Literal length of the recognised `LiteralString`. +/// @brief `isLiteralString` specialization recognising `LiteralString`, +/// where `N` is the recognised literal's length. template inline constexpr bool isLiteralString> = true; diff --git a/include/morph/qt/qt_websocket_backend.hpp b/include/morph/qt/qt_websocket_backend.hpp index e5bdfab5..ad5d9dba 100644 --- a/include/morph/qt/qt_websocket_backend.hpp +++ b/include/morph/qt/qt_websocket_backend.hpp @@ -102,9 +102,11 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { /// @param serverUrl `ws://` or `wss://` URL of the remote `RemoteServer`. /// @param dispatcher Action dispatcher (defaults to the process-level singleton). /// @param registry Model registry (defaults to the process-level singleton). +#ifndef QT_NO_SSL /// @param tls If non-null, enables TLS and applies this configuration. Not /// declared at all on an SSL-less Qt build (`QT_NO_SSL`) — see /// the class doc comment's "SSL-less Qt builds" section. +#endif /// @param cfg Reconnect tuning. Default: enabled, 500ms initial / 30s cap, 2x backoff. explicit QtWebSocketBackend( QUrl serverUrl, @@ -204,6 +206,31 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { const std::string& typeId, std::function()> factory, ::morph::backend::detail::InstanceIdentity identity) override; + /// @brief Sends a shared (register-or-attach) `register` and, if async + /// registration is enabled, returns without blocking. + /// + /// The non-blocking counterpart to `registerModelShared`, matching + /// `registerModelAsync`'s shape exactly (same `callId` counter, same + /// `_pendingRegistrations` map, same verb-agnostic reply routing in + /// `onTextMessage`). An empty `identity.primary` degrades to the private + /// path, i.e. to `registerModelAsync`, mirroring the synchronous + /// `registerModelShared`'s own degrade-to-private behaviour. + /// + /// @param typeId String type-id of the model. + /// @param factory Ignored — model construction is delegated to the server. + /// @param identity Entity key for the action log plus the directory primary key. + /// @param onRegistered Invoked with the assigned `ModelId` on success. + /// @param onError Invoked with a diagnostic message on failure. + /// @return `true` if `asyncRegistrationEnabled` is set (see + /// `QtWebSocketBackendConfig`) and the request was sent; + /// `false` otherwise, falling back to the synchronous + /// `registerModelShared`. + bool registerModelSharedAsync(const std::string& typeId, + std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity, + std::function onRegistered, + std::function onError) override; + /// @brief Sends an `attach` and blocks for the reply, re-pointing from @p current. /// @param typeId String type-id of the model. /// @param factory Ignored — model construction is delegated to the server. @@ -215,6 +242,30 @@ class QtWebSocketBackend : public ::morph::backend::detail::IBackend { const std::string& typeId, std::function()> factory, ::morph::backend::detail::InstanceIdentity identity, ::morph::exec::detail::ModelId current) override; + /// @brief Sends an `attach` and, if async registration is enabled, + /// returns without blocking. + /// + /// The non-blocking counterpart to `attachModel`; see + /// `registerModelSharedAsync` immediately above for the shared shape. An + /// empty `identity.primary` releases @p current and degrades to a private + /// async registration, mirroring the synchronous `attachModel`'s own + /// empty-primary branch. + /// + /// @param typeId String type-id of the model. + /// @param factory Ignored — model construction is delegated to the server. + /// @param identity Entity key for the action log plus the directory primary key. + /// @param current Instance currently held, or `ModelId{0}` if none. + /// @param onRegistered Invoked with the `ModelId` now attached to, on success. + /// @param onError Invoked with a diagnostic message on failure. + /// @return `true` if `asyncRegistrationEnabled` is set and the request + /// was sent; `false` otherwise, falling back to the synchronous + /// `attachModel`. + bool attachModelAsync(const std::string& typeId, + std::function()> factory, + ::morph::backend::detail::InstanceIdentity identity, ::morph::exec::detail::ModelId current, + std::function onRegistered, + std::function onError) override; + /// @brief Files a live server-side instance under @p primary. /// @param mid Live instance to promote. /// @param typeId Model type id. diff --git a/include/morph/util/quantity.hpp b/include/morph/util/quantity.hpp index e49f883b..9ad0ee0d 100644 --- a/include/morph/util/quantity.hpp +++ b/include/morph/util/quantity.hpp @@ -1030,6 +1030,36 @@ struct NamedQuantity : Quantity { [[nodiscard]] static NamedQuantity fromDouble(double raw) { return NamedQuantity{Base::fromDouble(raw)}; } }; +/// @brief Renders @p quantity as value + unit (`5.2kW`, `N/A%`) — the same +/// text `std::format("{}", quantity)` produces via the `std::formatter` +/// specialization just below, exposed as a plain function so a caller +/// can render a `Quantity` without going through `std::format` itself. +/// +/// Exists because Emscripten's bundled libc++ (older than the Linux/Windows +/// standard library this project otherwise builds against — see +/// `.github/workflows/wasm-ladder.yml`'s `EMSDK_VERSION`) has a known +/// limitation recognising `std::formatter` partial specializations +/// parameterized over an `auto` non-type template parameter (`U` here) for +/// `std::format`'s compile-time formattability check — `std::format("{}", +/// someQuantity)` fails to compile there with "the supplied type is not +/// formattable" even though the specialization is valid and the identical +/// call compiles and runs correctly on every other toolchain this project +/// targets. `toString()` bypasses that check entirely: it calls the same +/// underlying logic directly instead of through `std::format`'s trait +/// machinery, so it works identically everywhere, WASM included. +/// @tparam U Unit enumerator. +/// @tparam Dec Declared decimals. +/// @param quantity The value to render. +/// @return The formatted text. +template +[[nodiscard]] inline std::string toString(const Quantity& quantity) { + constexpr auto display = UnitTraits::meta(U).display; + if (quantity.value()) { + return detail::formatRationalDecimal(*quantity.value()) + std::string{display}; + } + return "N/A" + std::string{display}; +} + } // namespace morph::units #if MORPH_QUANTITY_PROVENANCE @@ -1037,6 +1067,7 @@ struct NamedQuantity : Quantity { #endif /// @brief Renders value + unit (`5.2kW`, `N/A%`); no `operator<<` is provided. +/// Delegates to `morph::units::toString` so the two never drift. /// @tparam U Unit enumerator. /// @tparam Dec Declared decimals. template @@ -1051,12 +1082,7 @@ struct std::formatter> { /// @param ctx Format context. /// @return Output iterator past the written text. auto format(const morph::units::Quantity& quantity, std::format_context& ctx) const { - constexpr auto display = morph::units::UnitTraits::meta(U).display; - if (quantity.value()) { - return std::format_to(ctx.out(), "{}{}", morph::units::detail::formatRationalDecimal(*quantity.value()), - display); - } - return std::format_to(ctx.out(), "N/A{}", display); + return std::format_to(ctx.out(), "{}", morph::units::toString(quantity)); } }; diff --git a/src/qt/qt_websocket_backend.cpp b/src/qt/qt_websocket_backend.cpp index 0fa25abc..850222e5 100644 --- a/src/qt/qt_websocket_backend.cpp +++ b/src/qt/qt_websocket_backend.cpp @@ -190,14 +190,22 @@ void QtWebSocketBackend::sendRegisterAsync(const std::string& typeId, std::strin std::function onRegistered, std::function onError) { uint64_t const callId = ++_nextCallId; + auto env = ::morph::wire::makeRegister(typeId, std::string{contextKey}); + env.callId = callId; + env.session = _session; + // Encoded before the map insertion below: wire::encode() can throw on + // serialization failure, and a throw after inserting would leave this + // callId's onRegistered/onError parked in _pendingRegistrations forever, + // waiting for a reply to a message that was never sent -- nothing erases + // an entry whose send never happened. Encoding first means a throw here + // propagates to the caller (Bridge::registerHandlerImpl et al. already + // handle it) with nothing to clean up. + auto const encoded = QString::fromStdString(::morph::wire::encode(env)); { 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; - env.session = _session; - _socket.sendTextMessage(QString::fromStdString(::morph::wire::encode(env))); + _socket.sendTextMessage(encoded); } void QtWebSocketBackend::flushQueuedRegistrations() { @@ -211,6 +219,77 @@ void QtWebSocketBackend::flushQueuedRegistrations() { } } +bool QtWebSocketBackend::registerModelSharedAsync( + const std::string& typeId, std::function()> /*factory*/, + ::morph::backend::detail::InstanceIdentity identity, + std::function onRegistered, + std::function onError) { + if (!_cfg.asyncRegistrationEnabled) { + return false; + } + if (identity.primary.empty()) { + // Degrades to the private (non-shared) path, exactly like the + // synchronous registerModelShared below -- and that path already + // has an async form: this class's own registerModelAsync. + return registerModelAsync(typeId, nullptr, identity.contextKey, std::move(onRegistered), std::move(onError)); + } + if (!_connected) { + onError("disconnected"); + return true; + } + uint64_t const callId = ++_nextCallId; + auto env = + ::morph::wire::makeRegisterShared(typeId, std::string{identity.primary}, std::string{identity.contextKey}); + env.callId = callId; + // See registerModelAsync's identical comment: encoded before the map + // insertion, so a throwing encode() cannot orphan a pending entry. + auto const encoded = QString::fromStdString(::morph::wire::encode(env)); + { + std::scoped_lock const lock{_pendingMtx}; + _pendingRegistrations[callId] = + PendingRegistration{.onRegistered = std::move(onRegistered), .onError = std::move(onError)}; + } + _socket.sendTextMessage(encoded); + return true; +} + +bool QtWebSocketBackend::attachModelAsync( + const std::string& typeId, std::function()> /*factory*/, + ::morph::backend::detail::InstanceIdentity identity, ::morph::exec::detail::ModelId current, + std::function onRegistered, + std::function onError) { + if (!_cfg.asyncRegistrationEnabled) { + return false; + } + if (identity.primary.empty()) { + // Mirrors the synchronous attachModel's empty-primary branch: release + // the current instance (fire-and-forget, as deregisterModel already + // is) and degrade to a private async registration. + if (current.v != 0U) { + deregisterModel(current); + } + return registerModelAsync(typeId, nullptr, identity.contextKey, std::move(onRegistered), std::move(onError)); + } + if (!_connected) { + onError("disconnected"); + return true; + } + uint64_t const callId = ++_nextCallId; + auto env = + ::morph::wire::makeAttach(typeId, std::string{identity.primary}, current.v, std::string{identity.contextKey}); + env.callId = callId; + // See registerModelAsync's identical comment: encoded before the map + // insertion, so a throwing encode() cannot orphan a pending entry. + auto const encoded = QString::fromStdString(::morph::wire::encode(env)); + { + std::scoped_lock const lock{_pendingMtx}; + _pendingRegistrations[callId] = + PendingRegistration{.onRegistered = std::move(onRegistered), .onError = std::move(onError)}; + } + _socket.sendTextMessage(encoded); + return true; +} + ::morph::wire::ProtocolNegotiationResult QtWebSocketBackend::negotiateProtocolVersion() { std::string replyJson; try { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 17e22017..74243c72 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -31,6 +31,8 @@ add_executable(morph_tests test_action_validation.cpp test_security_fixes.cpp test_bridge_lifetime.cpp + test_client_execute_deadline.cpp + test_timeout_scheduler.cpp test_dispatch_di.cpp test_handler_binding.cpp test_switch_backend.cpp diff --git a/tests/qt/qt_test_server_main.cpp b/tests/qt/qt_test_server_main.cpp index bfaf334a..eaedda36 100644 --- a/tests/qt/qt_test_server_main.cpp +++ b/tests/qt/qt_test_server_main.cpp @@ -70,10 +70,10 @@ int main(int argc, char* argv[]) { std::cout.flush(); // Watch stdin for a "quit" command so the test can shut us down cleanly. -#ifdef _MSC_VER - // MSVC flags the POSIX `fileno` name itself (C4996) even though the - // symbol it names is the one actually deprecated; `_fileno` is its - // ISO-conformant replacement on this toolchain only. +#ifdef _WIN32 + // MSVC (and other Windows toolchains) flag the POSIX `fileno` name itself + // (C4996) even though the symbol it names is the one actually deprecated; + // `_fileno` is the ISO-conformant replacement on Windows. auto* stdinWatcher = new QSocketNotifier(_fileno(stdin), QSocketNotifier::Read, &app); #else auto* stdinWatcher = new QSocketNotifier(fileno(stdin), QSocketNotifier::Read, &app); diff --git a/tests/qt/test_qt_websocket.cpp b/tests/qt/test_qt_websocket.cpp index 717f86d5..88f29d48 100644 --- a/tests/qt/test_qt_websocket.cpp +++ b/tests/qt/test_qt_websocket.cpp @@ -287,6 +287,156 @@ TEST_CASE( CHECK(binding->currentId.load() == 0U); // onRegistered never fired; still safely unbound } +// ── The shared/keyed async wire methods ────────────────────────────────────── +// Same opt-in gate and same callId-keyed reply routing as registerModelAsync +// above; these drive them against a real RemoteServer, end to end. + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("morph::qt::QtWebSocketBackend: registerModelSharedAsync registers-or-attaches without blocking", + "[qt][ws][issue26][shared-instances]") { + 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())}; + morph::qt::QtWebSocketBackend backend{url, morph::model::detail::defaultDispatcher(), + morph::model::detail::defaultRegistry(), std::nullopt, + morph::qt::QtWebSocketBackend::Config{.asyncRegistrationEnabled = true}}; + REQUIRE(backend.waitForConnected()); + + std::atomic registered{0}; + std::string failure; + REQUIRE(backend.registerModelSharedAsync( + "WsEchoModel", nullptr, {.contextKey = "acct-1", .primary = "acct-1"}, + [&](morph::exec::detail::ModelId mid) { registered.store(mid.v); }, + [&](const std::string& message) { failure = message; })); + + // Returned true without waiting for the reply: nothing has arrived yet. + CHECK(registered.load() == 0U); + + pumpUntil([&] { return registered.load() != 0U || !failure.empty(); }); + CHECK(failure.empty()); + REQUIRE(registered.load() != 0U); + + // It really went out as a *shared* register, not a private one: the key is + // now in the server's instance directory. + auto const keys = backend.listInstances("WsEchoModel"); + REQUIRE(keys.size() == 1); + CHECK(keys.front() == "acct-1"); + + // A second shared register for the same key joins the same instance rather + // than creating a second one -- the register-or-attach half of the name. + std::atomic second{0}; + REQUIRE(backend.registerModelSharedAsync( + "WsEchoModel", nullptr, {.contextKey = "acct-1", .primary = "acct-1"}, + [&](morph::exec::detail::ModelId mid) { second.store(mid.v); }, + [&](const std::string& message) { failure = message; })); + pumpUntil([&] { return second.load() != 0U || !failure.empty(); }); + CHECK(failure.empty()); + CHECK(second.load() == registered.load()); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("morph::qt::QtWebSocketBackend: attachModelAsync joins the existing shared instance without blocking", + "[qt][ws][issue26][shared-instances]") { + 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())}; + morph::qt::QtWebSocketBackend backend{url, morph::model::detail::defaultDispatcher(), + morph::model::detail::defaultRegistry(), std::nullopt, + morph::qt::QtWebSocketBackend::Config{.asyncRegistrationEnabled = true}}; + REQUIRE(backend.waitForConnected()); + + // Seed the directory synchronously, so the async attach below has something + // to join and its reply can be compared against a known id. + auto const seeded = + backend.registerModelShared("WsEchoModel", nullptr, {.contextKey = "acct-7", .primary = "acct-7"}); + REQUIRE(seeded.v != 0U); + + std::atomic attached{0}; + std::string failure; + REQUIRE(backend.attachModelAsync( + "WsEchoModel", nullptr, {.contextKey = "acct-7", .primary = "acct-7"}, morph::exec::detail::ModelId{0}, + [&](morph::exec::detail::ModelId mid) { attached.store(mid.v); }, + [&](const std::string& message) { failure = message; })); + CHECK(attached.load() == 0U); // the reply has not arrived yet + + pumpUntil([&] { return attached.load() != 0U || !failure.empty(); }); + CHECK(failure.empty()); + CHECK(attached.load() == seeded.v); + + // Re-pointing to a different key gets a different instance, still async. + std::atomic repointed{0}; + REQUIRE(backend.attachModelAsync( + "WsEchoModel", nullptr, {.contextKey = "acct-8", .primary = "acct-8"}, + morph::exec::detail::ModelId{attached.load()}, + [&](morph::exec::detail::ModelId mid) { repointed.store(mid.v); }, + [&](const std::string& message) { failure = message; })); + pumpUntil([&] { return repointed.load() != 0U || !failure.empty(); }); + CHECK(failure.empty()); + REQUIRE(repointed.load() != 0U); + CHECK(repointed.load() != seeded.v); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("morph::qt::QtWebSocketBackend: attachModelAsync with an empty primary degrades to a private registration", + "[qt][ws][issue26][shared-instances]") { + 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())}; + morph::qt::QtWebSocketBackend backend{url, morph::model::detail::defaultDispatcher(), + morph::model::detail::defaultRegistry(), std::nullopt, + morph::qt::QtWebSocketBackend::Config{.asyncRegistrationEnabled = true}}; + REQUIRE(backend.waitForConnected()); + + std::atomic registered{0}; + std::string failure; + REQUIRE(backend.attachModelAsync( + "WsEchoModel", nullptr, {.contextKey = "ctx", .primary = ""}, morph::exec::detail::ModelId{0}, + [&](morph::exec::detail::ModelId mid) { registered.store(mid.v); }, + [&](const std::string& message) { failure = message; })); + + pumpUntil([&] { return registered.load() != 0U || !failure.empty(); }); + CHECK(failure.empty()); + REQUIRE(registered.load() != 0U); + // Private, exactly like the synchronous attachModel's own empty-primary + // branch: nothing was filed in the shared directory. + CHECK(backend.listInstances("WsEchoModel").empty()); +} + +TEST_CASE("morph::qt::QtWebSocketBackend: registerModelSharedAsync on a never-connected socket reports onError", + "[qt][ws][issue26][shared-instances][disconnect]") { + ensureApp(); + // Port 1 is reserved and never listening — the socket never reaches Connected. + QUrl url{QString("ws://127.0.0.1:1")}; + morph::qt::QtWebSocketBackend backend{url, morph::model::detail::defaultDispatcher(), + morph::model::detail::defaultRegistry(), std::nullopt, + morph::qt::QtWebSocketBackend::Config{.asyncRegistrationEnabled = true}}; + REQUIRE_FALSE(backend.waitForConnected(200)); + + std::string failure; + std::atomic registered{0}; + // Accepts the request (returns true) and reports the failure through + // onError rather than blocking or throwing. Bridge::ensureBoundAsync + // tolerates this firing inline, from inside the call itself. + REQUIRE(backend.registerModelSharedAsync( + "WsEchoModel", nullptr, {.contextKey = "acct-1", .primary = "acct-1"}, + [&](morph::exec::detail::ModelId mid) { registered.store(mid.v); }, + [&](const std::string& message) { failure = message; })); + CHECK(registered.load() == 0U); + CHECK(failure == "disconnected"); +} + TEST_CASE( "morph::qt::QtWebSocketBackend: registerModelAsync called before the socket connects queues and retries once " "connected fires", diff --git a/tests/test_async_registration.cpp b/tests/test_async_registration.cpp index d4bc10b9..6741dd8b 100644 --- a/tests/test_async_registration.cpp +++ b/tests/test_async_registration.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include "test_support.hpp" @@ -42,6 +43,53 @@ struct ARModel { int execute(const ARCount& a) { return a.x; } }; +// --- Keyed/shared coverage: the same deferred-reply idea applied to +// --- registerModelSharedAsync/attachModelAsync (the register-or-attach and +// --- attach counterparts of registerModelAsync). + +/// Names the instance it wants in the action payload -> payload-keyed, so +/// executing it attaches the handler first (Bridge::attachHandlerAsync). +struct ARTouch { + std::int64_t id = 0; + int amount = 0; +}; + +/// Result of the creating action below; its `id` establishes the key. +struct ARKeyedCreated { + std::int64_t id = 0; + int value = 0; +}; + +/// Creates the entity, so its key can only come back in the reply -> +/// result-keyed, and executing it binds the handler first +/// (Bridge::ensureBoundAsync) and promotes it once the reply names the key. +struct ARKeyedCreate { + int initial = 0; +}; + +/// Payload-keyed, like ARTouch, but its ActionKeyTraits::key() below throws -- +/// exercises execute()'s "key extraction is user code" guard (the try/catch +/// around ActionKeyTraits::key(action), ahead of the attach dispatch). +struct ARThrowingKeyTouch { + int amount = 0; +}; + +struct ARKeyedModel { + int value = 0; + int execute(const ARTouch& act) { + value += act.amount; + return value; + } + ARKeyedCreated execute(const ARKeyedCreate& act) { + value = act.initial; + return {.id = 4242, .value = value}; + } + int execute(const ARThrowingKeyTouch& act) { + value += act.amount; + return value; + } +}; + } // namespace template <> @@ -58,6 +106,59 @@ struct morph::model::ModelTraits { static constexpr std::string_view typeId() { return "AR_Model"; } }; +template <> +struct morph::model::ActionTraits { + using Result = int; + static constexpr std::string_view typeId() { return "AR_Touch"; } + static std::string toJson(const ARTouch& act) { + return R"({"id":)" + std::to_string(act.id) + R"(,"amount":)" + std::to_string(act.amount) + "}"; + } + static ARTouch fromJson(std::string_view /*json*/) { return {}; } + static std::string resultToJson(const int& res) { return std::to_string(res); } + static int resultFromJson(std::string_view text) { return std::stoi(std::string{text}); } +}; +template <> +struct morph::model::ActionTraits { + using Result = ARKeyedCreated; + static constexpr std::string_view typeId() { return "AR_KeyedCreate"; } + static std::string toJson(const ARKeyedCreate& act) { + return R"({"initial":)" + std::to_string(act.initial) + "}"; + } + static ARKeyedCreate fromJson(std::string_view /*json*/) { return {}; } + static std::string resultToJson(const ARKeyedCreated& res) { + return R"({"id":)" + std::to_string(res.id) + R"(,"value":)" + std::to_string(res.value) + "}"; + } + static ARKeyedCreated resultFromJson(std::string_view /*json*/) { return {}; } +}; +template <> +struct morph::model::ModelTraits { + static constexpr std::string_view typeId() { return "AR_KeyedModel"; } +}; + +template <> +struct morph::model::ActionTraits { + using Result = int; + static constexpr std::string_view typeId() { return "AR_ThrowingKeyTouch"; } + static std::string toJson(const ARThrowingKeyTouch& act) { return R"({"amount":)" + std::to_string(act.amount) + "}"; } + static ARThrowingKeyTouch fromJson(std::string_view /*json*/) { return {}; } + static std::string resultToJson(const int& res) { return std::to_string(res); } + static int resultFromJson(std::string_view text) { return std::stoi(std::string{text}); } +}; + +// Written directly rather than via BRIDGE_KEY_FROM: that macro's generated +// key() body cannot be made to throw, which is the entire point of this type. +template <> +struct morph::model::ActionKeyTraits { + static constexpr bool hasKey = true; + static constexpr bool fromResult = false; + static std::string key(const ARThrowingKeyTouch&) { + throw std::runtime_error("key extraction failed"); + } +}; + +BRIDGE_MODEL_KEY(ARKeyedModel, ARTouch, &ARTouch::id); +BRIDGE_KEY_FROM_RESULT(ARKeyedCreate, &ARKeyedCreated::id); + // ── Issue #67: assignHandlerPrimary prefers IBackend::assignPrimaryAsync ──── // // A model whose result-keyed action (BRIDGE_KEY_FROM_RESULT) drives @@ -136,6 +237,49 @@ class AsyncRegisterBackend : public morph::backend::detail::IBackend { return true; } + // The shared/keyed counterparts, deferred exactly the same way: the reply + // lands in the same queue completeNext()/failNext() drain, so a keyed + // attach is observably non-blocking for the same reason a plain + // registration is. + bool registerModelSharedAsync(const std::string& typeId, + std::function()> factory, + morph::backend::detail::InstanceIdentity /*identity*/, + std::function onRegistered, + std::function onError) override { + std::scoped_lock const lock{_pendingMtx}; + _pending.push_back(Pending{.typeId = typeId, + .factory = std::move(factory), + .onRegistered = std::move(onRegistered), + .onError = std::move(onError)}); + return true; + } + + bool attachModelAsync(const std::string& typeId, + std::function()> factory, + morph::backend::detail::InstanceIdentity /*identity*/, + morph::exec::detail::ModelId /*current*/, + std::function onRegistered, + std::function onError) override { + std::scoped_lock const lock{_pendingMtx}; + _pending.push_back(Pending{.typeId = typeId, + .factory = std::move(factory), + .onRegistered = std::move(onRegistered), + .onError = std::move(onError)}); + return true; + } + + void assignPrimary(morph::exec::detail::ModelId mid, const std::string& /*typeId*/, + std::string_view primary) override { + std::scoped_lock const lock{_regMtx}; + _assigned.emplace_back(mid.v, std::string{primary}); + } + + /// The (modelId, primary) pairs assignPrimary was asked to file, in order. + [[nodiscard]] std::vector> assignments() const { + std::scoped_lock const lock{_regMtx}; + return _assigned; + } + // Test hooks: settle the oldest still-pending async registration. void completeNext() { Pending pending; @@ -175,9 +319,103 @@ class AsyncRegisterBackend : public morph::backend::detail::IBackend { mutable std::mutex _regMtx; std::unordered_map> _models; + std::vector> _assigned; uint64_t _nextId{100}; }; +// Completes its async attach/bind callbacks *inline* -- synchronously, from +// inside attachModelAsync/registerModelSharedAsync itself, before the dispatch +// call returns. This is legal (nothing in IBackend forbids it) and it is what +// QtWebSocketBackend already does on its !_connected error branch, so +// Bridge::attachHandlerAsync/ensureBoundAsync must survive it: at that moment +// the Bridge is still holding _attachMtx around the dispatch, and anything the +// callback does that re-enters the Bridge under that lock -- publishing the +// binding's primary, or a result-keyed dispatch's assignHandlerPrimary -- +// self-deadlocks unless the outcome is deferred out of the dispatch frame. +class InlineCompletingBackend : public AsyncRegisterBackend { +public: + /// @param failInline When set, both methods report this message via onError + /// inline instead of succeeding. + explicit InlineCompletingBackend(std::optional failInline = std::nullopt) + : _failInline{std::move(failInline)} {} + + bool registerModelSharedAsync(const std::string& typeId, + std::function()> factory, + morph::backend::detail::InstanceIdentity /*identity*/, + std::function onRegistered, + std::function onError) override { + completeInline(typeId, std::move(factory), onRegistered, onError); + return true; + } + + bool attachModelAsync(const std::string& typeId, + std::function()> factory, + morph::backend::detail::InstanceIdentity /*identity*/, + morph::exec::detail::ModelId /*current*/, + std::function onRegistered, + std::function onError) override { + completeInline(typeId, std::move(factory), onRegistered, onError); + return true; + } + +private: + void completeInline(const std::string& typeId, + std::function()> factory, + const std::function& onRegistered, + const std::function& onError) { + if (_failInline) { + onError(*_failInline); + return; + } + onRegistered(registerModel(typeId, std::move(factory))); + } + + std::optional _failInline; +}; + +// A backend whose async dispatch call itself throws synchronously, before +// returning -- e.g. QtWebSocketBackend::attachModelAsync's wire::encode() +// failing before send. Bridge::attachHandlerAsync/ensureBoundAsync must +// report this through onDone (matching execute()'s documented never-throws +// contract) rather than letting it escape. +class ThrowingDispatchBackend : public AsyncRegisterBackend { +public: + bool attachModelAsync(const std::string&, std::function()>, + morph::backend::detail::InstanceIdentity, morph::exec::detail::ModelId, + std::function, + std::function) override { + throw std::runtime_error("attachModelAsync dispatch failed"); + } + + bool registerModelSharedAsync(const std::string&, + std::function()>, + morph::backend::detail::InstanceIdentity, + std::function, + std::function) override { + throw std::runtime_error("registerModelSharedAsync dispatch failed"); + } +}; + +// Violates IBackend's documented "exactly one callback per dispatch" +// contract by invoking onRegistered twice, inline, from inside +// attachModelAsync itself. Exercises detail::parkIfInFrame's own guard +// against a second callback claiming an outcome that inline dispatch already +// parked -- Bridge::attachHandlerAsync must still report exactly once. +class DoubleFiringBackend : public AsyncRegisterBackend { +public: + bool attachModelAsync(const std::string& typeId, + std::function()> factory, + morph::backend::detail::InstanceIdentity /*identity*/, + morph::exec::detail::ModelId /*current*/, + std::function onRegistered, + std::function /*onError*/) override { + auto mid = registerModel(typeId, std::move(factory)); + onRegistered(mid); + onRegistered(mid); // Contract violation: fires a second time inline. + return true; + } +}; + // 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 @@ -207,6 +445,22 @@ class AsyncBackendShim : public morph::backend::detail::IBackend { return _target->registerModelAsync(typeId, std::move(factory), contextKey, std::move(onRegistered), std::move(onError)); } + bool registerModelSharedAsync(const std::string& typeId, + std::function()> factory, + morph::backend::detail::InstanceIdentity identity, + std::function onRegistered, + std::function onError) override { + return _target->registerModelSharedAsync(typeId, std::move(factory), identity, std::move(onRegistered), + std::move(onError)); + } + bool attachModelAsync(const std::string& typeId, + std::function()> factory, + morph::backend::detail::InstanceIdentity identity, morph::exec::detail::ModelId current, + std::function onRegistered, + std::function onError) override { + return _target->attachModelAsync(typeId, std::move(factory), identity, current, std::move(onRegistered), + std::move(onError)); + } private: std::shared_ptr _target; @@ -364,6 +618,93 @@ TEST_CASE("Bridge::registerHandler: a stale async reply after switchBackend() do CHECK(binding->currentId.load() == idAfterSwitch); } +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE( + "Bridge::attachHandlerAsync: a stale async attach reply after switchBackend() does not clobber the new " + "binding, and still resolves the caller's Completion", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + auto asyncBackendA = std::make_shared(); + morph::bridge::Bridge bridge{std::make_unique(asyncBackendA)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic result{-1}; + std::atomic failed{false}; + auto pending = handler.execute(ARTouch{.id = 42, .amount = 5}); + pending.then([&](int val) { result.store(val); }).onError([&](const std::exception_ptr&) { failed.store(true); }); + + // The attach was dispatched but has not replied yet. + REQUIRE(asyncBackendA->pendingCount() == 1); + CHECK(result.load() == -1); + CHECK_FALSE(failed.load()); + + // Switch away WHILE the attach on asyncBackendA is still outstanding. The + // handler never attached (its primary is still empty), so switchBackend's + // re-registration loop leaves it live-but-unbound on the new backend -- + // matching the `binding->shared && binding->primary.empty()` carry-over + // path. + auto secondBackend = std::make_unique(); + auto* rawSecond = secondBackend.get(); + bridge.switchBackend(std::move(secondBackend)); + + // The original (now-stale) attach reply from asyncBackendA finally + // arrives. It must not be published into the binding as if it were a + // valid id on the now-active backend -- and, unlike a fire-and-forget + // re-registration, this caller's Completion is genuinely waiting on + // `onDone`, so the stale reply must still resolve it (with an error) + // rather than leaving it hanging forever. + asyncBackendA->completeNext(); + REQUIRE(morph::testing::waitUntil([&] { return result.load() != -1 || failed.load(); })); + CHECK(result.load() == -1); + CHECK(failed.load()); + CHECK_FALSE(handler.primary().has_value()); + + // The handler is still usable against the now-active backend: a fresh + // attach succeeds normally, proving the stale reply left no corruption + // behind. + std::atomic secondResult{-1}; + handler.execute(ARTouch{.id = 42, .amount = 9}) + .then([&](int val) { secondResult.store(val); }) + .onError([&](const std::exception_ptr&) { failed.store(true); }); + REQUIRE(rawSecond->pendingCount() == 1); + rawSecond->completeNext(); + REQUIRE(morph::testing::waitUntil([&] { return secondResult.load() != -1; })); + CHECK(secondResult.load() == 9); + CHECK(handler.primary().value_or(-1) == 42); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE( + "Bridge::ensureBoundAsync: a stale async bind reply after switchBackend() does not clobber the new binding, " + "and still resolves the caller's Completion", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + auto asyncBackendA = std::make_shared(); + morph::bridge::Bridge bridge{std::make_unique(asyncBackendA)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic value{-1}; + std::atomic failed{false}; + auto pending = handler.execute(ARKeyedCreate{.initial = 11}); + pending.then([&](ARKeyedCreated res) { value.store(res.value); }).onError([&](const std::exception_ptr&) { + failed.store(true); + }); + + REQUIRE(asyncBackendA->pendingCount() == 1); + CHECK(value.load() == -1); + CHECK_FALSE(failed.load()); + + auto secondBackend = std::make_unique(); + bridge.switchBackend(std::move(secondBackend)); + + // The stale bind reply must not publish `currentId` from a backend + // nothing uses any more, and must still resolve the waiting Completion. + asyncBackendA->completeNext(); + REQUIRE(morph::testing::waitUntil([&] { return value.load() != -1 || failed.load(); })); + CHECK(value.load() == -1); + CHECK(failed.load()); +} + TEST_CASE("Bridge::registerHandler: an async reply arriving after ~Bridge() is a safe no-op", "[bridge][registration][issue26]") { morph::exec::ThreadPoolExecutor pool{2}; @@ -898,3 +1239,586 @@ TEST_CASE("Bridge::whenBound: concurrent callers racing the exact moment registr REQUIRE(resolvedTrue.load() == kWaitersPerTrial); } } + +// --------------------------------------------------------------------------- +// Shared/keyed registration: registerModelSharedAsync + attachModelAsync. +// +// Same opt-in/fallback contract as registerModelAsync above, reached through +// Bridge::attachHandlerAsync (payload-keyed actions) and +// Bridge::ensureBoundAsync (result-keyed ones), both of which BridgeHandler's +// execute() now routes its keyed dispatches through. execute()'s own contract +// is unchanged: the attach/promote step never throws out of the call, it +// resolves the returned Completion. +// --------------------------------------------------------------------------- + +using morph::bridge::AllowShared; + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("Bridge prefers attachModelAsync over the synchronous attachModel when the backend offers it", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + + // An AllowShared handler registers nothing at construction -- it acquires + // an instance only when a keyed action names one. + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + REQUIRE(rawBackend->pendingCount() == 0); + + std::atomic result{-1}; + std::atomic failed{false}; + auto pending = handler.execute(ARTouch{.id = 42, .amount = 5}); + pending.then([&](int val) { result.store(val); }).onError([&](const std::exception_ptr&) { failed.store(true); }); + + // The attach was dispatched but has not replied: execute() returned a + // still-pending Completion rather than blocking in a nested wait, which is + // the entire point on a WASM main thread. + REQUIRE(rawBackend->pendingCount() == 1); + CHECK(result.load() == -1); + CHECK_FALSE(failed.load()); + + rawBackend->completeNext(); + REQUIRE(morph::testing::waitUntil([&] { return result.load() != -1; })); + CHECK(result.load() == 5); + CHECK_FALSE(failed.load()); + CHECK(handler.primary().value_or(-1) == 42); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("A backend with no async attach path falls back to the synchronous attachModel unchanged", + "[bridge][registration][shared-instances][issue26]") { + // LocalBackend overrides neither attachModelAsync nor + // registerModelSharedAsync, so IBackend's defaults (returning false) apply + // and the keyed execute() runs the identical synchronous attach it always + // has -- bound before the dispatch, on this thread. + morph::exec::ThreadPoolExecutor pool{2}; + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic result{-1}; + std::atomic failed{false}; + handler.execute(ARTouch{.id = 7, .amount = 3}) + .then([&](int val) { result.store(val); }) + .onError([&](const std::exception_ptr&) { failed.store(true); }); + + REQUIRE(morph::testing::waitUntil([&] { return result.load() != -1 || failed.load(); })); + CHECK_FALSE(failed.load()); + CHECK(result.load() == 3); + CHECK(handler.primary().value_or(-1) == 7); + + // A second keyed action on the same key is the idempotent-attach path, and + // lands on the same instance (3 + 4), proving the fallback kept the + // binding, not just the first reply. + std::atomic second{-1}; + handler.execute(ARTouch{.id = 7, .amount = 4}) + .then([&](int val) { second.store(val); }) + .onError([&](const std::exception_ptr&) { failed.store(true); }); + REQUIRE(morph::testing::waitUntil([&] { return second.load() != -1 || failed.load(); })); + CHECK_FALSE(failed.load()); + CHECK(second.load() == 7); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE( + "attachModelAsync's onError path surfaces through the returned Completion's onError, matching the synchronous " + "path's documented contract", + "[bridge][registration][shared-instances][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}; + + // execute() itself must not throw, whatever the attach does -- the failure + // is a Completion outcome, not a synchronous exception. + std::optional> pending; + REQUIRE_NOTHROW(pending.emplace(handler.execute(ARTouch{.id = 99, .amount = 1}))); + + std::string message; + std::atomic succeeded{false}; + pending->then([&](int) { succeeded.store(true); }).onError([&](const std::exception_ptr& err) { + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + message = exc.what(); + } + }); + + REQUIRE(rawBackend->pendingCount() == 1); + REQUIRE_NOTHROW(rawBackend->failNext("attach refused")); + + REQUIRE(morph::testing::waitUntil([&] { return !message.empty(); })); + CHECK(message == "attach refused"); + CHECK_FALSE(succeeded.load()); + // The failed attach left the handler unattached, exactly as the + // synchronous path's throwing attach does. + CHECK_FALSE(handler.primary().has_value()); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("A backend that completes attachModelAsync inline does not deadlock and resolves normally", + "[bridge][registration][shared-instances][issue26]") { + // Regression guard for the inline-completion hole: attachHandlerAsync + // dispatches under _attachMtx, and its success callback re-acquires that + // lock to publish contextKey/primary. A callback that fires inline would + // therefore re-enter a mutex this very frame holds. The dispatch frame must + // park such an outcome and apply it after the lock is released instead. + SyncExec cbExec; + auto backend = std::make_unique(); + morph::bridge::Bridge bridge{std::move(backend)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic result{-1}; + std::atomic failed{false}; + // If the frame deadlocked, execute() never returns and this test hangs. + handler.execute(ARTouch{.id = 8, .amount = 6}) + .then([&](int val) { result.store(val); }) + .onError([&](const std::exception_ptr&) { failed.store(true); }); + + CHECK_FALSE(failed.load()); + CHECK(result.load() == 6); + // The inline outcome was published exactly as an out-of-frame one would be: + // primary() reads binding->primary under _attachMtx, which is also proof + // the lock was released rather than left held by the dispatch frame. + CHECK(handler.primary().value_or(-1) == 8); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("A backend that completes registerModelSharedAsync inline still promotes a result-keyed action", + "[bridge][registration][shared-instances][issue26]") { + // The sharpest form of the same hole: an inline bind runs onDone -- i.e. + // the whole dispatch -- inside ensureBoundAsync's frame, and a result-keyed + // dispatch's onResult calls assignHandlerPrimary, which takes _attachMtx. + SyncExec cbExec; + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic value{-1}; + std::atomic failed{false}; + handler.execute(ARKeyedCreate{.initial = 17}) + .then([&](ARKeyedCreated res) { value.store(res.value); }) + .onError([&](const std::exception_ptr&) { failed.store(true); }); + + CHECK_FALSE(failed.load()); + CHECK(value.load() == 17); + CHECK(handler.primary().value_or(-1) == 4242); + auto const assigned = rawBackend->assignments(); + REQUIRE(assigned.size() == 1); + CHECK(assigned.front().second == "4242"); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("A backend that reports its async attach failure inline surfaces it through onError, exactly once", + "[bridge][registration][shared-instances][issue26]") { + // QtWebSocketBackend's !_connected branch, in miniature: onError invoked + // synchronously from inside attachModelAsync, which then returns true. + SyncExec cbExec; + auto backend = std::make_unique(std::optional{"disconnected"}); + morph::bridge::Bridge bridge{std::move(backend)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::optional> pending; + REQUIRE_NOTHROW(pending.emplace(handler.execute(ARTouch{.id = 3, .amount = 1}))); + + std::string message; + int errorCount = 0; + std::atomic succeeded{false}; + pending->then([&](int) { succeeded.store(true); }).onError([&](const std::exception_ptr& err) { + ++errorCount; + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + message = exc.what(); + } + }); + + CHECK(message == "disconnected"); + CHECK(errorCount == 1); + CHECK_FALSE(succeeded.load()); + CHECK_FALSE(handler.primary().has_value()); +} + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("ensureBoundAsync mirrors the same three cases for a result-keyed (creating) action", + "[bridge][registration][shared-instances][issue26]") { + SECTION("prefers registerModelSharedAsync when the backend offers it") { + 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() == 0); + + std::atomic value{-1}; + std::atomic failed{false}; + auto pending = handler.execute(ARKeyedCreate{.initial = 11}); + pending.then([&](ARKeyedCreated res) { value.store(res.value); }).onError([&](const std::exception_ptr&) { + failed.store(true); + }); + + // Bound asynchronously: still nothing resolved, nothing blocked. + REQUIRE(rawBackend->pendingCount() == 1); + CHECK(value.load() == -1); + + rawBackend->completeNext(); + REQUIRE(morph::testing::waitUntil([&] { return value.load() != -1; })); + CHECK(value.load() == 11); + CHECK_FALSE(failed.load()); + // The result-sourced key was adopted in place before the caller's + // .then() saw the result. + CHECK(handler.primary().value_or(-1) == 4242); + auto const assigned = rawBackend->assignments(); + REQUIRE(assigned.size() == 1); + CHECK(assigned.front().second == "4242"); + } + + SECTION("falls back to the synchronous registerModelShared when it does not") { + morph::exec::ThreadPoolExecutor pool{2}; + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic value{-1}; + std::atomic failed{false}; + handler.execute(ARKeyedCreate{.initial = 23}) + .then([&](ARKeyedCreated res) { value.store(res.value); }) + .onError([&](const std::exception_ptr&) { failed.store(true); }); + + REQUIRE(morph::testing::waitUntil([&] { return value.load() != -1 || failed.load(); })); + CHECK_FALSE(failed.load()); + CHECK(value.load() == 23); + CHECK(handler.primary().value_or(-1) == 4242); + } + + SECTION("surfaces registerModelSharedAsync's onError through the returned Completion") { + SyncExec cbExec; + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::optional> pending; + REQUIRE_NOTHROW(pending.emplace(handler.execute(ARKeyedCreate{.initial = 5}))); + + std::string message; + std::atomic succeeded{false}; + pending->then([&](ARKeyedCreated) { succeeded.store(true); }).onError([&](const std::exception_ptr& err) { + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + message = exc.what(); + } + }); + + REQUIRE(rawBackend->pendingCount() == 1); + REQUIRE_NOTHROW(rawBackend->failNext("no capacity")); + + REQUIRE(morph::testing::waitUntil([&] { return !message.empty(); })); + CHECK(message == "no capacity"); + CHECK_FALSE(succeeded.load()); + CHECK_FALSE(handler.primary().has_value()); + } +} + +TEST_CASE("attachHandlerAsync reports a synchronously-throwing dispatch call through onDone", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique()}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::string message; + std::atomic succeeded{false}; + REQUIRE_NOTHROW(handler.execute(ARTouch{.id = 9, .amount = 1}) + .then([&](int) { succeeded.store(true); }) + .onError([&](const std::exception_ptr& err) { + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + message = exc.what(); + } + })); + + CHECK(message == "attachModelAsync dispatch failed"); + CHECK_FALSE(succeeded.load()); + CHECK_FALSE(handler.primary().has_value()); +} + +TEST_CASE("ensureBoundAsync reports a synchronously-throwing dispatch call through onDone", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique()}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::string message; + std::atomic succeeded{false}; + REQUIRE_NOTHROW(handler.execute(ARKeyedCreate{.initial = 3}) + .then([&](ARKeyedCreated) { succeeded.store(true); }) + .onError([&](const std::exception_ptr& err) { + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + message = exc.what(); + } + })); + + CHECK(message == "registerModelSharedAsync dispatch failed"); + CHECK_FALSE(succeeded.load()); + CHECK_FALSE(handler.primary().has_value()); +} + +TEST_CASE("attachHandlerAsync's out-of-frame success callback is a no-op once the Bridge is gone", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + // The backend must outlive the Bridge for this test to complete the reply + // after destroying it, so it is co-owned via AsyncBackendShim (the same + // pattern test_bridge_lifetime.cpp uses) rather than owned solely by the + // Bridge's unique_ptr. + auto sharedBackend = std::make_shared(); + auto bridge = std::make_unique(std::make_unique(sharedBackend)); + auto handler = std::make_unique>(*bridge, &cbExec); + + REQUIRE_NOTHROW(handler->execute(ARTouch{.id = 11, .amount = 4})); + REQUIRE(sharedBackend->pendingCount() == 1); + + // Destroy the handler and the Bridge itself before the deferred reply + // lands: attachHandlerAsync's success callback holds only weak references + // to both, so completing it now must be a quiet no-op rather than + // dereferencing freed memory. + handler.reset(); + bridge.reset(); + + REQUIRE_NOTHROW(sharedBackend->completeNext()); + SUCCEED("completing an attach reply after the Bridge and handler are both gone did not crash"); +} + +TEST_CASE("attachHandlerAsync's out-of-frame success callback tolerates the BridgeHandler being gone", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + auto handler = std::make_unique>(bridge, &cbExec); + + REQUIRE_NOTHROW(handler->execute(ARTouch{.id = 12, .amount = 4})); + REQUIRE(rawBackend->pendingCount() == 1); + + // Destroy only the handler; the Bridge itself stays alive. Note that the + // binding itself does *not* actually go away here: execute()'s dispatch + // copies `_binding` into a local held by this very completion's own + // onDone closure (see BridgeHandler::execute), so the pending dispatch + // keeps it alive independent of the BridgeHandler. This still exercises a + // real case worth having a test for -- a caller that drops its handler + // while a keyed attach is in flight must not crash when the reply lands. + handler.reset(); + + REQUIRE_NOTHROW(rawBackend->completeNext()); + SUCCEED("completing an attach reply after the BridgeHandler is gone did not crash"); +} + +TEST_CASE("ensureBoundAsync's out-of-frame success callback is a no-op once the Bridge is gone", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + // See the identical attachHandlerAsync test above: the backend must + // outlive the Bridge, so it is co-owned via AsyncBackendShim. + auto sharedBackend = std::make_shared(); + auto bridge = std::make_unique(std::make_unique(sharedBackend)); + auto handler = std::make_unique>(*bridge, &cbExec); + + REQUIRE_NOTHROW(handler->execute(ARKeyedCreate{.initial = 6})); + REQUIRE(sharedBackend->pendingCount() == 1); + + handler.reset(); + bridge.reset(); + + REQUIRE_NOTHROW(sharedBackend->completeNext()); + SUCCEED("completing a registerModelSharedAsync reply after the Bridge and handler are both gone did not crash"); +} + +TEST_CASE("ensureBoundAsync's out-of-frame success callback tolerates the BridgeHandler being gone", + "[bridge][registration][shared-instances][issue26]") { + SyncExec cbExec; + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + auto handler = std::make_unique>(bridge, &cbExec); + + REQUIRE_NOTHROW(handler->execute(ARKeyedCreate{.initial = 7})); + REQUIRE(rawBackend->pendingCount() == 1); + + // See attachHandlerAsync's identical test above: the binding itself stays + // alive here (pinned by the pending dispatch's own onDone closure), but + // dropping the handler while the reply is still in flight is still a real + // case worth covering. + handler.reset(); + + REQUIRE_NOTHROW(rawBackend->completeNext()); + SUCCEED("completing a registerModelSharedAsync reply after the BridgeHandler is gone did not crash"); +} + +TEST_CASE("ensureBound is a no-op when the binding already has an instance", + "[bridge][registration][shared-instances][issue26]") { + // Bridge::ensureBound is the synchronous counterpart to ensureBoundAsync, + // used directly (not through BridgeHandler::execute) when a caller wants + // to force-bind an anonymous instance ahead of time. Calling it twice on + // the same binding exercises its already-bound early-return: the second + // call must not register a second instance. + morph::exec::ThreadPoolExecutor pool{2}; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + + auto binding = std::make_shared(); + binding->typeId = "AR_KeyedModel"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + + REQUIRE_NOTHROW(bridge.ensureBound(binding)); + auto const firstId = binding->currentId.load(); + REQUIRE(firstId != 0U); + + REQUIRE_NOTHROW(bridge.ensureBound(binding)); + CHECK(binding->currentId.load() == firstId); +} + +TEST_CASE("ensureBoundAsync's onError path is a no-op once the dispatching frame already claimed the outcome", + "[bridge][registration][shared-instances][issue26]") { + // Mirrors attachModelAsync's identical inline-failure test above, for + // registerModelSharedAsync: onError invoked synchronously from inside the + // dispatch call (which then returns true) exercises the parkIfInFrame + // no-op inside ensureBoundAsync's error callback, not just its success one. + SyncExec cbExec; + auto backend = std::make_unique(std::optional{"disconnected"}); + morph::bridge::Bridge bridge{std::move(backend)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::optional> pending; + REQUIRE_NOTHROW(pending.emplace(handler.execute(ARKeyedCreate{.initial = 8}))); + + std::string message; + int errorCount = 0; + std::atomic succeeded{false}; + pending->then([&](ARKeyedCreated) { succeeded.store(true); }).onError([&](const std::exception_ptr& err) { + ++errorCount; + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + message = exc.what(); + } + }); + + CHECK(message == "disconnected"); + CHECK(errorCount == 1); + CHECK_FALSE(succeeded.load()); + CHECK_FALSE(handler.primary().has_value()); +} + +TEST_CASE("execute() surfaces a throwing ActionKeyTraits::key() through onError instead of escaping", + "[bridge][registration][shared-instances][issue26]") { + // Key extraction runs ahead of the attach dispatch, on execute()'s own + // stack -- a throwing key() must resolve the returned Completion's + // onError, matching every other keyed-dispatch failure, rather than + // throwing out of execute() itself. + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique()}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::string message; + std::atomic succeeded{false}; + REQUIRE_NOTHROW(handler.execute(ARThrowingKeyTouch{.amount = 1}) + .then([&](int) { succeeded.store(true); }) + .onError([&](const std::exception_ptr& err) { + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + message = exc.what(); + } + })); + + CHECK(message == "key extraction failed"); + CHECK_FALSE(succeeded.load()); + CHECK_FALSE(handler.primary().has_value()); +} + +TEST_CASE("attachHandlerAsync reports exactly once even when the backend fires its callback twice inline", + "[bridge][registration][shared-instances][issue26]") { + // DoubleFiringBackend violates attachModelAsync's documented one-callback + // contract on purpose: detail::parkIfInFrame's `handoff.fired` guard must + // swallow the second, already-claimed callback rather than letting + // attachHandlerAsync invoke onDone (and, downstream, publish the binding) + // twice for a single dispatch. + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique()}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + int completions = 0; + std::atomic result{-1}; + REQUIRE_NOTHROW(handler.execute(ARTouch{.id = 21, .amount = 6}) + .then([&](int val) { + ++completions; + result.store(val); + }) + .onError([&](const std::exception_ptr&) { ++completions; })); + + CHECK(completions == 1); + CHECK(result.load() == 6); + CHECK(handler.primary().value_or(-1) == 21); +} + +TEST_CASE("attachHandlerAsync's out-of-frame success callback is a genuine no-op once the binding itself is gone", + "[bridge][registration][shared-instances][issue26]") { + // The other attachHandlerAsync/ensureBoundAsync "binding is gone" tests + // above go through BridgeHandler::execute, whose own dispatch closure + // captures the binding by value -- so the binding never actually dies + // while that dispatch is in flight (see those tests' comments). Calling + // attachHandlerAsync directly, with an onDone that captures nothing + // binding-related, removes that hidden strong reference: dropping the + // test's own shared_ptr before completing the reply is what actually + // exercises weakBinding.lock() failing. + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + + std::weak_ptr weakBinding; + { + auto binding = std::make_shared(); + binding->typeId = "AR_KeyedModel"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + weakBinding = binding; + + std::atomic onDoneFired{false}; + bridge.template attachHandlerAsync(binding, "13", + [&onDoneFired](std::exception_ptr) { onDoneFired.store(true); }); + REQUIRE(rawBackend->pendingCount() == 1); + // `binding` (the only remaining strong reference, now that onDone + // captures none) goes out of scope at the end of this block. + } + REQUIRE(weakBinding.expired()); + + REQUIRE_NOTHROW(rawBackend->completeNext()); + SUCCEED("completing an attach reply after the binding itself is gone did not crash"); +} + +TEST_CASE("ensureBoundAsync's out-of-frame success callback is a genuine no-op once the binding itself is gone", + "[bridge][registration][shared-instances][issue26]") { + // Mirrors attachHandlerAsync's identical direct-call test above. + auto backend = std::make_unique(); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + + std::weak_ptr weakBinding; + { + auto binding = std::make_shared(); + binding->typeId = "AR_KeyedModel"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + weakBinding = binding; + + std::atomic onDoneFired{false}; + bridge.ensureBoundAsync(binding, [&onDoneFired](std::exception_ptr) { onDoneFired.store(true); }); + REQUIRE(rawBackend->pendingCount() == 1); + } + REQUIRE(weakBinding.expired()); + + REQUIRE_NOTHROW(rawBackend->completeNext()); + SUCCEED("completing a registerModelSharedAsync reply after the binding itself is gone did not crash"); +} diff --git a/tests/test_client_execute_deadline.cpp b/tests/test_client_execute_deadline.cpp new file mode 100644 index 00000000..80c8b98c --- /dev/null +++ b/tests/test_client_execute_deadline.cpp @@ -0,0 +1,266 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Coverage for the client-side execute deadline (examples/LADDER.md's +// "Framework prerequisites" #2): Bridge::setExecuteDeadline races the real +// reply against a client-owned timeout, so a frame silently dropped by +// QtWebSocketServerConfig::messagesPerSecond, or a genuinely hung server, +// no longer blocks the calling Completion forever. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "test_support.hpp" + +namespace { + +struct DeadlineCount { + int x = 0; +}; + +struct DeadlineModel { + int execute(const DeadlineCount& a) { return a.x; } +}; + +// A backend whose execute() never resolves its Completion, simulating a frame +// the server dropped -- no reply, ever, on this path -- or a hung server. +class NeverRepliesBackend : public morph::backend::detail::IBackend { +public: + morph::exec::detail::ModelId registerModel( + const std::string&, std::function()>) override { + return morph::exec::detail::ModelId{1}; + } + void deregisterModel(morph::exec::detail::ModelId) override {} + morph::async::Completion> execute(morph::exec::detail::ModelId, + morph::backend::detail::ActionCall, + morph::exec::IExecutor* cbExec) override { + auto state = std::make_shared>>(); + return morph::async::Completion>{state, cbExec}; + // `state` is intentionally dropped here with no setValue/setException + // ever called -- the Completion this returns never settles on its + // own, matching a dropped frame or a server that never replies. + } + void notifyBackendChanged() override {} + // Deliberately a no-op: a real backend resolves its outstanding states + // here, which is exactly the "something eventually settles it" behaviour + // these tests must not rely on. + void cancelPending(const std::exception_ptr&) override {} +}; + +// A backend that holds every state it hands out and only settles it when the +// test says so -- a server whose reply arrives *after* the client already gave +// up. Lets the "a late real reply is silently discarded" guarantee be asserted +// deterministically rather than by racing wall-clock timers. +class LateReplyBackend : public morph::backend::detail::IBackend { +public: + morph::exec::detail::ModelId registerModel( + const std::string&, std::function()>) override { + return morph::exec::detail::ModelId{1}; + } + void deregisterModel(morph::exec::detail::ModelId) override {} + morph::async::Completion> execute(morph::exec::detail::ModelId, + morph::backend::detail::ActionCall, + morph::exec::IExecutor* cbExec) override { + auto state = std::make_shared>>(); + { + std::scoped_lock const lock{_mtx}; + _pending.push_back(state); + } + return morph::async::Completion>{state, cbExec}; + } + void notifyBackendChanged() override {} + void cancelPending(const std::exception_ptr&) override {} + + /// Settles every outstanding request with @p value, as a server reply that + /// finally turned up would. + void replyLate(int value) { + std::vector>>> pending; + { + std::scoped_lock const lock{_mtx}; + pending.swap(_pending); + } + for (auto& state : pending) { + state->setValue(std::static_pointer_cast(std::make_shared(value))); + } + } + +private: + std::mutex _mtx; + std::vector>>> _pending; +}; + +} // namespace + +template <> +struct morph::model::ActionTraits { + using Result = int; + static constexpr std::string_view typeId() { return "Deadline_Count"; } + static std::string toJson(const DeadlineCount& a) { return R"({"x":)" + std::to_string(a.x) + "}"; } + static DeadlineCount 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 "Deadline_Model"; } +}; + +TEST_CASE("Bridge::setExecuteDeadline(0) (the default) never fires -- a call that never replies " + "stays pending, matching pre-existing behavior", + "[core][bridge][client-deadline]") { + morph::exec::MainThreadExecutor exec; + morph::bridge::Bridge bridge{std::make_unique()}; + CHECK(bridge.executeDeadline() == std::chrono::milliseconds{0}); + morph::bridge::BridgeHandler handler{bridge, &exec}; + + bool resolved = false; + handler.execute(DeadlineCount{.x = 1}) + .then([&resolved](int) { resolved = true; }) + .onError([&resolved](const std::exception_ptr&) { resolved = true; }); + exec.runFor(std::chrono::milliseconds{200}); + CHECK_FALSE(resolved); +} + +TEST_CASE("Bridge::setExecuteDeadline fires ClientTimeoutError when no reply arrives in time", + "[core][bridge][client-deadline]") { + morph::exec::MainThreadExecutor exec; + morph::bridge::Bridge bridge{std::make_unique()}; + bridge.setExecuteDeadline(std::chrono::milliseconds{50}); + CHECK(bridge.executeDeadline() == std::chrono::milliseconds{50}); + morph::bridge::BridgeHandler handler{bridge, &exec}; + + bool failed = false; + bool threwClientTimeout = false; + handler.execute(DeadlineCount{.x = 1}).onError([&](const std::exception_ptr& err) { + failed = true; + try { + std::rethrow_exception(err); + } catch (const morph::backend::ClientTimeoutError&) { + threwClientTimeout = true; + } catch (...) { + } + }); + // Poll rather than a single runFor(): the deadline fires on the + // TimeoutScheduler's own background thread, which posts to `exec` -- + // give it real wall-clock slack, matching this codebase's other + // cross-thread test patterns. + for (int i = 0; i < 50 && !failed; ++i) { + exec.runFor(std::chrono::milliseconds{20}); + } + REQUIRE(failed); + CHECK(threwClientTimeout); +} + +TEST_CASE("A deadline that is cancelled by a real, on-time reply does not also fire", + "[core][bridge][client-deadline]") { + // Uses the ordinary in-process LocalBackend, which always replies quickly. + // Pins the happy path: enabling a deadline must not perturb a call that + // replies in time. It runs *through* the disarm path but cannot detect its + // absence -- because CompletionState is first-result-wins, a stray timer + // firing after an on-time reply is a silent no-op at the value level, so + // deleting the disarm entirely would leave this case green. The disarm's + // actual, observable effect is *lifetime*, and that is what the next test + // ("An on-time reply releases the deadline's scheduler entry") covers. + morph::exec::ThreadPoolExecutor workerPool{2}; + morph::exec::MainThreadExecutor guiExec; + morph::bridge::Bridge bridge{std::make_unique(workerPool)}; + bridge.setExecuteDeadline(std::chrono::milliseconds{2000}); // generous; must not fire + morph::bridge::BridgeHandler handler{bridge, &guiExec}; + + int result = -1; + bool failed = false; + handler.execute(DeadlineCount{.x = 7}) + .then([&result](int r) { result = r; }) + .onError([&failed](const std::exception_ptr&) { failed = true; }); + for (int i = 0; i < 50 && result == -1 && !failed; ++i) { + guiExec.runFor(std::chrono::milliseconds{10}); + } + CHECK(result == 7); + CHECK_FALSE(failed); + // If the disarm did not work, the 2000ms deadline would still be pending on + // the scheduler's background thread when the Bridge goes out of scope here. + // That must not hang the test process: ~TimeoutScheduler drops pending + // entries without firing them and joins its thread unconditionally, so a + // leaked entry costs nothing at teardown. Noted rather than asserted -- + // there is no public handle to observe it through. +} + +TEST_CASE("An on-time reply releases the deadline's scheduler entry (and the state it pins)", + "[core][bridge][client-deadline]") { + // What the disarm actually buys: TimeoutScheduler::cancel() erases the + // pending entry and with it the std::function that captures a + // shared_ptr>. Without the disarm that entry -- and the + // completed state it keeps alive -- would be pinned for the full deadline. + morph::exec::ThreadPoolExecutor workerPool{2}; + morph::exec::MainThreadExecutor guiExec; + morph::bridge::Bridge bridge{std::make_unique(workerPool)}; + bridge.setExecuteDeadline(std::chrono::milliseconds{5000}); // long enough that only + morph::bridge::BridgeHandler handler{bridge, &guiExec}; // an actual cancel frees it + + int result = -1; + std::weak_ptr stateWatch; + { + auto completion = handler.execute(DeadlineCount{.x = 7}); + stateWatch = completion.state(); // Completion::state(), completion.hpp:208 + completion.then([&result](int r) { result = r; }); + } + for (int i = 0; i < 50 && result == -1; ++i) { + guiExec.runFor(std::chrono::milliseconds{10}); + } + REQUIRE(result == 7); + guiExec.runFor(std::chrono::milliseconds{100}); + // Without the disarm, the timer entry still owns the state for 5 s. + CHECK(stateWatch.expired()); +} + +TEST_CASE("A real reply that arrives after the deadline already fired is silently discarded", + "[core][bridge][client-deadline]") { + // The idempotency half of the race documented in docs/spec/core/completion.md: + // once the deadline resolved the Completion with ClientTimeoutError, the + // server's eventual reply must not resurrect it with a value. Driven + // explicitly by the test rather than by wall-clock luck. + morph::exec::MainThreadExecutor exec; + auto backendOwner = std::make_unique(); + auto* const backend = backendOwner.get(); + morph::bridge::Bridge bridge{std::move(backendOwner)}; + bridge.setExecuteDeadline(std::chrono::milliseconds{50}); + morph::bridge::BridgeHandler handler{bridge, &exec}; + + int settleCount = 0; + int value = -1; + bool threwClientTimeout = false; + handler.execute(DeadlineCount{.x = 1}) + .then([&](int r) { + ++settleCount; + value = r; + }) + .onError([&](const std::exception_ptr& err) { + ++settleCount; + try { + std::rethrow_exception(err); + } catch (const morph::backend::ClientTimeoutError&) { + threwClientTimeout = true; + } catch (...) { + } + }); + + for (int i = 0; i < 50 && settleCount == 0; ++i) { + exec.runFor(std::chrono::milliseconds{20}); + } + REQUIRE(settleCount == 1); + REQUIRE(threwClientTimeout); + + // The server finally replies. Nothing may change. + backend->replyLate(99); + exec.runFor(std::chrono::milliseconds{200}); + CHECK(settleCount == 1); + CHECK(value == -1); +} diff --git a/tests/test_completion.cpp b/tests/test_completion.cpp index 4b00b72e..20f24dc1 100644 --- a/tests/test_completion.cpp +++ b/tests/test_completion.cpp @@ -77,6 +77,31 @@ TEST_CASE("morph::async::Completion on_error does not fire on value", "[completi REQUIRE_FALSE(errorFired); } +TEST_CASE("morph::async::Completion onError composes every attached handler, in attachment order", + "[completion]") { + // CompletionState::attachOnError appends to a vector of handlers, so a + // second .onError() call on the same still-pending Completion — even + // via the separate Completion& returned from the first call, since + // then()/onError() both return *this — fires alongside the first rather + // than replacing it (see docs/spec/core/completion.md, "Handler + // fan-out"). This is the mechanism behind Presenter::track()'s onErr + // parameter (examples/common/gui/presenter.hpp), documented here at its + // source. + SyncExecutor exec; + auto state = std::make_shared>(); + morph::async::Completion comp{state, &exec}; + + bool handlerAFired = false; + bool handlerBFired = false; + comp.onError([&](const std::exception_ptr&) { handlerAFired = true; }); + comp.onError([&](const std::exception_ptr&) { handlerBFired = true; }); + + state->setException(std::make_exception_ptr(std::runtime_error{"test error"})); + + REQUIRE(handlerAFired); + REQUIRE(handlerBFired); +} + TEST_CASE("morph::async::Completion callback is posted through executor", "[completion]") { struct CountingExecutor : morph::exec::IExecutor { std::atomic count{0}; diff --git a/tests/test_timeout_scheduler.cpp b/tests/test_timeout_scheduler.cpp new file mode 100644 index 00000000..801738c9 --- /dev/null +++ b/tests/test_timeout_scheduler.cpp @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Direct unit coverage for morph::async::detail::TimeoutScheduler (the +// threaded build, compiled whenever __EMSCRIPTEN__ without +// __EMSCRIPTEN_PTHREADS__ is not defined -- see timeout_scheduler.hpp's @file +// comment for the single-threaded browser build, which this file's own +// target never compiles and cannot exercise). Bridge::executeVia and +// RemoteServer only ever call schedule()/cancel() with callbacks that don't +// throw, so this file covers the case they don't: a scheduled callback that +// throws is logged and swallowed rather than propagating out of the +// scheduler's background thread. + +#include +#include +#include +#include +#include +#include + +using morph::async::detail::TimeoutScheduler; +using namespace std::chrono_literals; + +namespace { + +// Polls until predicate is true or the deadline elapses, to avoid a +// fixed-sleep race between the test thread and the scheduler's background +// thread invoking the callback. +template +bool waitFor(Predicate predicate, std::chrono::milliseconds timeout = 2s) { + auto const deadline = std::chrono::steady_clock::now() + timeout; + while (std::chrono::steady_clock::now() < deadline) { + if (predicate()) { + return true; + } + std::this_thread::sleep_for(1ms); + } + return predicate(); +} + +} // namespace + +TEST_CASE("TimeoutScheduler: a callback throwing std::exception is logged and swallowed", + "[timeout_scheduler]") { + TimeoutScheduler scheduler; + std::atomic fired{false}; + + scheduler.schedule(1ms, [&fired] { + fired = true; + throw std::runtime_error{"boom"}; + }); + + // The scheduler thread must survive the throw: the callback still runs + // (fired becomes true) and the destructor below still joins cleanly + // instead of std::terminate-ing on an escaped exception. + REQUIRE(waitFor([&] { return fired.load(); })); +} + +TEST_CASE("TimeoutScheduler: a callback throwing a non-std::exception is logged and swallowed", + "[timeout_scheduler]") { + TimeoutScheduler scheduler; + std::atomic fired{false}; + + scheduler.schedule(1ms, [&fired] { + fired = true; + throw 42; // NOLINT(hicpp-exception-baseclass) -- exercises the catch(...) arm deliberately. + }); + + REQUIRE(waitFor([&] { return fired.load(); })); +} + +TEST_CASE("TimeoutScheduler: a callback that throws does not stop later callbacks from firing", + "[timeout_scheduler]") { + TimeoutScheduler scheduler; + std::atomic secondFired{false}; + + scheduler.schedule(1ms, [] { throw std::runtime_error{"first callback throws"}; }); + scheduler.schedule(5ms, [&secondFired] { secondFired = true; }); + + REQUIRE(waitFor([&] { return secondFired.load(); })); +} + +TEST_CASE("TimeoutScheduler: cancel() on an unknown handle is a no-op", "[timeout_scheduler]") { + TimeoutScheduler scheduler; + // Never returned by schedule() on this instance -- exercises the + // not-found arm of cancel() without racing a real entry. + scheduler.cancel(TimeoutScheduler::Handle{999999}); + SUCCEED("cancel() on an unknown handle returned without firing or throwing"); +} + +TEST_CASE("TimeoutScheduler: cancel() before the deadline prevents the callback from firing", + "[timeout_scheduler]") { + TimeoutScheduler scheduler; + std::atomic fired{false}; + + auto const handle = scheduler.schedule(50ms, [&fired] { fired = true; }); + scheduler.cancel(handle); + + // Give the (cancelled) deadline time to have elapsed, then confirm the + // callback never ran. + std::this_thread::sleep_for(80ms); + REQUIRE_FALSE(fired.load()); +} diff --git a/tests/test_wire_hardening.cpp b/tests/test_wire_hardening.cpp index 41c45491..5f6b6f37 100644 --- a/tests/test_wire_hardening.cpp +++ b/tests/test_wire_hardening.cpp @@ -35,6 +35,17 @@ // for a different reason now (see Bug E): an err message is log-bound text, // and a raw 0x1B in it would carry an ANSI escape into the reader's terminal. // +// Bug G (control bytes in the action/result codec): the Bug E gap, one layer +// down. An execute envelope's `body` is not written by `wire::encode` at all +// — it is produced by `ActionTraits::toJson` / `resultToJson` +// (registry.hpp's BRIDGE_REGISTER_ACTION macro), which wrote with plain +// `glz::write_json` and so reproduced Bug E exactly for every string field +// of every action and result. Action bodies are pure caller data (a paste's +// content, a chat message, a filename), so this is at least as exposed as +// the envelope was. Found by pastebin (ladder rung 1) replaying +// tests/fuzz/findings/ as paste content; fixed with the same instrument, +// `model::detail::EscapingWriteOpts`. +// // Bug E (control bytes in the remaining string fields): the same writer gap // applies to every `Envelope` string, not just `message` — `body`, // `modelType`, `actionType`, `contextKey`, `typeId`, and the session's @@ -50,6 +61,7 @@ #include #include #include +#include #include #include #include @@ -289,3 +301,70 @@ TEST_CASE("wire::detail::peekCallId cannot be spoofed from an earlier string fie env.callId = 5U; CHECK(morph::wire::detail::peekCallId(encode(env)) == 5U); } + +// ── Bug G: the same writer gap, one layer down, in the action/result codec ─── +// +// `wire::encode` escapes control bytes in the *envelope*, but an execute +// envelope's `body` is produced separately, by `ActionTraits::toJson` / +// `resultToJson` (registry.hpp's BRIDGE_REGISTER_ACTION macro). Those wrote +// with plain `glz::write_json` and so reproduced Bug E exactly: an action +// carrying a raw control byte in any of its string fields serialized to a body +// its own peer's `fromJson` then rejected — and, alongside an escaped +// character, was silently rewritten into two 0x00 bytes. Action bodies are +// pure caller data (a paste's content, a chat message, a filename), so this is +// at least as exposed as the envelope was. Found by pastebin (ladder rung 1) +// replaying tests/fuzz/findings/ as paste content; fixed with the same +// instrument, `model::detail::EscapingWriteOpts`. See docs/spec/core/registry.md, +// "Control bytes in action and result bodies". + +// Namespace scope, not anonymous: glaze's reflection needs external linkage on +// the reflected type (see test_backend_rig.cpp's identical note). +struct WireCtlAction { + std::string text; +}; +struct WireCtlResult { + std::string text; +}; +struct WireCtlModel { + WireCtlResult execute(WireCtlAction action) { return WireCtlResult{.text = action.text}; } +}; + +BRIDGE_REGISTER_MODEL(WireCtlModel, "WireCtlModel") +BRIDGE_REGISTER_ACTION(WireCtlModel, WireCtlAction, "WireCtlAction") + +TEST_CASE("ActionTraits::toJson escapes control bytes so the action body re-decodes", "[wire][hardening]") { + const std::string payload = ctl(); + const auto json = morph::model::ActionTraits::toJson(WireCtlAction{.text = payload}); + CHECK(morph::model::ActionTraits::fromJson(json).text == payload); +} + +TEST_CASE("ActionTraits::resultToJson escapes control bytes so the result body re-decodes", + "[wire][hardening]") { + const std::string payload = ctl(); + const auto json = morph::model::ActionTraits::resultToJson(WireCtlResult{.text = payload}); + CHECK(morph::model::ActionTraits::resultFromJson(json).text == payload); +} + +TEST_CASE("ActionTraits preserves the whole control range byte-for-byte", "[wire][hardening]") { + std::string all; + for (int byte = 0x00; byte < 0x20; ++byte) { + all.push_back(static_cast(byte)); + } + const auto json = morph::model::ActionTraits::toJson(WireCtlAction{.text = all}); + const auto back = morph::model::ActionTraits::fromJson(json); + CHECK(back.text == all); + CHECK(back.text.size() == 32U); +} + +TEST_CASE("ActionTraits survives a control byte alongside an escaped character", "[wire][hardening]") { + // The corrupting half of the failure mode, not merely the invalid-output + // half — see the identical sweep for `encode` above. The value comparison, + // not the absence of a throw, is the assertion that matters. + for (std::size_t pad = 0; pad <= 40; ++pad) { + std::string payload = "\\" + std::string(pad, 'x'); + payload.push_back(static_cast(0x0B)); + payload += "\"tail"; + const auto json = morph::model::ActionTraits::toJson(WireCtlAction{.text = payload}); + CHECK(morph::model::ActionTraits::fromJson(json).text == payload); + } +}