fleetd #334: close ask()'s turn before forgetting its Task #353

2026-09-04T12:06:52+02:00

agent commented

2026-09-04 12:06:52 +02:00

Fixes fleetd #334.

Bug

`ask()`'s TimeoutException catch used to run `clearAsyncQuestion(turnId, true)` (forgetting the Task's `asyncTasksByTurn` mapping) BEFORE `rendezvous.closeAsk(turnId)` ran, in the shared `finally`. Between those two calls the ask was still "answerable" (`rendezvous.askSession(turnId)` still non-null) but the Task mapping was already gone. A primary's `answer()` call racing that window found `task == null`, its `task != null` guard skipped `finishAsyncTask`, and the async ticket sat at `PENDING` forever even though `answer()` itself reported a result.

`#329` fixed one half of this (removing `answer()`'s second, racy lookup). This fixes the remaining half: the ordering inside `ask()`'s own timeout teardown.

ask()'s exits (5 total, unchanged in number)

Early return before the try block: NO_WAITER, when `resolveQuestion` finds no live delegator. Traced this and confirmed it cannot race a real `answer()` call — the turnId is never disclosed to the primary when this branch is taken, so nothing external can reference it. Left unchanged.
Normal return (ANSWERED) inside the try.
`catch (TimeoutException)` — TIMED_OUT. This is the one fixed.
`catch (ExecutionException)` — rethrows.
`catch (InterruptedException)` — rethrows, restores interrupt flag.

All four try-exits (2-5) still go through the shared `finally`, unchanged.

Mechanism (candidate #1, applied precisely)

Inside the TimeoutException catch, for the fresh ticket owner only:

`rendezvous.closeAsk(turnId)` runs FIRST.
Then `markAskTimedOut` + `clearAsyncQuestion(turnId, true)` forget the Task mapping.

Both happen on the same thread in this fixed order, so a racing `answer()` call now either:

still observes `askSession(turnId) != null` — which, given the ordering, guarantees the Task mapping is still intact for it to find, and it completes the ticket normally via the worker's eventual real reply; or
observes `askSession(turnId) == null` — and bails out `STALE_TURN` from `answer()`'s own top check, before ever reaching `asyncTasksByTurn`.

`rendezvous.closeAsk` is idempotent (checked in `Rendezvous.java`: a no-op once the turn is already removed), so the shared `finally`'s later re-call of `closeAsk` for the same fresh call is harmless.

I also gated the whole close/mark/forget sequence by `ticket.fresh()` (it previously ran unconditionally for BOTH the fresh owner and a coalesced duplicate). This matches the invariant the `finally` block already states in its own comment ("Only the fresh owner tears down the shared turn; a duplicate must leave it open") — before this fix a duplicate's own (possibly shorter) timeout could forget the shared Task mapping while the fresh owner's ask was still legitimately open, reproducing an analogous stranding via a different door. No existing test exercised that edge; this closes it as part of the same reorder.

The lead's markAskTimedOut argument

Checked it and it holds. `askTimedOut` is read only by `askAnsweredAsyncTasks`, and `reply()` never reaches that fallback while a live rendezvous waiter is open for the target. The probe's use of `forgetTurnForTest` (which omits `markAskTimedOut`) does not change the reproduced outcome — confirmed this by tracing every call site of `askTimedOut`.

Mutation proof

Copied `MessageService.java` to `/tmp` first, then reverted the fix in place: put `clearAsyncQuestion`/`markAskTimedOut` back BEFORE `closeAsk` (the pre-fix order), keeping the new test-only hook positioned in the analogous spot (mapping already gone, ask still open). Ran only the new test:

```
mvn -q -Dtest=MessageServiceTest#aLateAnswerDuringAskTimeoutTeardownStillCompletesTheAsyncTicket test
```

Result:
```
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
org.opentest4j.AssertionFailedError: a late answer racing the timeout teardown must see the ask as already lapsed ==> expected: <STALE_TURN> but was: <TIMED_OUT_WORKING>
at dev.ltms.fleet.msg.MessageServiceTest.aLateAnswerDuringAskTimeoutTeardownStillCompletesTheAsyncTicket(MessageServiceTest.java:427)
```

This is exactly the invariant-1 violation the ticket named: the late answer proceeded as if the ask were still live (opened its own waiter, then timed out on its own 500ms window) instead of being told the turn had already lapsed. Restored the file from `/tmp` afterward (`diff` confirmed byte-identical to the fix) and reran the full suite to confirm green again.

Housekeeping

Updated the `fleetd #334` comment in `answer()` (previously ending "Open as fleetd #334 — do not read this guard as complete") to describe the fix and point at the new test, and clarified that `task == null` in `answer()` now only means "this was a blocking-send ask, which never had a Task" (or the already-fixed #329 race elsewhere) — not this window, which is now closed.

Build

`cd fleetd && mvn clean install`, run unpiped, full output read (not `| tail`):

```
[INFO] Tests run: 1368, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```

(main was reported green at 1367 before this change; this PR adds one test, MessageServiceTest now runs 81 instead of 80.)

Out of scope, noted but not touched

The NO_WAITER early-return branch (exit 1) has the same clear-then-close statement order as the pre-fix timeout catch, but I traced it and confirmed it's unreachable by a racing `answer()` (the turnId is never disclosed when that branch fires), so I left it as-is rather than touching code outside the actual race.
Several pre-existing stale line-number references in nearby comments (`:802`, `:860`, `:861`, `:1017`, `:991` in the `#282`/`#329` commentary) were already stale before this change, from earlier PRs landing in this file today. Did not fix these — out of this ticket's scope — only fixed the ones inside the comment paragraph I rewrote.

Fixes fleetd #334. ## Bug \`ask()\`'s TimeoutException catch used to run \`clearAsyncQuestion(turnId, true)\` (forgetting the Task's \`asyncTasksByTurn\` mapping) BEFORE \`rendezvous.closeAsk(turnId)\` ran, in the shared \`finally\`. Between those two calls the ask was still "answerable" (\`rendezvous.askSession(turnId)\` still non-null) but the Task mapping was already gone. A primary's \`answer()\` call racing that window found \`task == null\`, its \`task != null\` guard skipped \`finishAsyncTask\`, and the async ticket sat at \`PENDING\` forever even though \`answer()\` itself reported a result. \`#329\` fixed one half of this (removing \`answer()\`'s second, racy lookup). This fixes the remaining half: the ordering inside \`ask()\`'s own timeout teardown. ## ask()'s exits (5 total, unchanged in number) 1. Early return before the try block: NO_WAITER, when \`resolveQuestion\` finds no live delegator. Traced this and confirmed it cannot race a real \`answer()\` call — the turnId is never disclosed to the primary when this branch is taken, so nothing external can reference it. Left unchanged. 2. Normal return (ANSWERED) inside the try. 3. \`catch (TimeoutException)\` — TIMED_OUT. This is the one fixed. 4. \`catch (ExecutionException)\` — rethrows. 5. \`catch (InterruptedException)\` — rethrows, restores interrupt flag. All four try-exits (2-5) still go through the shared \`finally\`, unchanged. ## Mechanism (candidate #1, applied precisely) Inside the TimeoutException catch, for the fresh ticket owner only: 1. \`rendezvous.closeAsk(turnId)\` runs FIRST. 2. Then \`markAskTimedOut\` + \`clearAsyncQuestion(turnId, true)\` forget the Task mapping. Both happen on the same thread in this fixed order, so a racing \`answer()\` call now either: - still observes \`askSession(turnId) != null\` — which, given the ordering, guarantees the Task mapping is still intact for it to find, and it completes the ticket normally via the worker's eventual real reply; or - observes \`askSession(turnId) == null\` — and bails out \`STALE_TURN\` from \`answer()\`'s own top check, before ever reaching \`asyncTasksByTurn\`. \`rendezvous.closeAsk\` is idempotent (checked in \`Rendezvous.java\`: a no-op once the turn is already removed), so the shared \`finally\`'s later re-call of \`closeAsk\` for the same fresh call is harmless. I also gated the whole close/mark/forget sequence by \`ticket.fresh()\` (it previously ran unconditionally for BOTH the fresh owner and a coalesced duplicate). This matches the invariant the \`finally\` block already states in its own comment ("Only the fresh owner tears down the shared turn; a duplicate must leave it open") — before this fix a duplicate's own (possibly shorter) timeout could forget the shared Task mapping while the fresh owner's ask was still legitimately open, reproducing an analogous stranding via a different door. No existing test exercised that edge; this closes it as part of the same reorder. ## The lead's markAskTimedOut argument Checked it and it holds. \`askTimedOut\` is read only by \`askAnsweredAsyncTasks\`, and \`reply()\` never reaches that fallback while a live rendezvous waiter is open for the target. The probe's use of \`forgetTurnForTest\` (which omits \`markAskTimedOut\`) does not change the reproduced outcome — confirmed this by tracing every call site of \`askTimedOut\`. ## Mutation proof Copied \`MessageService.java\` to \`/tmp\` first, then reverted the fix in place: put \`clearAsyncQuestion\`/\`markAskTimedOut\` back BEFORE \`closeAsk\` (the pre-fix order), keeping the new test-only hook positioned in the analogous spot (mapping already gone, ask still open). Ran only the new test: \`\`\` mvn -q -Dtest=MessageServiceTest#aLateAnswerDuringAskTimeoutTeardownStillCompletesTheAsyncTicket test \`\`\` Result: \`\`\` [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 org.opentest4j.AssertionFailedError: a late answer racing the timeout teardown must see the ask as already lapsed ==> expected: <STALE_TURN> but was: <TIMED_OUT_WORKING> at dev.ltms.fleet.msg.MessageServiceTest.aLateAnswerDuringAskTimeoutTeardownStillCompletesTheAsyncTicket(MessageServiceTest.java:427) \`\`\` This is exactly the invariant-1 violation the ticket named: the late answer proceeded as if the ask were still live (opened its own waiter, then timed out on its own 500ms window) instead of being told the turn had already lapsed. Restored the file from \`/tmp\` afterward (\`diff\` confirmed byte-identical to the fix) and reran the full suite to confirm green again. ## Housekeeping Updated the \`fleetd #334\` comment in \`answer()\` (previously ending "Open as fleetd #334 — do not read this guard as complete") to describe the fix and point at the new test, and clarified that \`task == null\` in \`answer()\` now only means "this was a blocking-send ask, which never had a Task" (or the already-fixed #329 race elsewhere) — not this window, which is now closed. ## Build \`cd fleetd && mvn clean install\`, run unpiped, full output read (not \`| tail\`): \`\`\` [INFO] Tests run: 1368, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS \`\`\` (main was reported green at 1367 before this change; this PR adds one test, MessageServiceTest now runs 81 instead of 80.) ## Out of scope, noted but not touched - The NO_WAITER early-return branch (exit 1) has the same clear-then-close statement order as the pre-fix timeout catch, but I traced it and confirmed it's unreachable by a racing \`answer()\` (the turnId is never disclosed when that branch fires), so I left it as-is rather than touching code outside the actual race. - Several pre-existing stale line-number references in nearby comments (\`:802\`, \`:860\`, \`:861\`, \`:1017\`, \`:991\` in the \`#282\`/\`#329\` commentary) were already stale before this change, from earlier PRs landing in this file today. Did not fix these — out of this ticket's scope — only fixed the ones inside the comment paragraph I rewrote.

agent added 1 commit 2026-09-04 12:06:52 +02:00

fleetd #334 : close ask()'s turn before forgetting its Task, closing the last stranding window

CI / build (pull_request) Successful in 2m14s

Details

CI / contract (pull_request) Successful in 19m14s

Details

0d5944af63

ask()'s TimeoutException catch used to run clearAsyncQuestion(turnId, true) -- forgetting the
Task's asyncTasksByTurn mapping -- before rendezvous.closeAsk(turnId) ran in the shared finally.
Between those two calls the ask was still "answerable" (askSession(turnId) non-null) but the Task
mapping was already gone, so a racing answer() call found task == null, skipped
finishAsyncTask, and stranded the async ticket at PENDING even though answer() itself reported a
result. #329 fixed one step of this same race; this closes the remaining one.

The fix reorders the fresh owner's teardown: closeAsk runs first, then markAskTimedOut and
clearAsyncQuestion. A racing answer() call now either sees the ask still open (and the Task
mapping guaranteed intact) or sees it already closed (STALE_TURN, before it ever reaches
asyncTasksByTurn). It also gates the whole block by ticket.fresh(), matching the invariant the
finally block already states ("only the fresh owner tears down the shared turn") -- a duplicate
coalesced ask() timing out no longer forgets bookkeeping the fresh owner still needs.

Adds aLateAnswerDuringAskTimeoutTeardownStillCompletesTheAsyncTicket, which pins the exact window
with a new test-only hook (askTimeoutRaceHookForTest) and proves both invariants: a late answer()
racing the timeout sees STALE_TURN, and the async ticket still resolves DONE from the worker's
real reply. Reverting the reorder (verified locally, not committed) makes this test fail with
"expected STALE_TURN but was TIMED_OUT_WORKING".

ltms closed this pull request

2026-09-04 12:13:29 +02:00

CI / build (pull_request) Successful in 2m14s

Details

CI / contract (pull_request) Successful in 19m14s

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#353