mirror of
https://github.com/logos-co/logos-protocol.git
synced 2026-08-31 14:01:14 +00:00
fix/bound-request-module-timeout
13
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dario LipicarandClaude Opus 5
|
07b0fb1c64 |
fix: make event subscriptions survive a module that is not reachable yet (#47)
* fix: make isConnected() mean connected, and stop the log claiming it QRemoteObjectNode::connectToNode() returns false only when the URL SCHEME is unregistered -- it never contacts the peer. Our registry URLs are COMPUTED rather than discovered (logos_instance.h: local:logos_<module>_<instanceId>), so they are identical whether or not the module exists. Latching m_connected from that return therefore made isConnected() answer "yes" for modules that were never loaded, which made every `if (!client->isConnected()) return;` guard in the codebase DEAD CODE. Callers then paid a 20 s waitForSource per call, twice over, because the token handshake tries capability_module first. Measured in Basecamp with package_manager absent: ~417 s of blocked GUI thread on macOS and 361 s on Linux before the window appeared, and over 900 s under load. Not a Windows bug -- the Windows port merely exposed it. isConnected() now also requires a listener at the endpoint. For `local:` that is a direct socket / named-pipe probe, which costs microseconds precisely in the case that used to cost 20 seconds; any other scheme keeps its previous behaviour. Two logging changes, because the diagnostics cost more than the defect: "Successfully connected to registry" asserted a connection that often did not exist and sent three separate investigations to the wrong place -- it now says a connect attempt started and makes no claim about the peer. And requestObject warns BEFORE a doomed wait instead of going silent for 20 s and then reporting failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: let event subscriptions survive a module that is not reachable yet requestObject() answers "is the module there RIGHT NOW", and every subscriber in this codebase asks at the one moment the answer is no: a module's init(), a UI backend's onContextReady(), a QML view's Component.onCompleted. All of those run while the dependency's host process has been spawned but has not called listen() yet. The subscriber then gave up permanently -- lp_subscribe returned nullptr with no log at all, and callers turned that into a `false` the documented example discards. Method calls kept working through the same window because acquireCachedObject() reaches the replica by a path that never asks, so the symptom was "events are broken", not "the subscription never happened". |
||
|
Dario LipicarandClaude Opus 5
|
dda5dae1bf |
test(plain): bound the burst-drain assertion against the burst, not a constant (#56)
BurstThatGoesIdleDrainsWithoutAnotherCall failed on ubuntu-latest at 21, then at
10 on a re-run, against EXPECT_LE(idle, 8u) — having scored 7 against that same
8 the run before. The change under review is not involved: the same source
compiles to a byte-identical object file with and without it.
WHAT THE RESIDUE IS. A waiter reaps only OTHERS, never itself, so what survives
an idle burst is whatever published after the FINAL reap: the last waiter to
finish has nobody behind it, and a waiter sitting in the join loop of its own
reap has not published yet while the batch it did not collect already has. That
is the size of the last exit batch, which is the scheduler's business.
AND NOTHING TAKES IT LATER, so this is not a window that was too short. Sampled
from 100ms to 25.6s after the burst went quiet the count does not move: 5->5 and
17->17 idle, 2->2 under 4x CPU oversubscription, and 24->24, 33->33, 49->49,
82->82, 123->123, 138->138, 168->168 under 32x — 12 runs, every one flat. It is
a residue, not a drain in progress.
MEASURED, 20 runs per cell, this 800-call burst, m_waiters when it goes idle:
this code reaping only on the spawn path
macOS idle 1 every run 572-723
Linux idle 1-80 (median 9) 4-168 (median 80)
Linux 2x CPU 1-145 (median 14) 16-527 (median 275)
Linux 4x CPU 1-104 (median 28) 10-504 (median 271)
Two things fall out of that, and the second is why this commit says more than
"the number was too small".
1. 8 WAS READ OFF THE macOS COLUMN. On Linux it sits under the MEDIAN of a
correct build — 35 of 60 unloaded runs of correct code exceed it — so the
test was failing correct code in most Linux runs. CI's 7 was luck.
2. THE ~600 THIS TEST IS DOCUMENTED AGAINST IS macOS-ONLY. On Linux the burst
is not concurrent: spawning 800 std::threads costs more than a loopback
ping, so most of it has already been collected by the SPAWN-path reaper
before the last call is issued, and the defect's own residue collapses
into the same range as a correct build's (4-168 idle, min 4). The two arms
overlap there at any bound, 8 included. This assertion is a coarse
retention check on Linux, not the detector for that defect.
THE NEW BOUND is kBurst/2 — the majority of the burst must have retired itself
with no further call — because the residue has no ceiling for a tighter
fraction to sit under. Worst per load level, 620 runs on a 6-core Linux box:
idle 152 (n=60) 2x 145 (n=20) 4x 172 (n=80) 8x 175 (n=160)
16x 278 (n=120) 32x 402 (n=60) 64x 317 (n=40)
Flat out to 8x, climbing after. A quarter of the burst (200) would have been
the original mistake in a new unit: it clears the worst by 1.14x, the same
ratio as 7-against-8. Half clears everything up to 16x by 1.44x and the worst
CI has ever produced (21) by 19x. The single run in 620 that scored 402, at 32x
oversubscription, is recorded in the comment rather than rounded away.
Both assertions in the test take the same expression, the second included: a
residue the follow-up call did not collect is the same retention bug, and a
tighter hard-coded number there would only move the magic constant somewhere
quieter.
Also corrects the retention note in plain_logos_object.h, which quoted "1-2
after a 2000-call burst" as though it were platform-independent.
THE DETECTOR, rebuilt with the defect this test exists to catch — the reap
dropped from the waiter's exit guard, leaving only the spawn path:
this assertion, macOS RED 10/10, 550-614 against 400
this assertion, Linux 16x RED 4/10, up to 645
this assertion, Linux idle GREEN 0/15, 25-208 — see below
publish-is-last, macOS RED 5/5
publish-is-last, Linux RED 8/8 (green 3/3 with the reap in place)
nix build '.#tests' fails its own checkPhase with the defect in
The third line is a real loss of Linux coverage in THIS assertion and it is
stated in the comment rather than glossed: on an unloaded Linux box no bound
that a correct build survives will catch it, because the burst is not
concurrent there. It costs the SUITE nothing — with the exit-guard reap gone,
PublishedWaiterDoesNotTouchTheRegistryAgain is RED deterministically on both
platforms, and it is that test, not this one, that pins the reap. If this one
ever has to be the detector again, the answer is to pace the provider so the
burst is concurrent on every platform, not to tighten the number.
No behaviour change: the only non-comment edit is the bound.
VERIFIED: nix build '.#tests' green on macOS (289/289, 69.6s) and Linux
(289/289, 78.3s).
(cherry picked from commit
|
||
|
|
0f26ffdeef |
fix(protocol): report the failures that happen AFTER acquire — on both twins, without moving the ABI (#41)
* fix(lp): lp_invoke_async can finally report a failure
lp_result_cb has always been documented as carrying an outcome —
"ok != 0 -> `json` is the result JSON value; ok == 0 -> `json` is the
canonical error object" — and the synchronous twin lp_invoke has always
honoured it (LP_ERR_UNAVAILABLE + out_error_json). lp_invoke_async did
not: it subscribed with the VALUE-ONLY invokeRemoteMethodAsync overload
and called back `cb(1, json, user_data)` with ok hard-coded to 1, so a
call to a module that cannot be acquired reached the callback as a
SUCCESS carrying a default-constructed value.
The fix is to pass a TWO-argument lambda, which is invocable only as
LogosAPIClient::AsyncResultErrorCallback and so binds to the
CallError-aware overload that already exists next to the value-only one.
The failure is then rendered with the same makeErrorJson the sync path
uses, so both entry points report the same event in the same shape.
The ABI is unchanged. lp_result_cb's (ok, json, user_data) signature
already reserves ok == 0 for exactly this; nothing had to grow a new
entry point, and every in-tree consumer already branches on `ok`
(logos-rust-sdk's async_call_trampoline even parses `message` out of the
canonical error object — code written against a contract the
implementation never kept).
Regression test: a matched pair over a REAL transport (plain TCP), not
the mock.
FAILING async call -> ok=0 {"code":"object_unavailable", ...}
SUCCEEDING async call -> ok=1 7
The first fails on the unfixed tree (ok=1, json "null"); the second
passes on both, so an over-eager "report failure everywhere" fix cannot
sneak through.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(protocol): report the failures that happen AFTER acquire, on both twins
#40 made lp_invoke_async able to report a failure, but only for the two
conditions produced ABOVE the transport: acquire failure and the unauthorized
sentinel. Everything the transport learns while the call is in flight was still
discarded — PlainLogosObject answered a bare QVariant() for a timeout and for
`ResultMessage.ok == false` alike, and LogosAPIConsumer hard-coded an empty
CallError next to it.
Two ordinary failures therefore still reported success on both entry points:
a TIMEOUT, and MODULE NOT LOADED against a host that is up (which is not an
acquire failure on the plain wire — requestObject hands back a handle for any
name over an open connection).
The information already exists: ResultMessage carries err/errCode, the futures
know they expired, QtRO knows its pending call never finished. It had nowhere to
go because LogosObject's callMethod returns a lone QVariant and its
callMethodAsync callback takes a lone QVariant.
Widening those virtuals would append a vtable slot to an installed, subclassed
interface, so instead this adds LogosObjectErrorChannel — a SIBLING interface
reached by dynamic_cast. LogosObject's size, layout and vtable are unchanged
(verified: a subclass compiled against the old and new headers emits the same
14-entry vtable with identical slot indices), and a transport that does not
implement it keeps today's behaviour.
logos_protocol.cpp needs no change: lp_invoke and lp_invoke_async already render
this CallError, so both twins gain the coverage together.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(protocol): stop the macOS flake that was sinking #41
Three real races the new CallErrorAfterAcquire suite exposed (and that
Copilot flagged on the QtRO half):
1. ~PlainTransportHost stopped the acceptor but did not quiesce the shared
Asio I/O thread. Server-side RpcConnections hold a raw IncomingCallHandler*
back to the host; a fail()/onConnectionClosed racing teardown freed the
handler mid-call. That is the macOS CI SIGSEGV in
AsyncSuccessStillReportsTheValue — it fires with no output of its own
because the previous live-host test's destructor left the heap corrupted.
Restore the I/O barrier that landed on the qtfree branches but never on
master (proven: 80/80 clean on the CI crash sequence that was ~2/50 before).
2. PlainLogosObject::callMethodAsync detached its per-call waiter while
capturing `this`. release()/delete this could then race the waiter.
Join waiters in the destructor/release, and register the thread under the
lock before it can outrun teardown.
3. QtRO async could deliver the user callback twice when the timeout timer
and the pending-call watcher finished around the same moment, violating
the exactly-once contract. Gate both paths (and the deferred-completion
arm) on one atomic.
Also drain queued onCall invokes after host.reset() in the #40 live-target
control, matching LiveHost's teardown discipline.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(protocol): the drain barrier must not dangle on its own timeout
Two defects in the barrier added by
|
||
|
Dario LipicarandClaude Opus 5
|
3a31c91d13 |
fix(plain): close the RPC acceptor on the server's strand, not the caller's thread (#39)
RpcServerTcp::stop() and RpcServerSsl::stop() closed m_acceptor on whatever thread called them — in practice the host thread, via ~PlainTransportHost — while doAccept() re-armed async_accept from inside its own completion handler, on the io worker. Nothing serialized the two. This is the acceptor half of the race PR #38 fixed for RpcConnection, and it fails identically: asio acceptors are "Shared objects: Unsafe", and close() runs cleanup_descriptor_data(), which nulls the reactor's per-descriptor state while reactive_socket_service_base::start_op() holds it by reference. It was left out of #38 because every backtrace captured in the wild was a write initiation, never an accept — but it reproduces on demand: EXC_BAD_ACCESS KERN_INVALID_ADDRESS at 0x98 logos::plain::RpcServerTcp::doAccept() ...reactive_socket_move_accept_op<...>::do_complete(...) logos::plain::IoContextPool::IoContextPool()::$_0 <- io worker thread Both servers now own a strand. doAccept()'s completion handler is bind_executor'd onto it (so the re-arm runs there) and stop() hands the close to it with dispatch() — inline when already on the strand, queued and non-blocking from anywhere else, exactly as RpcConnection::closeStreamOnStrand does. start() still runs open/bind/listen inline: callers read boundPort() the moment it returns. That is safe because no async op on the acceptor exists yet, and PlainTransportHost serializes start()/stop() under its own mutex. Only the accept loop moves onto the strand, which is invisible to clients — listen() has already run, so an early connect waits in the backlog. Deferring the close leaves the listener open for the microseconds between stop() returning and the strand running it, so a connection can still be accepted in that gap. The accept path therefore tests m_stopped and publishes the connection under one lock, and drops a late socket instead of wrapping it in a connection and stop()ing it — conn->stop() would call onConnectionClosed() on the IncomingCallHandler whose destructor started this teardown. The TLS server gets the same guard, where it was already latent: an async_handshake in flight was never aborted by closing the acceptor. Adds RpcServerTeardownTest: a start/connect/stop stress loop shaped like test_rpc_connection_teardown.cpp, plus a round-trip check that a client connecting the instant start() returns is still served. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 5
|
4db061aba3 |
fix(plain): close the RPC socket on the connection's strand, not the caller's thread (#38)
* fix(plain): close the RPC socket on the connection's strand RpcConnection<Stream>::fail() closed the socket on whatever thread called it. Every other access to m_stream is serialized on m_strand — start() and writeFrame() post onto it, doRead()/doWrite() complete through bind_executor(m_strand, ...) — but a strand serializes handlers, not a raw call made from outside it, and asio sockets are documented as unsafe for concurrent use. Consumer teardown (~RpcClient -> ~PlainTransportConnection -> stop() -> fail()) therefore ran close() -> cleanup_descriptor_data(), nulling impl.reactor_data_, while the io worker thread was inside reactive_socket_service_base::start_op() for a doWrite() that had just been posted. start_op()'s 'descriptor_data' is a reference to that member: the null check passes before the store lands, then the shutdown_ read after it dereferences null. SIGSEGV at +0x98 on the IoContextPool thread. fail() now hands the close to the strand via boost::asio::dispatch, which runs it inline when fail() is already on the strand (the io-thread error path, unchanged behaviour) and queues it otherwise. dispatch never blocks, so teardown cannot deadlock or hang; the lambda holds a shared_ptr so a close queued from a destructor still finds a live object. writeFrame()'s m_stopped check is also repeated inside the posted lambda and in doWrite(): the outer load is only a hint, and fail() can land between it and the handler. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(plain): a teardown-race regression that also guards against leaks and hangs Hammers the shape that crashed: a consumer connection with frames still queued is destroyed from its own thread, 400 times over, while the io worker is initiating the async_write for a just-posted frame. Pre-fix this takes the whole test binary down inside asio's reactor; post-fix the close runs on the strand and can never overlap a write initiation. The same loop is the guard for the two things the fix could plausibly break: the descriptor count must come back (an async close that never runs would strand fds) and the loop must finish promptly (a close that blocked on the io thread would show up as a stall). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(plain): stop dispatching inbound frames once the connection failed Moving the close onto the strand left the socket open between stop() returning and the strand getting to it. A frame that arrived in that gap still ran through handleFrame -> dispatchIncoming and into the IncomingCallHandler — which, on the host side, the caller may already be in the middle of destroying (RpcServer::stop() runs from ~PlainTransportHost). Before the close moved, the immediate close aborted the read and that frame never landed. The connection is torn down either way: every pending promise has already been failed and every event callback cleared, so there is nothing a late frame could usefully resolve. Drop it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 5
|
c0df466172 |
fix: integer signedness in the codec, and a shape check on the pending-call sentinel (#31)
* fix(codec): signedness and range are part of the integer type Codec<T>::from accepted any integral JSON number and handed it to .get<T>(). That is silent in both directions: .get<uint64_t>() on -1 -> 18446744073709551615 (a sign flip) .get<int32_t>() on 2^40 -> truncated Both now reject with the usual path-carrying CodecError instead. Rejecting is the codec's existing contract — a value the declared type cannot represent must not reach business logic wearing a different one — this just extends it to the half of the integer domain it was skipping. Note the check is on the JSON category, not the value: a negative literal parses as number_integer and never as number_unsigned, so `is_number_unsigned()` is the reliable discriminator rather than a comparison after conversion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(async): the pending-call sentinel is matched by shape, not by key presence All four detection sites tested `m.contains(pendingCallKey())` and nothing else, so ANY user map carrying that key was taken for a deferred call: the consumer extracted a call id, found no completion, and waited out a nested event loop. The measured outcome is a ~20s HANG, not a fast failure. An `any` slot is enough to reach it — anything a user can put in a map. logos::isPendingCallSentinel now requires the canonical shape: exactly one entry, under the sentinel key, holding a non-empty string. Shape and signature are mirrored from isUnauthorizedSentinel (logos_rpc_status.h), QJsonObject arm included — the two are the same kind of in-band marker and there was no reason for them to be guarded differently. That guard, and isTaggedBytes's, both already existed in this repo; the difference was chronology, not principle. Behaviour-preserving: the generated glue builds this map with exactly one entry whose value is a QString call id, so no real sender changes. The concurrent dispatch tests pass unchanged. NARROWS, DOES NOT CLOSE — and the tests say so out loud. A one-key, string-valued forgery IS the sentinel; no predicate can separate them. It still hangs, and because call ids are a per-object counter from 0, a forged "lc-0" can collide with a genuine in-flight completion and steal its result. Closing that needs an out-of-band channel for "deferred", which the single-QVariant dispatch slot cannot express without an ABI break — the constraint is stated at logos_rpc_status.h:24-27 and is real. tests: 10 new, including one asserting the forgery still matches, so a future reader cannot mistake the green cells for "the sentinel is safe". 236/236. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 5
|
8b8a358c8b |
fix: uint64 survives the event path and the plain wire (#30)
* fix(events): the event bridge converts through the canonical helper
setEventListenerStdBridge adapts the universal event callback (name + JSON
string) to the Qt EventCallback (name + QVariantList). It is the event-path
counterpart of callMethodStdBridge, but it did the conversion itself:
callMethodStdBridge -> logos::nlohmannToQVariant (canonical)
setEventListenerStdBridge -> QJsonDocument::fromJson
+ QJsonValue::toVariant (Qt's parser)
Two consequences, both measured by the LIDL conformance matrix as M6:
* a uint64 above int64max degraded to a double. Qt 6 backs QJsonValue with
QCborValue, so integers up to int64 DID survive — only values with no
integral representation there fell back to double. echoUint(2^64-1) was
exact while uintEvent(2^64-1) arrived as 1.8446744073709552e+19: same
value, same process, one hop later.
* canonical tagged bytes {"_bytes": ...} were not decoded, arriving as a
QVariantMap where the method path yields a QByteArray. This never showed up
end-to-end because the undecoded map round-trips to JSON and the python
client decodes the tag itself — but a C++ or QML event subscriber got a map.
Both now go through logos::nlohmannArgsToQVariantList, which the generated
cdylib emitTrampoline already used. Numbers and bytes no longer depend on
whether a value left the module as a return or as an event.
Not the residue of the codec convergence, despite how M6 was originally
registered. #29 converged six copies of the VALUE codec; this was a seventh
conversion inside an ADAPTER, which that scope never touched. It is also not on
the providers' own path — a Qt provider stores its callback verbatim and a
cdylib provider already converted correctly. The one live caller is the
logoscore daemon's CoreServiceImpl, which forwards every watched module event;
that is why C++ and Rust providers measured identically.
Why it survived: the bridge appeared in the test suite once, in
test_universal_provider_dispatch.cpp, purely to satisfy the pure virtual. No
test asserted anything about an event payload. The method path got 15 contract
tests in #29; the event path got none.
tests: 11 new cells pin the bridge directly — uint64 past int64max, 2^53+1,
int64::min, large integers nested in containers, tagged bytes at top level and
at depth, plus the shapes that already worked (multi-param order, double staying
double, null elements, empty payload, the non-array raw-string fallback) so a
future rewrite cannot quietly drop them. 210/210.
verified: logos-cpp-sdk, logos-qt-sdk, logos-liblogos and logos-logoscore-cli
all green against this build; the conformance matrix goes 156 -> 158 pass with
M6's two cells retired, and the ext table stays 40/40.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(events): pin the signedness rule the convergence brings with it
nlohmannArgsToQVariantList classifies every non-negative integer as unsigned, so
a LIDL `int` event argument now arrives as ULongLong where it used to be
LongLong. That matches what nlohmannToQVariant (the method path) and the cdylib
emitTrampoline already did — the surfaces now agree — but it is an observable
metatype change that nothing asserted.
Pinned in both directions (non-negative -> ULongLong, negative -> LongLong) so
it stays a decision rather than a side effect. Value-level reads are unaffected.
212/212.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(plain): RpcValue can represent a uint64 above int64max
The plain (tcp/tcp_ssl) wire squeezed every unsigned value through int64_t, so a
LIDL `uint` above int64max wrapped — independently in each direction:
outbound qvariant_rpc_value.cpp QMetaType::ULongLong -> int64_t(...)
inbound json_mapping.cpp is_number_unsigned -> get<int64_t>()
Neither wraps loudly: .get<int64_t>() past int64max returns -1 with no
exception. Two peers both running this code agreed on -1, so nothing looked
broken from inside — and no plain-tier test used an integer outside int32 range.
Measured over real tcp before the fix:
echoUint(2^63) -> -9223372036854775808
echoUint(2^64-1) -> -1
This was never a wire-format constraint. Both codecs carry uint64 natively (CBOR
emits major type 0, `1b ff..ff`) and the envelope's own `id` field already
crossed this wire as uint64_t. Only RpcValue *payloads* could not represent it.
RpcValue gains a uint64_t alternative, used through `makeInteger()` and ONLY for
values above int64max — the sole case where int64_t loses information. Anything
broader would change the representation of every non-negative integer already on
this wire, and since std::variant equality compares the alternative index it
would break comparisons against int64-built values, to fix nothing. Small
unsigned values keep crossing as signed, pinned by a test so the rule stays
visible.
Also fixes an off-by-one in the QJsonValue::Double -> int64 guard while here:
double(int64max) rounds UP to exactly 2^63, so `d <= double(int64max)` admitted
2^63 and then ran int64_t(d) out of range — undefined behaviour, saturating on
arm64 and INT64_MIN on x86-64. Now a strict `<` against 2^63.
tests: 14 new. Both codecs round-trip 2^64-1 flat and nested; negatives stay
signed; the Qt boundary is exact in both directions; the narrow representation
rule and the 2^63 guard are pinned. 226/226.
verified end-to-end, cross-process, with a negative control: the new 64-bit
boundary cases in logos-logoscore-py fail on the pinned protocol over tcp with
exactly the values above, and all 68 pass with this build — on local, tcp and
tcp_ssl alike.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Dario LipicarandClaude Opus 5
|
362b03fb1e |
feat(codec): one canonical LIDL ↔ JSON codec, generic over composition (#29)
* feat(codec): one canonical LIDL <-> JSON codec, generic over composition
The tagged-bytes encoding {"_bytes": "<base64url, unpadded>"} was implemented
SIX times — the Qt conversion here, the plain wire's json_mapping, the lp helper
in logos-cpp-sdk, a copy emitted into every generated cdylib module, the Rust
SDK and the Python client — and they disagreed on which inputs they accept:
- {"_bytes":"AA","x":1} decoded as BYTES on the lp path (no size()==1 check)
but as a MAP on the plain wire and in the glue.
- Padded "AH-A_w==" gave correct bytes in one copy, empty in another, None in
Rust.
- A plain string / number / number-array argument was accepted by C++
providers (Qt and CLI parity) and rejected by Rust ones.
logos_codec.h is the single implementation. Leaves: tstr, bstr, every signed and
unsigned integral spelling, every floating spelling, bool, any (recursion stops).
Composition is GENERIC — std::vector<T> and std::map/unordered_map<std::string,T>
for any supported T, at any depth — so [bstr], [[bstr]], {tstr: [bstr]} and bytes
nested in a map all encode canonically without anything enumerating combinations.
Codec<T> is a trait, so an unsupported T is an incomplete type: a compile error
naming the type, never a silent fallback. Decode throws CodecError carrying the
path ("[0][1]", ".k") instead of substituting a default — a mangled value must
not reach business logic. bstr keeps a documented lenient form for provider-side
arguments, because the Qt consumer path and the logoscore CLI both produce plain
strings and number arrays for byte parameters.
JsonArg exists for generated dispatch: it converts itself into whatever the
callee's parameter type is. Naming the type instead is a trap — spelling [uint]
as std::vector<uint64_t> (the LIDL mapping) does not bind to an author's
std::vector<uint32_t>, since distinct vector instantiations do not convert.
logos_codec.h joins the installed header set; nix/include.nix already globs
cpp/*.h.
Tests: 198/198. 15 new ones pin the contract rather than the happy path —
[[bstr]] tagged at depth, map-of-bytes, empty elements surviving as elements,
uint64 past 2^63, an integral JSON number decoding as float64, padded base64,
the multi-key {"_bytes":...} case being a map, and path-carrying failures.
Not yet converged onto this header (follow-ups): the Qt conversion in
logos_json_convert.cpp, and the plain wire's copy in json_mapping.cpp — the
latter needs a strict variant first, because it THROWS on malformed base64
(via its own logos::plain::CodecError) where every other copy is tolerant.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(codec): fold the Qt and plain-wire copies into the shared codec
The two remaining in-repo implementations now delegate:
- logos_json_convert.cpp (the Qt CONSUMER path — argument encoding and return
decoding) dropped Qt's toBase64/fromBase64 and its own tagged-bytes
predicate. Only the QByteArray <-> std::vector<uint8_t> hop stays local, so
the Qt path cannot drift from the wire or from providers: same alphabet, same
padding rule, same single-key shape.
- implementations/plain/json_mapping.cpp dropped its anonymous-namespace
b64url_encode/decode.
The wire needed something the tolerant decode does not give it: it REJECTS a
corrupt frame rather than silently decoding fewer bytes. Hence
b64UrlDecodeChecked — strict about the alphabet and the length, tolerant of '='
padding — which json_mapping uses to keep throwing its own
logos::plain::CodecError. Consumer-facing decodes stay tolerant. Both behaviours
now come from one implementation instead of four that disagreed.
Also removed the local isTaggedBytes wrapper, which shadowed the shared one and
made unqualified calls ambiguous.
Tests: 199/199, with the strict decode's accept/reject set pinned (padding
tolerated, stray character rejected, impossible length rejected).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Dario LipicarandClaude Opus 4.8
|
6401e30ae1 |
feat: group-shareable local sockets, stale-socket reaper, bind-failure detection (#20)
* feat: group-shareable local sockets, stale-socket reaper, bind-failure detection
The QtRO local transport binds each module's unix socket at 0777 & ~umask
(0755) with no way for a second OS user to reach it, discards the listen
result so a failed bind surfaces only as clients hanging, and never cleans up
the socket file — a hard-killed logos_host leaks it forever.
Add a Qt-free helper (logos_socket_paths.{h,cpp}) usable from both the qt_remote
and plain transport paths:
- applySocketPerms(path): chgrp + chmod a bound socket per LOGOS_SOCKET_GROUP /
LOGOS_SOCKET_MODE (chgrp-then-chmod so a half-applied policy is only ever
too strict). No-op when unset, so default behaviour is unchanged. Connecting
to an AF_UNIX socket needs write permission, so 0660 is what lets a group
member in.
- isSocketDead(path): S_ISSOCK && owned-by-us && non-blocking connect returns
ECONNREFUSED/ENOENT. Fails closed on any other outcome, so it never reports
a live socket or a regular file dead.
- reapStaleSockets(dir, prefix): unlink only the dead sockets, never a regular
file that shares the prefix (e.g. a *.lgx build artefact).
Wire it into RemoteTransportHost::publishObject and QtRemoteRegistry:
- construct QRemoteObjectRegistryHost empty and listen via setRegistryUrl() so
a bind failure is observed and logged (with lastError() + the socket path)
instead of leaving a silently-broken host;
- apply the socket-access policy to the freshly-bound local: socket.
The env-driven policy means every process in a node's tree (daemon, logos_host
subprocesses, their children) applies the same rule to every socket it binds
without threading config through each layer — the daemon exports the vars once.
Adds test_socket_paths.cpp (8 gtests): mode/group application, no-op default,
bad-mode rejection, live/dead/regular-file classification, and the reaper
keeping live sockets and regular files while removing only dead ones.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* review: harden socket helpers (gid overflow, socket-owner check, empty-prefix guard, dedup path)
Addressing automated review feedback on the socket helpers:
- resolveGid(): validate strtoul() errno/range so an out-of-range numeric
LOGOS_SOCKET_GROUP is rejected instead of silently truncating to a wrong gid.
- applySocketPerms(): when a policy is requested, stat the path first and refuse
unless it's a socket we own (S_ISSOCK + st_uid == geteuid()), so a malformed
URL can never chmod/chown a stray file. No-op fast path when the env is unset.
- reapStaleSockets(): refuse an empty prefix (would make every dead socket the
process owns a deletion candidate).
- Extract the duplicated `localSocketFilePath()` (Qt QLocalServer name->path
rule) into a shared qt_remote/qt_socket_path.h so RemoteTransportHost and
QtRemoteRegistry can't drift.
Adds tests: non-socket path refused (mode unchanged), empty-prefix reaper no-op.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: transport-aware token validator hook on ModuleProxy (#22)
* feat: transport-aware token validator hook on ModuleProxy
Adds an injectable authorizer so a host (the logoscore daemon) can accept tokens
the built-in issued-token scan doesn't know — specifically operator-issued named
tokens validated against a persistent store — with per-token expiry and
local_only enforced against the transport the call arrived on.
- ModuleProxy::setTokenValidator(std::function<bool(token, transportProtocol)>).
isAuthorized() consults it ONLY after the existing m_tokens + TokenManager
scan fails, so installing a validator is purely additive: it can grant, never
revoke, access the built-in path already allows. Empty (default) = today's
behaviour exactly.
- callRemoteMethod() gains a defaulted `transportProtocol` ("local"). The QtRO
local path (RemoteTransportHost) uses the default; PlainTransportHost::onCall
passes the real wire ("tcp" | "tcp_ssl", fail-closed to non-local on an
unexpected protocol) so a local_only token presented over the network is
rejected. One ModuleProxy is shared across a provider's transports, so the
transport can't be inferred — it must be threaded per call, which the defaulted
arg does without changing the QtRO replica's 3-arg call.
The daemon backs the validator with TokenStore::lookupByToken; other modules
keep the default (no validator) and are unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* review: split callRemoteMethod into explicit 3-arg + 4-arg overloads; include <utility>
Addressing review feedback:
- Replace the defaulted transportProtocol argument with two explicit Q_INVOKABLE
overloads. The Qt meta-object system matches methods by their full parameter
list and doesn't apply C++ default arguments, so the QtRO/local 3-arg call
must remain a real 3-arg method rather than relying on moc's reduced-arity
generation. The 3-arg form forwards to the transport-aware 4-arg form with
"local"; PlainTransportHost keeps calling the 4-arg form with the real wire.
- Include <utility> explicitly in module_proxy.h for std::move rather than
relying on an indirect include.
Full protocol suite green (160/160).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Dario Lipicar
|
664b43f18a |
perf(qt_remote): cache the remote-object handle per name in LogosAPIConsumer (#24)
Acquiring a QtRO replica per call (acquireDynamic + waitForSource) is expensive:
under a tight loop — e.g. a proxy forwarding every method to its target, or a UI
backend driving a whole surface — it dominates and can even starve the nested
synchronous calls. Cache the LogosObject handle per object name in m_objectCache
and reuse it across calls (both the sync invokeRemoteMethod and the async
invokeRemoteMethodAsync paths); no per-call release(). A stale handle (source
went away — module unloaded / transport dropped) is detected via a new
LogosObject::isValid() (QtRO replica state == Valid) and transparently
re-acquired. The cache is released in clearObjectCache() from the destructor and
before reconnect().
- logos_object.h: add virtual bool isValid() (default true).
- qt_remote/remote_transport.{h,cpp}: RemoteLogosObject::isValid() (replica
Valid state) + a process-wide acquireCount() test hook.
- logos_api_consumer.{h,cpp}: m_objectCache + acquireCachedObject()/
clearObjectCache(); sync + async reuse the cached handle; async keeps the
QPointer guard and never releases the shared handle from its callback.
Test: RemoteEventTest.ConsumerReusesCachedHandleAcrossSyncAndAsyncCalls publishes
a provider over the qt_remote host, does 12 sync + 12 async echo calls, and
asserts every result is correct AND acquireCount() == 1 (one replica for all 24
calls). 164/164 green.
|
||
|
Dario LipicarandClaude Opus 4.8
|
315a3a2e0a |
fix(qt_remote): defer async completion delivery off the QtRO read stack (#7)
A `concurrency:"multi"` call's result comes back as a deferred completion event (`__logos_call_complete__`), delivered by RemoteEventHelper::onEventResponse — a slot fired by the replica's eventResponse signal. Cross-process, that slot runs on QtRO's read stack (QRemoteObjectNodePrivate::onClientRead). Until now the async user callback was invoked *inline* there, and that callback routinely (a) emits a module event — which the host-side ModuleProxy serializes onto the QtRO source — and (b) release()s the client object. Doing either while onClientRead is still unwinding re-enters QtRO and corrupts the node: a SIGSEGV in onClientRead (EXC_BAD_ACCESS, KERN_INVALID_ADDRESS at 0x80). This is the crash the EVM wallet backend hit from refresh_balances, which fans balance reads out to eth_rpc via call_async and then emits `balances_updated` from the gather completion. Primary fix (remote_transport.cpp): deliver the async completion callback on the next event-loop turn via QTimer::singleShot(0, m_helper, …) instead of inline, so all user code (event emits, release(), further calls) runs after onClientRead has fully unwound. m_helper is the context so the callback is dropped if the object is torn down first. Defense-in-depth for the same re-entrancy class: - remote_transport.cpp release()/disconnectEvents()/dtor: deleteLater() the helper (signal receiver) and replica (signal sender) and disconnect first, instead of deleting them inline — deleting a QObject mid-emission corrupts the connection list Qt is iterating. - module_proxy.cpp: always queue the source eventResponse emit to the owning thread (Qt::QueuedConnection), never emit inline, so a module that emits from inside a same-thread dispatch can't re-enter QtRO's source serialization. Tests (tests/protocol/test_remote_transport_events.cpp, newly wired): qt_remote LocalSocket event delivery (direct + full provider chain) and a reentrant-release regression that drives release() from inside a deferred-completion callback. The hard crash only reproduces cross-process (in-process QtRO posts the event, so the read stack has already unwound) — the cross-process guard is the wallet Anvil integration doctest, where this fix is A/B-proven: the published backend crashes on refresh_balances, the patched backend returns balances cleanly. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 4.8
|
4ea32a314a |
Per-module concurrent dispatch: async provider seam + transports (#5)
* feat: per-module concurrent dispatch (concurrency:"multi") — zero ABI change
A "multi" module serves calls concurrently behind the ORDINARY callMethod — no
new provider/host vtable method, so LogosProviderObject's ABI is byte-identical
to before and an old host/daemon loads and forwards a multi module unmodified.
Mechanism: a multi module's generated glue returns a pending sentinel
({"__logos_pending_call__": callId}) from callMethod and pushes the real result
back later as a __logos_call_complete__ event keyed by callId, over the existing
event channel. The consumer transport detects the sentinel and awaits the
completion transparently, so generated clients are unchanged.
- logos_async_dispatch.h: shared wire constants + the contract.
- remote_transport.cpp (QtRO) / plain_logos_object.{h,cpp} (plain): consumer
sentinel detection + await keyed by callId. The host is a pure forwarder.
- logos_protocol.h + nix/default.nix: protocol 0.2.0 (additive minor; same MAJOR
stays compatible, so an old host accepts a 0.2 "multi" module).
- rpc_server.cpp: fix a teardown self-deadlock (stop() held m_mu while invoking a
per-connection error handler that re-locks m_mu) that the new in-process
subscription path exposed.
- tests/protocol/test_concurrent_dispatch.cpp: proves a multi provider overlaps
two concurrent calls (peak 2) while single serializes (peak 1), over the plain
transport, with the host unchanged from master.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: coalesce concurrent async requestModule handshakes (+ async fan-out test)
A driver that fans out N async calls to an un-tokened target before any
completes used to fire N separate requestModule handshakes. Each mints a
distinct capability token and informs the target, and the later inform
OVERWRITES the earlier token there (the target stores one token per caller),
so the already-dispatched calls carried a superseded token and the target
rejected them as unauthorized ("auth token not recognized"). The sync path
never hit this — it blocks per call, so handshakes never overlap.
Coalesce in LogosAPIClient::invokeRemoteMethodAsync: the first async call to
an un-tokened target starts ONE handshake; concurrent calls to the same
target queue behind it and all drain with the single minted token when it
resolves. m_pendingHandshakes is touched only on the owner thread, so no lock
(appended last per the class's ABI note). This is what lets a concurrency:
"multi" worker actually run a single-threaded driver's fan-out concurrently —
otherwise the fanned-out calls are rejected before reaching dispatch.
Also add MultiProviderOverlapsAsync / SingleProviderSerializesAsync to the
concurrent-dispatch gtest: they fire N concurrent callMethodAsync() calls (the
fan-out pattern over the async consumer path, which the sync tests don't
exercise) and assert peak overlap 4 for "multi", 1 for "single".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Dario Lipicar
|
29afbac532 |
Extract the Logos protocol layer from logos-cpp-sdk (lp_* C ABI + protocol semver) (#2)
* Extract the Logos protocol layer from logos-cpp-sdk
Transports (plain TCP/TLS, qt_local, qt_remote/QRO, mock), token manager,
consumer core (LogosAPIClient/LogosAPIConsumer incl. the capability
auto-requestModule flow), ModuleProxy, the abstract LogosProviderObject
interface, and the canonical QVariant<->JSON conversion — now behind the
language-neutral lp_* C ABI (logos_protocol.h) carrying the protocol
semver (LOGOS_PROTOCOL_VERSION_*, lp_protocol_version()).
Bytes crossing the ABI use the lossless {"_bytes": base64url} tagging
(NUL-safe), matching the plain wire encoding.
Provider lp_* surface is compiled groundwork; serving lands with module
authoring.
* consumer: typed requestModule for the capability flow
Port of logos-cpp-sdk master f5a127dd ('use updated capability module',
cpp-sdk#85, Iuri Matias) — the touched files (logos_api_client.cpp,
logos_api_consumer.{h,cpp}) moved into this repo in the P1 extraction.
The capability auto-requestModule path now calls a typed std::string
helper on the consumer (which acquires the capability object directly)
instead of a stringly invokeRemoteMethod round-trip. 111/111 tests.
|