diff --git a/cpp/logos_api_client.cpp b/cpp/logos_api_client.cpp index 0a4bcad..d3db4d7 100644 --- a/cpp/logos_api_client.cpp +++ b/cpp/logos_api_client.cpp @@ -144,6 +144,21 @@ void LogosAPIClient::invokeRemoteMethodAsync(const QString& objectName, const QS { if (!callback) return; + // The async path acquires a replica too, so it must also run on the owner + // thread. Unlike the sync path we post non-blocking (QueuedConnection): the + // worker caller returns immediately and the result callback fires on the + // owner thread when the reply arrives. + if (QThread::currentThread() != this->thread()) { + QMetaObject::invokeMethod(this, + [this, objectName, methodName, args, + callback = std::move(callback), timeout]() mutable { + invokeRemoteMethodAsync(objectName, methodName, args, + std::move(callback), timeout); + }, + Qt::QueuedConnection); + return; + } + QString token = getToken(objectName); if (token.isEmpty() && objectName != "capability_module" && m_capability_consumer) { diff --git a/cpp/logos_thread_marshal.h b/cpp/logos_thread_marshal.h index de50830..55a5f18 100644 --- a/cpp/logos_thread_marshal.h +++ b/cpp/logos_thread_marshal.h @@ -19,13 +19,20 @@ namespace logos { // thread) without the module touching Qt — the SDK transparently marshals the // call onto the owner thread. // -// Requirements: `obj`'s thread must be running an event loop (it is — the -// module's main thread runs QCoreApplication::exec()). The same-thread guard -// avoids the BlockingQueuedConnection self-deadlock. +// Requirements: +// - `obj`'s thread must be running an event loop (it is — the module's main +// thread runs QCoreApplication::exec()). The same-thread guard avoids the +// BlockingQueuedConnection self-deadlock. +// - The return type must be void or default-constructible (the marshaled +// branch holds the result in a local before assigning it), and must not be +// a reference (there'd be nothing to bind the local to). Both are satisfied +// by the SDK's uses here (void, QVariant, LogosObject*, LogosAPIClient*). template auto runOnOwnerThread(QObject* obj, Fn&& fn) -> decltype(fn()) { using Ret = decltype(fn()); + static_assert(!std::is_reference_v, + "runOnOwnerThread does not support reference return types"); if (QThread::currentThread() == obj->thread()) { return fn(); } diff --git a/tests/sdk/test_worker_thread_ipc.cpp b/tests/sdk/test_worker_thread_ipc.cpp index 6ef31cd..ca0495a 100644 --- a/tests/sdk/test_worker_thread_ipc.cpp +++ b/tests/sdk/test_worker_thread_ipc.cpp @@ -76,10 +76,15 @@ TEST_F(WorkerThreadIpcTest, InvokeFromWorkerThreadRunsOnOwnerThread) { QThread* const ownerThread = QThread::currentThread(); + // The ModuleProxy created by registerObject (owned by providerApi) keeps a + // raw pointer to the provider while published, so the provider must outlive + // it. Declaring `provider` before `providerApi` ensures that: at end of + // scope, providerApi (and its proxy) is destroyed first, then the provider. + ThreadProbeProvider provider; + // Provider registered on the owner thread. LogosAPI providerApi("thread_probe"); - auto* provider = new ThreadProbeProvider(); - ASSERT_TRUE(providerApi.getProvider()->registerObject("thread_probe", provider)); + ASSERT_TRUE(providerApi.getProvider()->registerObject("thread_probe", &provider)); // Authorize the token the consumer will present, so the call reaches the // provider instead of being rejected by the authz check. providerApi.getProvider()->saveToken("caller", "tok"); @@ -117,10 +122,6 @@ TEST_F(WorkerThreadIpcTest, InvokeFromWorkerThreadRunsOnOwnerThread) // The crux: the inter-module call must execute on the owner thread, not the // worker thread. Without the marshaling fix it runs on the worker thread. - EXPECT_EQ(provider->calledThread.load(), ownerThread) + EXPECT_EQ(provider.calledThread.load(), ownerThread) << "inter-module call executed on the worker thread instead of the owner thread"; - - // provider is intentionally leaked: the ModuleProxy created by - // registerObject (owned by providerApi) references it for the lifetime of - // the LogosAPI, which outlives this scope. }