core: in-flight counters leak permanently when a throw crosses the reserve/handoff window (RemoteServer and Bridge)

[Yaraslaut]

Summary

Two counters that gate documented behaviour are incremented before a non-noexcept region and decremented only by a handler that region may never reach. A single throw leaves each permanently over-counted for the process's life.

Verification status

Structurally verified by reading; reachability not reproduced. I did not construct a throwing serializeAction/encode. Revision: origin/master adfe8e5f plus this branch's doc-only commits.

---

1. RemoteServer::_inFlightExecutes (include/morph/core/remote.hpp)

The reservation at :1406-1422 carries this comment:

"Reserving here, at the last point before the slot is genuinely taken, needs no unwind on the early-return paths above."

True of returns. But several non-noexcept, allocating, lock-taking calls follow before any handler can decrement it, with no RAII guard:

• :1423 emitMetric(...)
• :1433-1434 two std::make_shared
• :1467 _timeoutScheduler->schedule(...)
• :1491 ticketGuard.awaitTurn() (takes a mutex)
• :1492 _strand.post(mid, …)

A throw from any of these unwinds to dispatchMessage's catch at :1264-1274, which replies but never decrements.

Consequences:

• drainedWithin() (:585-589, predicate == 0) can never return true again — graceful shutdown is permanently broken for that server.
• health().inFlight is permanently wrong.
• With maxInFlightExecutes set, a slot is lost for good.
• If the throw lands after :1467 succeeded, the scheduled timeout later fires complete(...) and sends a second err "timeout" for the same callId — the catch at :1274 bypasses the finished atomic_flag (:1435-1451), breaking handle()'s documented reply-exactly-once contract (:257-258).

The catch's own comment (:1265-1269) reinforces the wrong model by enumerating only pre-reservation throw sites (authorize/authenticate/authorizeInstance/missingRequiredFields), reading as if the throwing region ended before the increment.

2. Bridge::_pendingCalls (include/morph/core/bridge.hpp)

_pendingCalls->fetch_add (:1420) and the deadline arming happen before auto anyCompletion = backend->execute(...) (:1544), which is not wrapped in a try. QtWebSocketBackend::execute (src/qt/qt_websocket_backend.cpp:423-449) calls call.serializeAction() (:440, user toJson/glaze code) and wire::encode(env) (:448) with no try — both can throw.

A throw escapes executeVia/BridgeHandler::execute, permanently inflates pendingCalls() — breaking the documented "gate a feature on quiescence" use at :1194-1207 — and leaves the scheduled timeout entry armed.

The doc's "The synchronous 'handler not bound' early return — the one case executeVia() resolves before ever dispatching" (:1201) is also not exhaustive.

Fix shape

Both are the same one-liner: a scope_exit-style guard on the reservation, released on the path that hands ownership to the completion/complete(...).

What would change the verdict

• Close it if every call in both windows is shown non-throwing in practice — but serializeAction runs user-supplied toJson, and wire::encode runs glaze, so that argument has to cover both.
• Raise it with a test whose action toJson throws, asserting pendingCalls() == 0 and drainedWithin(...) afterwards. Neither has such a test today, which is why both windows are open.

[Yaraslaut]

Triage: triage: valid, bug, area: core

Premise re-checked by reading the reservation window in remote.hpp. The comment at :1404-1405 — "Reserving here, at the last point before the slot is genuinely taken, needs no unwind on the early-return paths above" — is accurate about returns and silent about throws. Between the increment and any handler that could decrement it, the code calls emitMetric, two std::make_shared, _timeoutScheduler->schedule(...), ticketGuard.awaitTurn() (takes a mutex) and _strand.post(...), with no RAII guard. Confirmed by direct read.

The consequence that makes this more than bookkeeping is the one the issue names last: drainedWithin() (:585-589) predicates on == 0, so a single leaked slot means graceful shutdown can never again succeed for that server. That is a permanent, process-lifetime effect from one transient throw.

Invariant 7 is the governing one here, and it is why this is worth fixing rather than noting. The existing tests cannot distinguish a correct counter from a leaking one, because none of them makes the guarded region throw. A test whose action toJson throws — asserting pendingCalls() == 0 and drainedWithin(...) afterwards — would fail today and pass after; anything weaker would be green either way.

Both halves belong together. Bridge::_pendingCalls (bridge.hpp:1420 vs the untried backend->execute(...)) is the same defect with the same one-line fix shape (a scope_exit released on the hand-off path), and QtWebSocketBackend::execute genuinely can throw there — it runs user toJson and wire::encode. Keeping them in one issue.

Reachability is inferred, not reproduced, and the issue says so. That is the honest status: the window is verified structurally; nothing has been shown to throw into it in practice.