Close #506 with a reproduction, and establish that the mutation survivor list is not trustworthy (#453, #504)

docs/spec/core/callback_scope.md

@@ -148,8 +148,15 @@ QTimer::singleShot(0, _callbacks.guard([this] { tick(); }));
 ```
 
 The returned callable forwards every argument and returns `void`. A
-value-returning callable is rejected at compile time: there is no defensible
-value to return when delivery is suppressed.
+value-returning callable is rejected: there is no defensible value to return
+when delivery is suppressed.
+
+The rejection is a `static_assert` **inside the returned wrapper's body**, so it
+fires when the wrapper is *invoked*, not when `guard()` is called. A wrapper
+built from a value-returning callable and then never called compiles cleanly —
+worth knowing when the guarded callback is stored and its subscription torn down
+before its first tick. Constraining `guard()` itself would make the rejection
+unconditional; that has not been done.
 
 ### Gated overloads elsewhere
 

docs/spec/util/quantity_type.md

@@ -252,7 +252,15 @@ unit metadata travels only in the schema, never in the instance.
 Every printed form of a `Quantity` — `std::format`, and every number inside
 `equation()` — goes through **one** helper, `detail::formatRationalDecimal`, so
 a value reads identically everywhere. There is a single formatting path and
-**no `operator<<`** (streaming is done by formatting to a `std::string` first).
+**no `operator<<`** (streaming is done by formatting to a `std::string` first);
+in-code references to one are references to `std::formatter<Quantity>`.
+
+`formatRationalDecimal` takes the numerator's magnitude through
+`math::detail::absU64`, in unsigned arithmetic. It negated in `int64_t` until
+morph#496, which is undefined for `INT64_MIN` — reachable because the
+whole-integer `Rational{value, DecimalPlaces{n}}` constructor does not
+canonicalise, so the clamp that would otherwise remove the trap value never
+ran.
 
 **The decimal form.** `formatRationalDecimal` renders the exact `Rational` as a
 fixed decimal at its **runtime `DecimalPlaces`** and then trims trailing zeros

include/morph/core/backend.hpp

@@ -611,9 +611,11 @@ class LocalBackend : public detail::IBackend {
 
     /// @brief Creates a model instance via @p factory and registers it.
     ///
-    /// @p typeId is accepted for interface compatibility but not used — the
-    /// concrete type is captured by the factory closure. If the new holder's
-    /// `isBackendChangeAware()` returns `true`, @p mid is also recorded in
+    /// The `typeId` parameter is accepted for interface compatibility but not
+    /// used — it is unnamed in the signature below, and the concrete type is
+    /// captured by the factory closure. If the new holder's
+    /// `isBackendChangeAware()` returns `true`, the new id (a local in
+    /// `createAndTrack`, not a parameter here) is also recorded in
     /// `_changeAware` so `notifyBackendChanged()` finds it without a
     /// `dynamic_cast` sweep.
     /// @param factory  Callable that constructs the `IModelHolder`.

include/morph/core/callback_scope.hpp

@@ -98,8 +98,10 @@ class CallbackToken {
     ///
     /// @tparam F Callable type to wrap. Must return `void` for every argument
     ///           list it is invoked with; a value-returning callable has no
-    ///           defensible answer for the suppressed case and is rejected at
-    ///           compile time.
+    ///           defensible answer for the suppressed case. Rejected by a
+    ///           `static_assert` in the returned wrapper's body, so it fires
+    ///           when the wrapper is *invoked* -- a wrapper that is created and
+    ///           never called compiles either way.
     /// @param fn Callable to gate. Moved into the returned wrapper.
     /// @return A callable with @p fn's argument list and a `void` return.
     template <typename F>

include/morph/core/remote.hpp

@@ -1688,10 +1688,11 @@ class RemoteServer : public std::enable_shared_from_this<RemoteServer> {
     // (`test_remote_connection_scope.cpp`'s "an in-flight execute completes
     // safely across a disconnect" test — a lookup against a since-reclaimed
     // modelId must resolve without waiting on some other blocked model's
-    // strand — never touches this gate at all, since it never gets a ticket
-    // for a model that turns out to be gone... except it does get a ticket,
-    // and must release it immediately rather than hold up a live ticket
-    // behind it; see `ExecuteOrderGate::release`'s own doc comment).
+    // strand). That path *does* take a ticket: `handleImpl` takes one for any
+    // well-formed `execute` with a non-zero `modelId`, long before the registry
+    // lookup that discovers the model is gone. What keeps it fast is releasing
+    // that ticket immediately rather than holding up a live one behind it; see
+    // `ExecuteOrderGate::release`'s own doc comment.
     //
     // Keyed by ModelId internally, not held forever: a model with no
     // outstanding tickets has no entry in the gate's map at all (erased once

include/morph/net/detail/tcp_socket.hpp

@@ -374,6 +374,33 @@ class TcpSocket {
         }
     }
 
+    /// @brief Bounds how long a single `::send` inside `sendAll()` may block.
+    ///
+    /// Without this a `sendAll` against a peer that has stopped reading blocks
+    /// forever once the kernel send buffer fills, and it does so while holding
+    /// whatever lock its caller took -- which is how `~SocketBackend` came to be
+    /// parkable behind `_socketMtx` (morph#506). With `SO_SNDTIMEO` set, the
+    /// blocked `send` returns `EAGAIN`/`EWOULDBLOCK` instead, `sendAll` throws
+    /// as it already does for any other send error, and the lock is released.
+    ///
+    /// A timeout is not a "slow link" cutoff: it bounds one `send` syscall that
+    /// is making *no* progress, so it should be set generously. Zero disables it
+    /// (the kernel default, block forever).
+    ///
+    /// @param timeout Per-`send` bound; zero to disable.
+    /// @return `true` if the option was applied.
+    [[nodiscard]] bool setSendTimeout(std::chrono::milliseconds timeout) const noexcept {
+        // Assigned without casts on purpose: `milliseconds::rep` and
+        // `timeval`'s members are both `long` on the platforms this builds for,
+        // so an explicit cast is an identity cast and GCC rejects it under
+        // -Werror=useless-cast. clang-tidy also wants names of three characters
+        // or more, hence `timeoutVal` rather than the conventional `tv`.
+        timeval timeoutVal{};
+        timeoutVal.tv_sec = timeout.count() / 1000;
+        timeoutVal.tv_usec = (timeout.count() % 1000) * 1000;
+        return ::setsockopt(_fd, SOL_SOCKET, SO_SNDTIMEO, &timeoutVal, sizeof(timeoutVal)) == 0;
+    }
+
     /// @brief Shuts down both directions of the socket, unblocking a concurrent
     ///        `recvSome`/`sendAll` on another thread. Safe to call from any thread.
     ///

include/morph/net/socket_backend.hpp

@@ -36,6 +36,14 @@ struct SocketBackendConfig {
     double backoffMultiplier = 2.0;
     /// @brief Maximum time to wait for the initial TCP connect to complete.
     std::chrono::milliseconds connectTimeout{5000};
+    /// @brief Bound on a single `::send` that is making no progress.
+    ///
+    /// Applied as `SO_SNDTIMEO`. Without it, a peer that stops reading fills the
+    /// kernel send buffer and parks `sendFrame` inside `sendAll` **while holding
+    /// `_socketMtx`** -- which parks `~SocketBackend` behind the same lock, with
+    /// nothing able to release it (morph#506). Generous on purpose: it bounds a
+    /// send making *no* progress, not a slow one. Zero disables it.
+    std::chrono::milliseconds sendTimeout{30000};
 };
 
 /// @brief `IBackend` implementation that communicates with a `RemoteServer`
@@ -94,11 +102,13 @@ class SocketBackend : public ::morph::backend::detail::IBackend {
         // unlocked `_socket.valid()` here races the I/O thread replacing the
         // object out from under it.
         //
-        // The hazard #506 describes is real and remains open: `sendFrame` holds
-        // this mutex across a blocking, un-timed `sendAll`, so a peer that stops
-        // reading can park the destructor here. Closing that needs a way to
-        // reach the fd without the mutex (an atomic fd shadowing `_socket`, with
-        // its own fd-reuse story), not simply removing the lock.
+        // The hazard #506 describes is closed from the other end: `sendAll` is
+        // no longer un-timed. `Config::sendTimeout` (SO_SNDTIMEO, 30s default)
+        // bounds any single send that makes no progress, so a peer that stops
+        // reading can hold `_socketMtx` for at most that long instead of
+        // forever, and this wait is bounded rather than open-ended. Fixing it
+        // that way rather than by reaching the fd without the mutex avoids the
+        // fd-reuse hazard an atomic shadow descriptor would carry.
         {
             std::scoped_lock const lock{_socketMtx};
             if (_socket.valid()) {
@@ -639,6 +649,13 @@ class SocketBackend : public ::morph::backend::detail::IBackend {
             bool connectedOk = false;
             try {
                 auto socket = ::morph::net::detail::TcpSocket::connect(_url.host, _url.port, _cfg.connectTimeout);
+                if (_cfg.sendTimeout.count() > 0) {
+                    // Before the handshake, so even that cannot park forever.
+                    // Bounds any single send that makes no progress, which is
+                    // what keeps ~SocketBackend from being parked behind
+                    // _socketMtx by a peer that stopped reading (morph#506).
+                    (void)socket.setSendTimeout(_cfg.sendTimeout);
+                }
                 std::string leftover = ::morph::net::detail::performClientHandshake(socket, _url);
                 {
                     std::scoped_lock lock{_socketMtx};

include/morph/util/quantity.hpp

@@ -723,11 +723,11 @@ struct Quantity {
         Quantity out;
         out.payload = adjusted;
         // Not `out._ctx = _ctx`: that would copy this quantity's derivation
-        // node verbatim, so equation() and operator<<(std::format) would
+        // node verbatim, so equation() and std::formatter<Quantity> would
         // disagree -- the node's own recorded `result` is still *this*
         // quantity's old payload/precision, but `out.payload` is the
         // retagged one. A fresh node (same convention as operator
-        // Quantity<To>()'s unit conversion above) keeps the two consistent.
+        // Quantity<To>()'s unit conversion, below) keeps the two consistent.
         MORPH_Q_BUILD(out, "retag decimal places", payload, std::nullopt, out.payload, MORPH_Q_NODE(*this), nullptr);
         return out;
     }

include/morph/util/rational.hpp

@@ -95,7 +95,6 @@
 ///       (e.g. sums over large coprime denominators). Keep operands within
 ///       the decimal-scaled ranges the precision tags imply.
 
-#include <atomic>
 #include <cassert>
 #include <cmath>
 #include <compare>
@@ -108,8 +107,8 @@
 #include <morph/core/logger.hpp>
 #include <morph/core/payload_shape_tag.hpp>
 #include <numeric>
-#include <stdexcept>
 #include <string>
+#include <string_view>
 #include <type_traits>
 #include <utility>