Lead review of PR #557 (ticket comment 16916) found one path left open: sentHandled
is set to true BEFORE onDelivered() runs (correctly, per the earlier fix), so when
onDelivered() itself throws on the normal path, the finally's 'if (sent != null &&
!sentHandled)' guard skipped the whole recovery -- completion included -- and left
sent.delivered() pending forever for a message that really was delivered.
sentHandled must guard only the onDelivered RE-CALL (the permanent-suppression
hazard), never the future completion, since CompletableFuture.complete/
completeExceptionally are idempotent and a no-op on the already-handled path.
Split the one flag's two jobs: the outer 'if (sent != null)' now always runs the
recovery block, and '!sentHandled' moved onto just the onDelivered call inside it.
Added anOnDeliveredThrowOnTheNormalPathStillCompletesTheDeliveryFuture, proven with
the lead's own mutation (reverting !sentHandled onto the outer if): the new test
goes red while anOnDeliveredThrowAfterItsOwnRegistrationDoesNotRunASecondTime stays
green, showing the two concerns are genuinely separate.
The previous try/finally around onStatus's post-monitor region completed
sent.delivered() but never registered sent.token().waiter() when an earlier
listener threw. That waiter is registered only by turnListener.onDelivered(),
inside the very if (sent != null) block the finally backstops, so a caller
was told its send landed and then waited out its full timeout for an answer
that could never resolve (worse than a plain hang).
The finally now does that block's whole job on the unhandled path: it calls
onDelivered() (when sendError == null) before completing the future, guarded
by its own try/catch(Throwable) so a failure there cannot mask the original
throwable. A sentHandled flag, set true at the START of the normal block
(before any side effect), tells the finally whether that already ran, so a
throw partway through onDelivered cannot trigger a second, late captureBaseline
that would permanently suppress the turn's completion.
Also removed a leftover duplicated forget.accept(target) call (with a stray
'MUTATION-TEST-3' comment) in the notReady block — residue from the previous
worker's own mutation testing that was not fully reverted.
Dai Ha