diff --git a/docs/spec/core/callback_scope.md b/docs/spec/core/callback_scope.md index f001f05e..7e458704 100644 --- a/docs/spec/core/callback_scope.md +++ b/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 diff --git a/docs/spec/util/quantity_type.md b/docs/spec/util/quantity_type.md index 04dc4204..38ede613 100644 --- a/docs/spec/util/quantity_type.md +++ b/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`. + +`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 diff --git a/include/morph/core/backend.hpp b/include/morph/core/backend.hpp index d13b16ab..ab17469b 100644 --- a/include/morph/core/backend.hpp +++ b/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`. diff --git a/include/morph/core/callback_scope.hpp b/include/morph/core/callback_scope.hpp index d74612b5..62a9380c 100644 --- a/include/morph/core/callback_scope.hpp +++ b/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 diff --git a/include/morph/core/remote.hpp b/include/morph/core/remote.hpp index fb8e1408..c32a4f6e 100644 --- a/include/morph/core/remote.hpp +++ b/include/morph/core/remote.hpp @@ -1688,10 +1688,11 @@ class RemoteServer : public std::enable_shared_from_this { // (`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 diff --git a/include/morph/net/detail/tcp_socket.hpp b/include/morph/net/detail/tcp_socket.hpp index f5691a5e..0eac59eb 100644 --- a/include/morph/net/detail/tcp_socket.hpp +++ b/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. /// diff --git a/include/morph/net/socket_backend.hpp b/include/morph/net/socket_backend.hpp index dc133ab2..c8cd5884 100644 --- a/include/morph/net/socket_backend.hpp +++ b/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}; diff --git a/include/morph/util/quantity.hpp b/include/morph/util/quantity.hpp index be671ea2..ed4feffa 100644 --- a/include/morph/util/quantity.hpp +++ b/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 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()'s unit conversion above) keeps the two consistent. + // Quantity()'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; } diff --git a/include/morph/util/rational.hpp b/include/morph/util/rational.hpp index f0a3acc8..b0d1705c 100644 --- a/include/morph/util/rational.hpp +++ b/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 #include #include #include @@ -108,8 +107,8 @@ #include #include #include -#include #include +#include #include #include diff --git a/scripts/branch_partial_allowlist.json b/scripts/branch_partial_allowlist.json index b814da1d..41617565 100644 --- a/scripts/branch_partial_allowlist.json +++ b/scripts/branch_partial_allowlist.json @@ -20,21 +20,21 @@ "entries": [ { "file": "include/morph/util/rational.hpp", - "line": 272, + "line": 271, "source": "assert(rawDecimalPlaces <= kMaxDecimalPlaces);", "reason": "assert()'s false arm calls abort(). Reaching it means violating the precondition and terminating the process, so no test in a Catch2 binary can observe it and still report; a death test would prove only that assert() works. The decision the assert guards is checked by the caller-side tests that stay inside kMaxDecimalPlaces." }, { "file": "include/morph/util/rational.hpp", - "line": 698, + "line": 697, "source": "if (!std::is_constant_evaluated()) {", "reason": "The false arm is the constant-evaluated one, and constant evaluation does not execute instrumented code -- no counter can increment during it. So in every run this branch is true, structurally, and its other arm is not a gap in the tests but a gap in what runtime instrumentation can see. reportOverflow's constexpr behaviour is exercised by the constexpr Rational arithmetic in tests/test_rational.cpp; what cannot be exercised is llvm-cov's counter." }, { "file": "include/morph/util/rational.hpp", - "line": 732, + "line": 731, "source": "if (!std::is_constant_evaluated()) {", - "reason": "reportClamp's copy of the same shape as line 698 above, and uncoverable for the same reason: the untaken arm is constant evaluation, which increments no counters." + "reason": "reportClamp's copy of the same shape as line 697 above, and uncoverable for the same reason: the untaken arm is constant evaluation, which increments no counters." }, { "file": "include/morph/util/datetime.hpp", @@ -44,31 +44,31 @@ }, { "file": "include/morph/util/rational.hpp", - "line": 758, + "line": 757, "source": "return std::is_gt(ordering) ? 1 : 0;", - "reason": "The false arm (a tie, returning 0) is unreachable. compareForSaturation has exactly two call sites, both from the saturating branch of operator+=/operator-=, and both are guarded by addWouldOverflow/subWouldOverflow having just returned true. A tie in compareForSaturation (*this == -rhs for +=, or *this == rhs for -=) requires -- since operator== compares canonical numerator/denominator pairs directly -- identical denominators. With equal denominators, addWouldOverflow/subWouldOverflow's own scaling factors collapse to rightScaled == leftScaled == 1 (gcd(D, D) == D), so the cross-multiplication checks reduce to numerator +/- rhs.numerator directly on already-canonical (so already in-range) int64_t values whose sum/difference is provably 0 -- never an overflow (and canonicalise() already forbids either component from being INT64_MIN, so negating one to check the other is always representable). So whenever the two operands could produce a tie, the overflow predicate that gates the call to compareForSaturation is always false, and the saturating path -- hence compareForSaturation's tie return -- is never reached with a tied pair. Shares this root cause with saturateToward's sign == 0 arm at line 776 below." + "reason": "The false arm (a tie, returning 0) is unreachable. compareForSaturation has exactly two call sites, both from the saturating branch of operator+=/operator-=, and both are guarded by addWouldOverflow/subWouldOverflow having just returned true. A tie in compareForSaturation (*this == -rhs for +=, or *this == rhs for -=) requires -- since operator== compares canonical numerator/denominator pairs directly -- identical denominators. With equal denominators, addWouldOverflow/subWouldOverflow's own scaling factors collapse to rightScaled == leftScaled == 1 (gcd(D, D) == D), so the cross-multiplication checks reduce to numerator +/- rhs.numerator directly on already-canonical (so already in-range) int64_t values whose sum/difference is provably 0 -- never an overflow (and canonicalise() already forbids either component from being INT64_MIN, so negating one to check the other is always representable). So whenever the two operands could produce a tie, the overflow predicate that gates the call to compareForSaturation is always false, and the saturating path -- hence compareForSaturation's tie return -- is never reached with a tied pair. Shares this root cause with saturateToward's sign == 0 arm at line 775 below." }, { "file": "include/morph/util/rational.hpp", - "line": 776, + "line": 775, "source": "numerator = sign == 0 ? 0 : (sign < 0 ? -maxValue : maxValue);", - "reason": "The sign == 0 arm is unreachable, for the same root cause as compareForSaturation's tie return at line 758 above: saturateToward is only ever called with the sign compareForSaturation returned, and compareForSaturation can only return 0 (a tie) for a pair whose overflow predicate is provably false -- so the saturating path that calls saturateToward is never entered with a tied pair, and sign is never 0 when this line runs." + "reason": "The sign == 0 arm is unreachable, for the same root cause as compareForSaturation's tie return at line 757 above: saturateToward is only ever called with the sign compareForSaturation returned, and compareForSaturation can only return 0 (a tie) for a pair whose overflow predicate is provably false -- so the saturating path that calls saturateToward is never entered with a tied pair, and sign is never 0 when this line runs." }, { "file": "include/morph/util/rational.hpp", - "line": 901, + "line": 900, "source": "if (crossDivisorOne == 0 || crossDivisorTwo == 0) {", "reason": "Both disjuncts are unreachable, and the body they guard (the zero-numerator short-circuit `return false;` right below) is correspondingly dead. crossDivisorOne = gcd(|numerator|, rhs.denominator), crossDivisorTwo = gcd(|rhs.numerator|, denominator). std::gcd(a, b) == 0 iff both a == 0 and b == 0. denominator/rhs.denominator can never be 0: every constructor path (Rational(Numerator, Denominator, DecimalPlaces)) calls canonicalise(), which clamps a 0 denominator to 1 before returning, and every mutating operation either recomputes the denominator as a product of positive denominators or calls canonicalise() again. So denominator > 0 (and rhs.denominator > 0) is a whole-class invariant, making crossDivisorOne/crossDivisorTwo == 0 impossible regardless of numerator/rhs.numerator." }, { "file": "include/morph/util/rational.hpp", - "line": 1499, + "line": 1498, "source": "if (ctx.begin() == ctx.end() || *ctx.begin() == '}') {", "reason": "The ctx.begin() == ctx.end() true arm is unreachable. This is the textbook cppreference-style custom-formatter parse() idiom. Empirically verified on this toolchain (libc++, via a standalone probe compiled with clang++ -std=c++23 -stdlib=libc++): for both std::format(\"{}\", x) and std::format(\"{:}\", x), ctx.end() always points past the terminating '}', so ctx.begin() == ctx.end() is false and the terminator is always reachable via *ctx.begin() == '}' -- matching the observed 0/14 split exactly. std::format's top-level parser (and std::vformat's, which performs the same replacement-field validation before dispatching to a type's parse()) rejects an unterminated '{...' before ever calling into formatter::parse, so this function is never invoked with an already-exhausted range. The first disjunct is defensive boilerplate that this standard library's implementation (and the standard's own guarantee about validated replacement fields) makes structurally unreachable." }, { "file": "include/morph/util/rational.hpp", - "line": 1408, + "line": 1407, "source": "if (!detail::addOverflows(whole, step)) {", "reason": "The false arm (the overflow-would-occur case, declining to step) is unreachable, contrary to Task 2's initial classification of this line as testable -- verified both mathematically and empirically (a standalone probe sweeping denominators/precisions found 476 cases where the scale-up saturated to whole == INT64_MAX, and every one had a fractional remainder of exactly 0, never >= 0.5) before writing this entry. A Rational's magnitude can never exceed INT64_MAX: its value is numerator/denominator with denominator >= 1 and numerator in [-INT64_MAX, INT64_MAX] (canonicalise() clamps INT64_MIN away), so |value| <= |numerator| <= INT64_MAX always. `whole` is trunc(scaled), so whole == INT64_MAX forces scaled == INT64_MAX/1 exactly -- there is no room for scaled to be in (INT64_MAX, INT64_MAX + 1) the way an unbounded rational could land. With scaled == whole exactly, `fraction` (scaled minus whole) is always 0, so `roundAway` (set from comparing fraction against 1/2) is always false when whole == INT64_MAX, and stepping never happens on that side. On the other side, step == -1 would need whole == INT64_MIN to overflow, but whole's range is [-INT64_MAX, INT64_MAX] (INT64_MIN is never a valid Rational magnitude), so that direction cannot overflow either. The guard is defensive: reachable only if a future change let a Rational's magnitude exceed INT64_MAX, which the type's invariants currently forbid everywhere else in this file." }, @@ -80,7 +80,7 @@ }, { "file": "include/morph/core/backend.hpp", - "line": 777, + "line": 779, "source": "if (iter != _models.end()) {", "reason": "Unreachable by construction given the `_changeAware`/`_models` invariant (core audit finding BK2). Every model id is inserted into `_changeAware` (when change-aware) in the same `_regMtx`-held critical section that inserts it into `_models` (`createAndTrack`, this file: `if (holder->isBackendChangeAware()) { _changeAware.insert(mid); } _models[mid] = std::move(holder);`), and both are erased together at the single erasure site (`deregisterModel`: `_models.erase(mid); _changeAware.erase(mid);`, also under `_regMtx`). `notifyBackendChanged()` (this function) holds the same `_regMtx` while iterating `_changeAware` and looking each id up in `_models` at this line, so every id it walks is guaranteed still present in `_models` -- the \"not found\" arm cannot occur without a code change that breaks this lockstep bookkeeping." }, @@ -140,19 +140,19 @@ }, { "file": "include/morph/net/socket_backend.hpp", - "line": 109, + "line": 119, "source": "if (_ioThread.joinable()) {", "reason": "Unreachable by construction (net audit, `socket_backend.hpp` extra finding #5). `_ioThread` is started unconditionally in the constructor and has exactly one join site: this line, in the destructor. Nothing else in the file resets, joins, or detaches it, so at destructor time it is always joinable." }, { "file": "include/morph/net/socket_backend.hpp", - "line": 121, + "line": 131, "source": "if (_handlerThread.joinable()) {", - "reason": "Unreachable by construction, same shape as `_ioThread`'s line 109 above (net audit, `socket_backend.hpp` extra finding #6). `_handlerThread` is started unconditionally in the constructor and has exactly one join site: this line, in the destructor. Nothing else resets, joins, or detaches it, so at destructor time it is always joinable." + "reason": "Unreachable by construction, same shape as `_ioThread`'s line 119 above (net audit, `socket_backend.hpp` extra finding #6). `_handlerThread` is started unconditionally in the constructor and has exactly one join site: this line, in the destructor. Nothing else resets, joins, or detaches it, so at destructor time it is always joinable." }, { "file": "include/morph/net/socket_backend.hpp", - "line": 560, + "line": 570, "source": "default:", "reason": "Unreachable except via the adjacent `Error` case it deliberately shares a body with (net audit, `socket_backend.hpp` extra finding #7). `detail::ExecuteReplyKind` is a closed 3-value enum (`Value`/`Timeout`/`Error`), all three handled explicitly above this label; `default:` exists only to satisfy this project's `-Wswitch-default`, per the source's own inline comment directly below this line. Reaching it via any value other than through the `Error` case falling through would require an out-of-range `static_cast` producing a value outside the enum's domain -- undefined behavior, not a legitimate test target." } diff --git a/scripts/mutation_survivors.json b/scripts/mutation_survivors.json index a5bebdcf..cb414756 100644 --- a/scripts/mutation_survivors.json +++ b/scripts/mutation_survivors.json @@ -429,9 +429,12 @@ }, "classification_2026_09_09": { "_comment": [ - "Partial classification of run[2]'s 199 survivors (morph#453 item A).", - "Recorded so the next pass starts from evidence rather than from zero.", - "UNCLASSIFIED IS THE MAJORITY: 183 of 199 have not been assessed." + "Classification of run[2]'s 199 survivors (morph#453 item A).", + "Updated 2026-09-10. The headline finding is NOT a classification: sampling", + "showed the survivor list itself is unreliable -- 5 of 6 sampled survivors", + "are killed by the existing suite when the same mutation is applied by hand.", + "See survivor_list_unreliable below and morph#510. Classifying the remaining", + "entries against this list would be measuring the tool, not the test suite." ], "method": "Clustered by (mutator, source shape), then assessed cluster by cluster against what the code actually promises -- not by writing an assertion per mutant, which the issue's own constraint rules out.", "equivalent": { @@ -460,13 +463,68 @@ }, "unclassified": { "count": 183, - "largest_clusters": [ - "cxx_replace_scalar_call x92 -- replaces a scalar-returning call with 42; concentrated on .empty()/.size() guards", - "cxx_init_const x27 and cxx_assign_const x25", - "cxx_add_to_sub x13", - "cxx_gt_to_ge x8 -- the boundary family morph#434 confirmed mutates and kills correctly, so these should be read as real" + "note": "16 classified (13 equivalent, 3 real and closed). The rest are NOT classified, and per survivor_list_unreliable above, classifying them against this run is the wrong next step.", + "next_step": "Fix or characterise the tooling first (morph#510). A survivor list where 5 of 6 samples are false is not a worklist." + }, + "survivor_list_unreliable": { + "sampled": 6, + "detected_by_suite_despite_being_reported_survived": 5, + "issue": "morph#510", + "status": "Reproduced behaviourally, one of them on a worktree at the campaign's own revision (adfe8e5f) with its own build. Mechanism NOT established -- Mull is not installed here, so no disassembly, unlike morph#434.", + "samples": [ + { + "site": "remote.hpp:1031", + "mutator": "cxx_replace_scalar_call", + "mutation": "if (env.kind == \"register\") forced true", + "result": "140 assertion failures (baseline 21985 passing) -- at adfe8e5f", + "verdict": "detected" + }, + { + "site": "remote.hpp handleInline", + "mutator": "cxx_replace_scalar_call", + "mutation": "if (env.kind == \"execute\") forced true", + "result": "84 assertion failures", + "verdict": "detected" + }, + { + "site": "bridge.hpp whenBound", + "mutator": "cxx_replace_scalar_call", + "mutation": "if (isBound(binding)) forced true", + "result": "6 assertion failures", + "verdict": "detected" + }, + { + "site": "bridge.hpp:1683", + "mutator": "cxx_assign_const", + "mutation": "_lifetime->alive = false -> true in ~Bridge", + "result": "suite hangs; mull-runner's --timeout 60000 counts a timeout as detected", + "verdict": "detected" + }, + { + "site": "forms.hpp:1794", + "mutator": "cxx_init_const", + "mutation": "bool found = false -> true", + "result": "2 assertion failures", + "verdict": "detected" + }, + { + "site": "bridge.hpp:584", + "mutator": "cxx_init_const", + "mutation": "bool started = false -> true", + "result": "suite unchanged (21985 passing)", + "verdict": "genuinely equivalent -- the initialiser is dead, overwritten by `started = backend->attachModelAsync(...)` on the next statement" + } + ], + "important": [ + "The defect is per-mutant, not per-family: cxx_init_const produced one genuine equivalent AND one mis-report. An earlier draft of this entry blamed whole families and was wrong.", + "So the 199 cannot be bulk-reclassified by mutator. What the sampling establishes is that the list is not trustworthy enough to classify against by hand." + ], + "ruled_out": [ + "Test-scope mismatch: scripts/mutation.sh's core-forms scope runs tests/morph_tests, the same binary and full suite, with no filter passed to mull-runner.", + "Revision mismatch: the 140-failure measurement is on adfe8e5f itself and the quoted source line matches character-for-character.", + "Wholesale mutant-selection failure: 574 of 773 were killed in the same run, so the mechanism works in general." ], - "note": "remote.hpp (82) and bridge.hpp (33) hold 58% of all survivors. Neither has been assessed." + "consequence": "The 199-survivor baseline overstates real gaps by an unknown but large margin. It still works as a relative ratchet for morph#408's gate, since the artifacts are stable run to run." } } } diff --git a/tests/net/test_tcp_socket.cpp b/tests/net/test_tcp_socket.cpp index 3906dbe0..08af5fb4 100644 --- a/tests/net/test_tcp_socket.cpp +++ b/tests/net/test_tcp_socket.cpp @@ -477,3 +477,49 @@ TEST_CASE("TcpSocket::shutdownBoth: a safe no-op on an empty socket", "[net][tcp empty.shutdownBoth(); // must not crash REQUIRE_FALSE(empty.valid()); } + +// ── morph#506: a peer that stops reading must not park the sender forever ── +// +// Once the kernel send buffer fills against a peer that never reads, a blocking +// `::send` never returns -- and `sendAll` loops on it. `SocketBackend::sendFrame` +// holds `_socketMtx` across that call, so `~SocketBackend` parks on the same +// lock with nothing able to release it. (Removing the lock is NOT the fix: it +// races `onDisconnected()` reassigning `_socket`, 25 ThreadSanitizer reports.) +// +// `SO_SNDTIMEO` bounds one no-progress send instead, so `sendAll` throws the way +// it already does for any other send error and the lock is released. +TEST_CASE("TcpSocket: setSendTimeout bounds a send against a peer that never reads", "[net][tcp][morph506]") { + auto listener = TcpSocket::listen(0); + std::uint16_t const port = listener.boundPort(); + + // Accepts and then does nothing at all -- never reads a byte. Held open for + // the duration of the test so the connection stays established. + TcpSocket serverSide; + std::thread acceptThread{[&] { serverSide = listener.accept(); }}; + auto clientSide = TcpSocket::connect("127.0.0.1", port, std::chrono::milliseconds{2000}); + acceptThread.join(); + REQUIRE(serverSide.valid()); + + constexpr auto kTimeout = std::chrono::milliseconds{300}; + REQUIRE(clientSide.setSendTimeout(kTimeout)); + + // Push until the buffers fill. Without the timeout this loop never returns; + // with it, sendAll throws once a single send makes no progress. + std::string const chunk(std::size_t{256} * 1024, 'x'); + auto const start = std::chrono::steady_clock::now(); + bool threw = false; + for (int i = 0; i < 400 && !threw; ++i) { + try { + clientSide.sendAll(chunk.data(), chunk.size()); + } catch (const std::runtime_error&) { + threw = true; + } + } + auto const elapsed = std::chrono::steady_clock::now() - start; + + REQUIRE(threw); + // The bound that matters: it gave up rather than blocking indefinitely. + // Generous multiple of the timeout, since each successful send before the + // buffers filled costs real time too. + CHECK(elapsed < std::chrono::seconds{20}); +}