diff --git a/11-Features.md b/11-Features.md index 260b0e2..61fd03a 100644 --- a/11-Features.md +++ b/11-Features.md @@ -4344,3 +4344,41 @@ after the member's pane goes anyway. The same in-memory `spawnedBy` breaks `clearContext` a different way: it plainly no-ops on a cache miss. Open as fleetd #352, and the consequence is not measured yet. + +--- + +## A failed cleanup no longer strands the tickets behind it + +**What it does.** `MessageService.abandon` walks every task it must fail and completes each one. The +per-task cleanup after that completion is now inside a `try`/`catch`, and each task's own +`future.complete(outcome)` runs **before** the guard. A cleanup that throws costs that one task its +bookkeeping and nothing more; the loop still reaches every task behind it. The second half of the +same fix wraps `pushLoop.onTicketTerminal` inside `sendAsync`'s `whenComplete` action. + +**On.** Always on (fleetd #335). + +**Why it exists.** `abandon` runs on teardown — a release, or the health monitor's GONE sweep — and +nothing comes along later to finish what it misses. One call in that loop reaches a broker: +`inbox.publish` puts a recovered reply back, and `AmqpReplyInbox.publish` throws +`IllegalStateException` on an unroutable publish, on a confirm timeout, and on an interrupt. An +uncaught throw there aborted the loop, so every task after it stayed `PENDING` forever and its lead +waited on a ticket that would never resolve. The `whenComplete` half is the same failure with a +different cause: the daemon's shutdown hook closes `MessageService` before `ReplyPushLoop`, and +`messages.close()` does not cancel a send already in flight, so a ticket completing in that window +made the push loop's scheduler throw `RejectedExecutionException` into a discarded stage — no log, +no metric, and the push loop never learned the ticket was terminal. + +**One thing to know for maintenance.** A publish failure used to propagate out of `abandon` to its +caller. It is now logged at `error` with the ticket, target and turnId, and swallowed. That is +deliberate and matches what #293 already does for teardown in `HerdrPeerLauncher`: past the point +where the real work is done, a cleanup failure must not mask the steps behind it. + +A third site was reported and is **not** a defect: the `finally` blocks in `send()` and `answer()` +call `asyncTasksByWaiter.remove` and `Rendezvous.close`, which is `waiters.remove(session, waiter)`. +Neither can throw, so no guard was added. A guard that can never fire is worse than none — it reads +as evidence that somebody checked. + +Measured on merge: keeping the `catch` but adding a `break` to it leaves the suite failing at +`aPerTaskCleanupFailureDoesNotStrandTheRemainingMatchingTasks` with `expected: but was: +`. So the test pins the property that matters — the tasks behind the throwing one still +finish — and not merely that no exception escapes.