mirror of
https://github.com/logos-co/logos-protocol.git
synced 2026-08-27 20:11:07 +00:00
Dario LipicarandClaude Opus 5
ae2f7e1b58
lp_client_create() made the CALLING thread the client's owner thread. Callers reach it through a lazily-created wrapper (the generated bind_<iface>() -> LpClient::ensure()), so the first thread to make an outbound call captured the whole transport for the life of the process. For the qt_remote transport that thread also ends up owning the QRemoteObjectNode and its QLocalSocket, which are only serviced by a thread running a Qt event loop. A module whose first call came from a worker — an HTTP handler, a timer thread — bound its transport to a thread that only pumps events while it is already blocked inside a call. Replica acquisition then never completed: every requestObject() burned its full 20s timeout and returned nullptr, and since a failed acquire yields an empty result the data loss was silent. openmetrics-module hit exactly this: one GET /metrics took 40s (2 x 20s) and came back missing a module, /health went unanswered behind the wedged libmicrohttpd thread, and the follow-up stop RPC failed. Construct the client on the Qt main thread when the transport needs a Qt event loop, so the per-call marshal that already exists (logos::runOnOwnerThread) lands somewhere that can actually service it. This is the anchor the Qt path always had — LogosAPI::getClient marshals construction to the LogosAPI's thread — given to the lp path. Plain (tcp/tcp_ssl) and mock transports are Qt-free and thread-agnostic, so they keep the calling thread: a worker-thread consumer stays off the main thread's back. LogosTransportFactory::needsQtEventLoop() carries that rule next to the createConnection resolution it mirrors. When there is nothing to anchor to (a Qt-affine transport with no QCoreApplication) we now warn instead of letting it surface as a mute timeout. Tests: a worker thread creates an lp client over qt_remote and calls a provider published on the main thread; passes in ~0.15s, and with the construction hop reverted fails after 24.8s / 49.9s — the acquire timeouts themselves. Plus a truth table for needsQtEventLoop. 183/183 protocol tests pass. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
108 lines
3.7 KiB
C++
108 lines
3.7 KiB
C++
#include <gtest/gtest.h>
|
|
#include "logos_mode.h"
|
|
#include "logos_transport.h"
|
|
#include "logos_transport_factory.h"
|
|
#include "mock_transport.h"
|
|
|
|
class TransportFactoryTest : public ::testing::Test {
|
|
protected:
|
|
void SetUp() override
|
|
{
|
|
m_saved = LogosModeConfig::getMode();
|
|
}
|
|
void TearDown() override
|
|
{
|
|
LogosModeConfig::setMode(m_saved);
|
|
}
|
|
LogosMode m_saved;
|
|
};
|
|
|
|
TEST_F(TransportFactoryTest, MockModeCreatesHost)
|
|
{
|
|
LogosModeConfig::setMode(LogosMode::Mock);
|
|
auto host = LogosTransportFactory::createHost("local:test");
|
|
ASSERT_NE(host, nullptr);
|
|
// Verify it's a MockTransportHost by calling publishObject (no-op, returns true)
|
|
EXPECT_TRUE(host->publishObject("test", nullptr));
|
|
}
|
|
|
|
TEST_F(TransportFactoryTest, MockModeCreatesConnection)
|
|
{
|
|
LogosModeConfig::setMode(LogosMode::Mock);
|
|
auto conn = LogosTransportFactory::createConnection("local:test");
|
|
ASSERT_NE(conn, nullptr);
|
|
EXPECT_TRUE(conn->isConnected());
|
|
EXPECT_TRUE(conn->connectToHost());
|
|
}
|
|
|
|
TEST_F(TransportFactoryTest, MockConnectionRequestObject)
|
|
{
|
|
LogosModeConfig::setMode(LogosMode::Mock);
|
|
auto conn = LogosTransportFactory::createConnection("local:test");
|
|
LogosObject* obj = conn->requestObject("my_module", 5000);
|
|
ASSERT_NE(obj, nullptr);
|
|
obj->release();
|
|
}
|
|
|
|
TEST_F(TransportFactoryTest, LocalModeCreatesHost)
|
|
{
|
|
LogosModeConfig::setMode(LogosMode::Local);
|
|
auto host = LogosTransportFactory::createHost("local:test");
|
|
ASSERT_NE(host, nullptr);
|
|
}
|
|
|
|
TEST_F(TransportFactoryTest, LocalModeCreatesConnection)
|
|
{
|
|
LogosModeConfig::setMode(LogosMode::Local);
|
|
auto conn = LogosTransportFactory::createConnection("local:test");
|
|
ASSERT_NE(conn, nullptr);
|
|
EXPECT_TRUE(conn->isConnected());
|
|
}
|
|
|
|
// needsQtEventLoop mirrors the createConnection resolution rule: it answers
|
|
// "must this connection be owned by a thread running a Qt event loop?". It is
|
|
// what lp_client_create() consults to decide whether to anchor a client to the
|
|
// Qt main thread, so a wrong answer here is either a hang (missing anchor) or
|
|
// a needless main-thread hop for a Qt-free transport.
|
|
TEST_F(TransportFactoryTest, NeedsQtEventLoopFollowsTheResolutionRule)
|
|
{
|
|
LogosTransportConfig localSocket; // default protocol
|
|
localSocket.protocol = LogosProtocol::LocalSocket;
|
|
LogosTransportConfig tcp;
|
|
tcp.protocol = LogosProtocol::Tcp;
|
|
LogosTransportConfig tcpSsl;
|
|
tcpSsl.protocol = LogosProtocol::TcpSsl;
|
|
|
|
LogosModeConfig::setMode(LogosMode::Remote);
|
|
EXPECT_TRUE(LogosTransportFactory::needsQtEventLoop(localSocket))
|
|
<< "qt_remote owns a QRemoteObjectNode + QLocalSocket";
|
|
EXPECT_FALSE(LogosTransportFactory::needsQtEventLoop(tcp))
|
|
<< "the plain transport is Qt-free by design";
|
|
EXPECT_FALSE(LogosTransportFactory::needsQtEventLoop(tcpSsl));
|
|
|
|
// Mode wins over cfg.protocol, exactly as in createConnection.
|
|
LogosModeConfig::setMode(LogosMode::Local);
|
|
EXPECT_TRUE(LogosTransportFactory::needsQtEventLoop(tcp))
|
|
<< "local mode invokes in-process QObjects owned by the main thread";
|
|
|
|
LogosModeConfig::setMode(LogosMode::Mock);
|
|
EXPECT_FALSE(LogosTransportFactory::needsQtEventLoop(localSocket))
|
|
<< "mock has no Qt objects and no sockets";
|
|
}
|
|
|
|
TEST_F(TransportFactoryTest, ModeSwitchChangesTransportType)
|
|
{
|
|
LogosModeConfig::setMode(LogosMode::Mock);
|
|
auto mockConn = LogosTransportFactory::createConnection("url");
|
|
// Mock is always connected
|
|
EXPECT_TRUE(mockConn->isConnected());
|
|
|
|
LogosModeConfig::setMode(LogosMode::Local);
|
|
auto localConn = LogosTransportFactory::createConnection("url");
|
|
// Local is also always connected
|
|
EXPECT_TRUE(localConn->isConnected());
|
|
|
|
// They should be different transport implementations
|
|
EXPECT_NE(mockConn.get(), localConn.get());
|
|
}
|