Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/spec/core/backend.md
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ reason as `QtWebSocketBackendConfig`) bounds per-connection resource usage:
|---|---|---|
| `maxConnections` | `0` (unbounded) | A connection accepted beyond this count is closed immediately in `onNewConnection`, before any signal is wired or the socket is tracked. Logged at `morph::log::LogLevel::warn`, naming the live count and the cap — see [Server-side observability](#server-side-observability). |
| `maxMessageBytes` | `wire::kMaxEnvelopeBytes` | Checked against the UTF-8 byte length of every incoming frame before it reaches `RemoteServer::handle()`; an oversized frame gets an immediate `err` reply and is never dispatched. The reply carries the rejected call's `callId`, recovered by `wire::detail::peekCallId`'s bounded prefix scan since the frame is deliberately never decoded. A zeroed `callId` would not merely fail to resolve the execute — `0` is the client's synchronous-reply discriminator, so it would resume an unrelated parked `register`/`deregister` with another call's reply. |
| `messagesPerSecond` | `0` (unbounded) | A per-connection token bucket (capacity = `messagesPerSecond`, refilled continuously). A frame that finds an empty bucket is dropped silently — not replied to, not queued. |
| `messagesPerSecond` | `0` (unbounded) | A per-connection token bucket (capacity = `messagesPerSecond`, refilled continuously). A frame that finds an empty bucket is refused — it never reaches `RemoteServer` — and answered with an `err "rate limited"` addressed to that frame's own `callId`. Not queued, and the connection is not closed. |
| `handshakeTimeout` | `0` (disabled) | A one-shot timer per connection; if no frame arrives before it fires, the socket is closed. Cancelled on the first frame. Because `QWebSocketServer::newConnection()` only fires after the WS (and TLS, in `SecureMode`) opening handshake completes, this in practice bounds time-to-first-frame after that point, not the handshake itself. |
| `idleTimeout` | `0` (disabled) | A shared ~1-second housekeeping sweep closes any connection whose last frame is older than `idleTimeout`; the actual close can lag the configured value by up to the sweep interval. |
| `bindAddress` | `QHostAddress::LocalHost` | The address `listen()` binds to (see "Flow" above). |
Expand Down Expand Up @@ -1415,7 +1415,7 @@ not a behavior change to the existing loopback-only default.
| No reconnect for never-connected sockets | `disconnected` schedules a retry only if `_everConnected` | A socket that never reached the server (bad URL / refused) fails fast via `waitForConnected` returning false, rather than backing off forever. |
| Server reply marshalled to the Qt thread | `QMetaObject::invokeMethod(..., QueuedConnection)` with a `QPointer` | `RemoteServer::handle` produces the reply on a pool thread, but `QWebSocket::sendTextMessage` must run on the Qt thread; the weak `QPointer` drops the reply cleanly if the client disconnected meanwhile. |
| `executeTimeout` implementation | A dedicated, lazily-started background thread (`morph::async::detail::TimeoutScheduler`) per `RemoteServer`, not a per-call thread | `IExecutor` has no delayed-post primitive and `RemoteServer` is transport-agnostic (cannot assume Qt's `QTimer`). One thread amortizes across every timed call; it is only started the first time `executeTimeout` is actually configured, so a server that never uses the feature pays no cost. |
| `messagesPerSecond` algorithm | Per-connection token bucket, capacity = rate, continuous refill, drop (not close) on empty | Simplest correct rate limiter; allows a legitimate one-second burst without penalizing an otherwise well-behaved client. Dropping (vs. closing) keeps a transient burst from taking down the connection — pair with `LimitPolicy::executeTimeout` if bounded caller-side waiting is also needed. |
| `messagesPerSecond` algorithm | Per-connection token bucket, capacity = rate, continuous refill; on empty the frame is refused with an `err` reply, and the connection is left open | Simplest correct rate limiter; allows a legitimate one-second burst without penalizing an otherwise well-behaved client. Refusing rather than closing keeps a transient burst from taking down the connection. The frame is *answered* rather than discarded because a reply costs nothing at the protocol level and is the difference between a caller's `Completion` failing and it hanging: the id is recovered by the same bounded prefix scan (`peekCallId`) the `maxMessageBytes` branch uses, so no decode of a frame that will not run is needed. |
| Graceful shutdown drains via a shared in-flight counter, not a new `IExecutor::waitIdle` | `RemoteServer` counts its own accepted-but-unreplied executes rather than adding a general drain API to `IExecutor`/`StrandExecutor` | The drain condition morph can define precisely — "every accepted execute has replied" — lives at the server layer, where the work is counted; executor.md's "no graceful drain / `waitIdle`" limitation is deliberately left as-is for raw executor users. |
| Backend-change-awareness captured at registration | `IModelHolder::isBackendChangeAware()` (compile-time answer per model type) + `LocalBackend::_changeAware`, maintained by `registerModel`/`deregisterModel` | Replaces a per-`notifyBackendChanged`-call `dynamic_cast` sweep over every live model with a virtual query done once at registration, and a lookup restricted to the models that actually opted in. No RTTI dependency; cost is O(change-aware models) instead of O(all models) under `_regMtx`. No change to the model-facing contract (`IBackendChangedSink`, `BackendChangedMixin`) or to when/where `onBackendChanged()` runs. |
| `morph::net`'s I/O model | A dedicated I/O thread + `std::condition_variable`, instead of the Qt event loop | Lets `SocketBackend`/`SocketServer` run with no GUI event loop and no Qt dependency, and — as a side effect — lets `SocketBackend` be driven safely from multiple threads (`QtWebSocketBackend` cannot be, since it is pinned to one event-loop thread). |
Expand Down
14 changes: 9 additions & 5 deletions docs/spec/core/completion.md
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,15 @@ throw — they are silent by construction.
Nothing in `Completion<T>` itself imposes a time limit: a state that no producer
ever settles simply stays pending forever, and its handle's callbacks never
fire. For an in-process `LocalBackend` that is unreachable, but across a wire a
request can genuinely disappear — a frame silently discarded by
`QtWebSocketServerConfig::messagesPerSecond`'s rate limiter, a connection that
dropped between send and reply, or a server that hangs. In every one of those
cases *no reply of any kind* comes back, so no layer below the caller has
anything to resolve the `Completion` with.
request can genuinely disappear — a connection that dropped between send and
reply, or a server that hangs. In those cases *no reply of any kind* comes back,
so no layer below the caller has anything to resolve the `Completion` with.

(A frame refused by `QtWebSocketServerConfig::messagesPerSecond`'s rate limiter
used to belong on that list. It no longer does: the transport answers it with an
`err "rate limited"` addressed to the frame's own `callId`, so the caller's
`Completion` fails rather than hanging. A deadline is still worth arming for the
two cases above, which no reply can cover.)

`Bridge::setExecuteDeadline(std::chrono::milliseconds)` closes that hole.

Expand Down
10 changes: 6 additions & 4 deletions examples/TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -687,10 +687,12 @@ managed by path-filtering (`MORPH_LADDER_RUNGS` computed from changed paths:

## Framework gaps this strategy exposes (candidate issues)

1. Client-side execute deadline — no timeout on `Completion`; a
rate-limited/black-holed call hangs forever (`messagesPerSecond` drops
frames silently). Every polling helper must wrap its own timer until the
framework provides one.
1. Client-side execute deadline — no timeout on `Completion`; a black-holed
call hangs forever. Every polling helper must wrap its own timer until the
framework provides one. (A frame refused by `messagesPerSecond` no longer
belongs on that list: the transport answers it with an `err "rate limited"`
addressed to the frame's own `callId`, so the caller's `Completion` fails
rather than hanging — morph#225.)
2. `Bridge::pendingCalls()` (client-side quiescence observability) — makes
`settle()` exact; today presenter-level counters substitute.
3. `MainThreadExecutor::runOnce()/drain()` — a step, not a wall-clock pump.
Expand Down
6 changes: 4 additions & 2 deletions examples/polls/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,10 @@ the current framework source (not assumed from the ladder doc alone):
caller falls back to the synchronous path unaffected) so every backend
that has not opted in keeps its current behavior.
- **Client-side execute deadline.** No timeout exists anywhere on a
`Completion` today — a frame silently dropped by `messagesPerSecond`, or
a genuinely hung server, blocks the calling `Completion` forever.
`Completion` today — a genuinely hung server blocks the calling `Completion`
forever. (A frame refused by `messagesPerSecond` used to belong here too; it
no longer does, since the transport now answers it with an `err "rate
limited"` — morph#225.)
`Completion<T>::state()` already exposes the underlying
`CompletionState`, and `CompletionState::setException` is
idempotent-guarded (`if (ready) return;`), so the fix needs no
Expand Down
60 changes: 41 additions & 19 deletions examples/polls/tests/test_shared_instance_lifecycle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,15 @@
// must not let its holder finalize poll B. Written explicitly (rather
// than assumed from the per-instance keying alone) because a bug in
// requireAdmin()'s poll-row lookup could silently pass.
// 2. Bridge::setExecuteDeadline recovers a call the real rate limiter
// (QtWebSocketServerConfig::messagesPerSecond) silently drops -- the
// DoD's "run this rung's harness with messagesPerSecond configured ON"
// requirement, proven end to end (not merely at the framework-prereqs
// plan's own unit-test level) for the first time in this rung.
// 2. The real rate limiter (QtWebSocketServerConfig::messagesPerSecond)
// refuses an over-budget call without hanging it -- the DoD's "run this
// rung's harness with messagesPerSecond configured ON" requirement, proven
// end to end (not merely at the framework-prereqs plan's own unit-test
// level) for the first time in this rung. This case used to prove that
// setExecuteDeadline *recovered* such a call, because the transport dropped
// the frame silently and only the deadline could settle it; since morph#225
// the transport answers it, so the call settles on its own and the deadline
// is no longer what saves it.
// 3. The cross-model rename-race analogue (rung 2's TagModel-renames-while-
// BookmarkModel-writes race): this rung's README does not name an exact
// analogue -- there is only one model type here (PollModel), so that
Expand All @@ -68,6 +72,7 @@

#include <memory>
#include <string>
#include <string_view>
#include <vector>

using morph::bridge::AllowShared;
Expand Down Expand Up @@ -279,7 +284,7 @@ TEST_CASE("A poll's admin token does not finalize a different poll", "[polls][mo
REQUIRE(pumpUntil([&failed] { return failed; }));
}

TEST_CASE("Bridge::setExecuteDeadline recovers a call the real rate limiter silently drops",
TEST_CASE("The real rate limiter refuses an over-budget call without hanging it",
"[polls][model][shared-instances]") {
// BackendRig's Mode::Socket constructor takes an optional
// QtWebSocketServerConfig (Task 11's own README-named
Expand All @@ -288,8 +293,10 @@ TEST_CASE("Bridge::setExecuteDeadline recovers a call the real rate limiter sile
// second server) -- messagesPerSecond set here is the real per-connection
// token bucket documented in qt_websocket_server.hpp: capacity equals
// messagesPerSecond, one token per incoming frame of any kind, refilling
// continuously; a frame that finds an empty bucket is dropped silently,
// no reply of any kind (mirrors tests/qt/test_qt_websocket.cpp's own
// continuously; a frame that finds an empty bucket is refused -- it never
// reaches RemoteServer, and the sender is answered with an
// `err "rate limited"` addressed to that frame's own callId (morph#225)
// (mirrors tests/qt/test_qt_websocket.cpp's own
// "messagesPerSecond throttles a burst on one connection" construction
// pattern -- ThreadPoolExecutor -> RemoteServer -> QtWebSocketServer with
// a low-messagesPerSecond cfg -- except BackendRig already threads that
Expand Down Expand Up @@ -321,26 +328,34 @@ TEST_CASE("Bridge::setExecuteDeadline recovers a call the real rate limiter sile
// and this loop issues all 20 sends in a single native call stack with no
// real wall-clock time between them, so refill-during-the-burst is
// negligible: at least 15 of these 20 frames are guaranteed to find an
// empty bucket and be dropped at the transport, never reaching
// RemoteServer, with no reply of any kind. Distinct participant names so
// any call that *does* get through always succeeds -- never a business
// -logic Conflict -- keeping "no real reply" the only way a call can end
// up in `errors` without also being a ClientTimeoutError.
// empty bucket and be refused at the transport, never reaching
// RemoteServer, each answered with an `err "rate limited"`. Distinct
// participant names so any call that *does* get through always succeeds --
// never a business-logic Conflict -- keeping the rate-limit refusal the
// only way a call can end up in `errors`.
constexpr int kBurstSize = 20;
int successes = 0;
int errors = 0;
int clientTimeouts = 0;
int rateLimited = 0;
for (int i = 0; i < kBurstSize; ++i) {
handler
.execute(SubmitVotes{.participantName = "voter-" + std::to_string(i),
.votes = {{.optionId = opened.options[0].id, .choice = VoteChoice::Yes}}})
.then([&successes](polls::GetPollStateResult) { ++successes; })
.onError([&errors, &clientTimeouts](const std::exception_ptr& err) {
.onError([&errors, &clientTimeouts, &rateLimited](const std::exception_ptr& err) {
++errors;
try {
std::rethrow_exception(err);
} catch (const morph::backend::ClientTimeoutError&) {
++clientTimeouts;
} catch (const std::exception& ex) {
// A refused frame comes back as an `err` envelope, which
// the client surfaces as a std::runtime_error carrying the
// server's message (wire.hpp).
if (std::string_view{ex.what()}.find("rate limited") != std::string_view::npos) {
++rateLimited;
}
} catch (...) {
}
});
Expand All @@ -351,11 +366,18 @@ TEST_CASE("Bridge::setExecuteDeadline recovers a call the real rate limiter sile
REQUIRE(pumpUntil([&] { return successes + errors >= kBurstSize; }, std::chrono::milliseconds{3000}));
CHECK(successes + errors == kBurstSize);

// Proof the drop was real, not merely that the deadline fired for some
// unrelated reason: strictly fewer real replies than calls sent (the
// Proof the refusal was real, not merely that something errored for an
// unrelated reason: strictly fewer successes than calls sent (the
// "observing more calls than replies" confirmation the brief calls for),
// and at least one of the shortfall was specifically recovered via
// ClientTimeoutError rather than some other error.
// and at least one of the shortfall carried the transport's own
// rate-limit refusal rather than some other error.
CHECK(successes < kBurstSize);
CHECK(clientTimeouts >= 1);
CHECK(rateLimited >= 1);

// The deadline is armed above and is deliberately *not* what settles these
// calls any more: before morph#225 a refused frame was dropped silently and
// only setExecuteDeadline could end the wait, so this case asserted
// clientTimeouts >= 1. Now the transport answers, so a timeout here would
// mean a call really did go unanswered -- the regression this guards.
CHECK(clientTimeouts == 0);
}
8 changes: 6 additions & 2 deletions include/morph/qt/qt_websocket_server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,12 @@ struct QtWebSocketServerConfig {
///
/// The bucket capacity equals `messagesPerSecond` (an immediate one-second
/// burst is allowed right after connecting), refilling continuously at that
/// rate. A frame that arrives with an empty bucket is dropped — not queued,
/// not replied to. See `docs/spec/core/backend.md`.
/// rate. A frame that arrives with an empty bucket is **refused, not
/// queued**: it never reaches `RemoteServer`, and the sender is answered
/// with an `err "rate limited"` addressed to that frame's own `callId`, so
/// a caller's `Completion` fails rather than waiting forever. The
/// connection stays open — throttling slows a client down, it does not
/// evict one. See `docs/spec/core/backend.md`.
std::size_t messagesPerSecond = 0;

/// @brief Time allowed for a newly-accepted connection to send its first text
Expand Down
28 changes: 20 additions & 8 deletions src/qt/qt_websocket_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,27 +223,39 @@ void QtWebSocketServer::onTextMessage(const QString& message) {
state.handshakeTimer = nullptr;
}

if (const auto utf8 = message.toUtf8(); std::cmp_greater(utf8.size(), _cfg.maxMessageBytes)) {
// Hoisted: both rejection branches below address their reply to the call it
// answers, and neither decodes the frame to do it.
const auto utf8 = message.toUtf8();
const auto peekedCallId = [&utf8] {
// Address the rejection to the call it answers. The frame is never
// decoded (that is the point of the cap), so the id is recovered by a
// bounded prefix scan. Replying with a zeroed callId would be worse
// than useless: `callId == 0` is the client's synchronous-reply
// discriminator, so the error would resume some unrelated parked
// register/deregister with a reply meant for an execute, while the
// execute that triggered it still never resolves.
const auto callId = ::morph::wire::detail::peekCallId(
return ::morph::wire::detail::peekCallId(
std::string_view{utf8.constData(), static_cast<std::size_t>(utf8.size())});
};

if (std::cmp_greater(utf8.size(), _cfg.maxMessageBytes)) {
socket->sendTextMessage(QString::fromStdString(
::morph::wire::encode(::morph::wire::makeErr("message exceeds maxMessageBytes", callId))));
::morph::wire::encode(::morph::wire::makeErr("message exceeds maxMessageBytes", peekedCallId()))));
return;
}

if (!consumeToken(state)) {
// Over the per-connection rate limit: drop the frame silently rather
// than reply or close the connection (see docs/spec/core/backend.md,
// QtWebSocketServerConfig::messagesPerSecond). A pending client
// Completion for a dropped `execute` will not resolve on its own; pair
// messagesPerSecond with LimitPolicy::executeTimeout for a bounded wait.
// Over the per-connection rate limit. The frame is not executed, but it
// *is* answered: dropping it silently left a pending client Completion
// for an `execute` with nothing to resolve it, so the caller hung
// unless it had armed `LimitPolicy::executeTimeout` (off by default).
// A reply costs nothing at the protocol level and turns that hang into
// an ordinary error the caller's `.onError(...)` already handles --
// the same reply-without-full-decode pattern the maxMessageBytes
// branch above uses. The connection stays open: rate limiting throttles
// a client, it does not evict one (morph#225).
socket->sendTextMessage(QString::fromStdString(
::morph::wire::encode(::morph::wire::makeErr("rate limited", peekedCallId()))));
return;
}

Expand Down
Loading
Loading