experiment: drop deferredEmit, rely on cpp-sdk #68 - #29
Draft
igor-sirotin wants to merge 2 commits into
Draft
Conversation
Reverts the runtime portion of e3a9078: removes the QueuedConnection deferral in libchat callbacks, the m_emitRouter QObject anchor, and the QObject/QMetaObject includes. Result callbacks now invoke impl->emitEvent() directly. Rationale: cpp-sdk d77c3dd (#68) routes provider events through the source thread via AutoConnection. If libchat's result callbacks were actually firing from a worker thread (rather than the synchronous same-thread invocation the original comment assumed), the AutoConnection fix subsumes the workaround. The e2e job on ubuntu-latest is the validation — re-runs that stay green on this branch indicate the workaround was redundant. If e2e exit-4 timeouts reappear, the original same-thread diagnosis holds and this revert should be backed out.
Pulls logos-module-builder forward so logos-cpp-sdk advances to d77c3dd (#68), which marshals provider events onto the source thread via AutoConnection.
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
1 participant
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.
Why
chat-module #24 added
deferredEmit(commit e3a9078) to fix e2e exit-4 timeouts on GHA ubuntu-latest. The diagnosis at the time attributed it to same-thread event-loop starvation: libchat result callbacks fire synchronously insideLOGOS_METHOD, keeping theQ_INVOKABLEslot on the stack soQLocalSocketcan't flush the QtRO reply before the caller's 20s deadline.cpp-sdk #68 (merged 2026-05-25) addresses a related-but-distinct symptom: events fired from a foreign thread raced QtRO source serialization against the method-reply flush, silently dropping replies. The final form uses
Qt::AutoConnection, which keeps same-thread emits synchronous and only queues cross-thread emits.If the original same-thread diagnosis was correct, this revert will re-introduce the 20s timeout. If the failures were actually cross-thread (libchat dispatches callbacks from a worker), AutoConnection in cpp-sdk #68 already covers it and
deferredEmitis redundant.What
revert: drop deferredEmit workaround for event-loop starvation— undoes the runtime portion of e3a9078: removes theQueuedConnectiondeferral, them_emitRouterQObjectanchor, and theQObject/QMetaObjectincludes. Result callbacks now callimpl->emitEvent()directly.chore: bump logos-cpp-sdk for provider-event threading fix—nix flake update logos-module-builderadvances cpp-sdk tod77c3dd(#68).Validation
CI's
integration-testsjob on ubuntu-latest is the discriminator — that's the runner where the original exit-4s reproduced. Several consecutive green runs ⇒ the workaround was redundant. Any exit-4 timeouts ⇒ back this out.Draft until CI confirms.