core+qt: add opt-in asynchronous model registration - #36
Open
Yaraslaut wants to merge 2 commits into
Open
Conversation
Bridge registers a model by calling IBackend::registerModel, which is
synchronous -- it must have the server-assigned ModelId before it returns.
QtWebSocketBackend implements that via a nested QEventLoop parked until the
reply arrives. On a WASM main thread Qt refuses to do this at all
("WaitForMoreEvents is not supported on the main thread without asyncify")
and aborts the module, so a Qt/QML client compiled to WebAssembly cannot
register a single model against a remote backend -- the very first
registerModel call kills the page.
Add IBackend::registerModelAsync(typeId, factory, contextKey, onRegistered,
onError): an optional non-blocking counterpart defaulting to "unsupported"
(returns false, calls neither callback) so every backend that hasn't opted
in is unaffected. Bridge::registerHandler() (both overloads) now prefers this
path when a backend offers one, adding the binding to _handlers and issuing
the async call before taking _mtx (so a backend that -- unlike any documented
here -- invoked a callback synchronously could not self-deadlock), falling
back to the synchronous registerModelWithContext otherwise. The success
callback guards against a switchBackend() racing ahead of a still-pending
registration (same weak_ptr<IBackend> + liveness-token pattern
installReconnectHandler already uses) so a stale reply can never overwrite
the fresh id switchBackend's own re-registration already assigned.
QtWebSocketBackend overrides registerModelAsync, matching execute()'s
existing callId-based reply matching (the server already echoes callId on
every reply kind, register included -- no protocol change needed). Gated by
a new QtWebSocketBackendConfig::asyncRegistrationEnabled, defaulting to
false: making the override unconditional broke the existing Qt test suite
(measured directly -- 20 of 50 test cases failed) because any caller that
fires an action immediately after constructing a BridgeHandler, assuming
synchronous-enough-to-execute-next-line registration, now sees "handler not
bound" instead. Defaulting the flag off preserves every existing embedder's
behavior exactly; a WASM build opts in explicitly and adapts its call sites
to wait for registration before firing an action.
Scope: only the plain (non-shared) registration path -- a BridgeHandler's
initial construction -- goes through registerModelAsync. registerModelShared/
attachModel (shared/keyed handlers) and the re-registration switchBackend()/
the reconnect handler perform after a backend swap remain synchronous; making
those async too is a larger change to Bridge's locking model, left for a
follow-up if it proves necessary.
Closes #26
Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Doxygen's WARN_AS_ERROR build fails on any undocumented parameter; QtWebSocketBackend::registerModelAsync's factory param (unused — this backend has no local model to construct) was missing its @param tag. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bridgeregisters a model viaIBackend::registerModel, which is synchronous.QtWebSocketBackendimplements that with a nestedQEventLoopparked until the reply arrives. On a WASM main thread, Qt refuses to spin a nested loop at all, so the very firstregisterModelcall aborts the page — exactly the issue's repro.IBackend::registerModelAsync(typeId, factory, contextKey, onRegistered, onError), defaulting to "unsupported" (false, calls neither callback) so every backend that hasn't opted in is unaffected.Bridge::registerHandler()(both overloads) prefers this path when a backend offers one, falling back to the synchronous call otherwise.switchBackend()racing ahead of a still-pending registration, reusing the exactweak_ptr<IBackend>+ liveness-token patterninstallReconnectHandleralready uses — a stale reply can never clobber the fresh idswitchBackend's own re-registration already assigned. Verified with a dedicated regression test.QtWebSocketBackendoverridesregisterModelAsync, matchingexecute()'s existingcallId-based reply matching (no protocol change needed — the server already echoescallIdon every reply kind,registerincluded).BridgeHandler(assuming synchronous-enough registration) now sees "handler not bound" instead. Gated the override behind a newQtWebSocketBackendConfig::asyncRegistrationEnabled, defaulting tofalse: every existing embedder's behavior is unchanged; a WASM build opts in explicitly and adapts its call sites to wait for registration before firing an action. Confirmed by running the real Qt/WebSocket test suite against Qt6 both before and after this fix.BridgeHandler's initial construction — goes through this.registerModelShared/attachModel(shared/keyed handlers) and the re-registrationswitchBackend()/reconnect-handler paths remain synchronous; making those async too is a larger change toBridge's locking model, left for a follow-up if it proves necessary.docs/spec/core/backend.md(new "Asynchronous registration" section,IBackend/QtWebSocketBackendAPI reference rows) anddocs/spec/core/bridge.md(registerHandlerbehavior).Test plan
tests/test_async_registration.cpp(new): async path preferred and binding starts unbound; async execute works once registration completes;onErrorleaves the binding unbound with no crash; a stale reply afterswitchBackend()does not clobber the new id; a reply arriving after~Bridge()is a safe no-op; a backend with no async support still falls back to the synchronous path.tests/qt/test_qt_websocket.cpp(new case): end-to-end against a real Qt WebSocket server — registration doesn't block, and execute succeeds once the deferred registration completes../build/tests/morph_tests— all 817 test cases / 8305 assertions pass.-DMORPH_BUILD_QT=ON): 51 test cases / 340 assertions pass — confirmed byte-for-byte matching the pre-change baseline (334 assertions / 50 cases) plus the one new case.Closes #26
🤖 Generated with Claude Code