#324: read task.turnId once in finishAsyncTask; analysis of the wider unlocked-ask asymmetry #327
Closed
agent
wants to merge 0 commits from
worker/fix-324-3e9bbf-14 into main
pull from: worker/fix-324-3e9bbf-14
merge into: fleet:main
fleet:main
fleet:worker/fleetd-612-unita-87807e-1
fleet:worker/612-b3-mcpwirings-da2b58-3
fleet:worker/612-b2-cb185-176d3a-2
fleet:worker/612-b1-completion-457459-1
fleet:worker/612-agaps-73a926-2
fleet:worker/608-sleeps-3a64ff-3
fleet:worker/621-b4520b-1
fleet:worker/618-b83894-2
fleet:worker/fleetd-615-e05481-5
fleet:worker/lead-autocompact-5f1ab2-3
fleet:worker/fleetd-613-f85deb-3
fleet:worker/fleetd-608-flaky-nudge-test-d0c2d1-3
fleet:worker/lead-context-gauge-ad404f-1
fleet:worker/gauge-wiring-9158c1-4
fleet:worker/redeploy-slowstart-ead0e5-5
fleet:worker/charter-bytes-13668c-6
fleet:worker/rollover-outcome-291483-2
fleet:worker/589-f64303-2
fleet:worker/593-1a8025-5
fleet:worker/589-fcd2aa-1
fleet:worker/568-9fdaa2-3
fleet:worker/571-attempted-outcome-5739f7-2
fleet:worker/581-completionresolver-cas-sites-0542b7-6
fleet:worker/562-loop-health-wiring-test-99611c-5
fleet:worker/562-surface-loop-health-7df5cc-4
fleet:worker/575-waiter-cleanup-sites-62ad80-1
fleet:worker/572-answer-lock-release-46a9ae-5
fleet:worker/567-probe-channel-leak-a38fc5-6
fleet:worker/551-record-before-send-7cbf56-1
fleet:worker/561-listener-fanout-survives-a-throw-61d538-2
fleet:worker/555-redeploy-main-flow-seam-65c2f5-2
fleet:worker/556-injector-owns-registration-e027a5-1
fleet:worker/552-post-restart-mktemp-abort-bc2672-4
fleet:worker/553-onstatus-completion-leak-0da881-2
fleet:worker/550-shasum-linux-196132-1
fleet:worker/538-loop-dies-on-error-4a5eeb-6
fleet:worker/426-health-coverage-ef1fd4-4
fleet:worker/504-failed-reported-clean-3cfd66-3
fleet:worker/537-capturedlog-close-e4c437-2
fleet:worker/459-broken-link-targets-cadc17-5
fleet:worker/535-appender-leak-fe74c1-1
fleet:worker/512-part2-shutdown-detection-434701-9
fleet:worker/529-logger-level-sweep-2a5533-8
fleet:worker/528-drain-gate-call-site-5de83d-7
fleet:charter/forge-mcp-vs-token
fleet:worker/521-swap-guard-unpinned-28e931-5
fleet:worker/519-probe-test-harness-d25ab8-4
fleet:worker/525-logger-level-leak-1b4eb0-6
fleet:worker/518-fleetmcp-resolver-wiring-8ef96c-1
fleet:worker/512-drain-complete-line-7edd71-3
fleet:worker/517-abort-branch-and-jar-id-41b641-2
fleet:worker/500-9e52c9-3
fleet:worker/509-4912f4-2
fleet:worker/511-9a4b23-1
fleet:worker/493-479f45-2
fleet:worker/505-03f8b2-1
fleet:worker/492-followup-detect-unclear
fleet:worker/501-a31fa0-7
fleet:worker/498-451d1c-5
fleet:worker/494-1015ce-2
fleet:worker/492-209647-1
fleet:worker/489-001902-2
fleet:worker/480-relative-handover-path-906323-1
fleet:worker/480-b-handover-skill-45bf1f-5
fleet:worker/474-followup-source-pin-f54a55-17
fleet:worker/474-charter-check-on-reload-f54a55-17
fleet:worker/466-quarantine-repeatcount-report
fleet:worker/393-opencode-skill-seeding-71854b-13
fleet:worker/469-canonical-tool-names-2a472a-16
fleet:worker/466-quarantine-escalation-5ae9c1-15
fleet:worker/446-hot-exhausted-pattern-0af580-6
fleet:worker/464-charter-tool-name-guard-a85635-12
fleet:worker/463-listfleet-default-fails-open-f1c76c-11
fleet:worker/458-invariant-5-by-purpose-862f9a-10
fleet:worker/439-coordinator-row-gate-bc032a-8
fleet:worker/449-herdr-protocol-576015-4
fleet:worker/450-abstract-spawn-599e1c-5
fleet:worker/437-ack-refuses-177d91-1
fleet:worker/444-placement-window-feb56a-2
fleet:worker/440-helddurable-derived-d462d7-13
fleet:worker/425-rework-placement-resolve-c58ba1-9
fleet:worker/421-lead-peek-held-msgs-cdbad2-10
fleet:worker/435-fixed-policy-cap-fe11de-12
fleet:worker/422-gate-state-observability-9e79d6-11
fleet:worker/431-memberregistry-live-readers-cdbad2-10
fleet:worker/424-architect-slot-hot-038b41-7
fleet:worker/422-model-gate-spawn-c29f48-6
fleet:worker/425-default-profile-live-f55534-8
fleet:worker/415-coverage-wording-2cbf9c-5
fleet:worker/416-3ad1da-1
fleet:worker/418-588283-3
fleet:worker/deterministic-stamp-race-409-3cb7b6-10
fleet:worker/armed-reads-live-config-404-ed931f-9
fleet:worker/reply-peer-refusal-391-5a34bd-7
fleet:worker/models-allowlist-aa9e9b-3
fleet:worker/ttl-stamp-race-399-f1122f-8
fleet:worker/scrub-receipt-400-316b3e-5
fleet:worker/exhaustion-detection-395-105105-6
fleet:worker/scrub-abort-394-316b3e-5
fleet:fix/scrub-uid-abort
fleet:worker/task-scrub-517574-2
fleet:worker/t386-clock-bd5b78-4
fleet:worker/t384-scrub-813790-5
fleet:worker/t381-cc-748314-2
fleet:worker/t373-336973-2
fleet:worker/t365-3920c5-3
fleet:worker/t358-6e989b-1
fleet:worker/t355-8b321c-1
fleet:worker/fleetd-369-hermetic-git-tests-e8b19a-3
fleet:worker/fleetd-368-stale-lead-binding-f5682e-2
fleet:worker/fleetd-360-deploy-units-0d3793-1
fleet:worker/359-dead-lead-tabs-f1253b-4
fleet:worker/362-worktree-skills-c03e51-3
fleet:worker/361-coord-visibility-655144-1
fleet:362-plugin-visibility-and-drift
fleet:worker/errscan-bed2ca-2
fleet:worker/amqp-log-identity-bed2ca-2
fleet:worker/withdefaults-guard-561704
fleet:worker/sleepguard-82076d-1
fleet:worker/fd334-9ee1b6-5
fleet:worker/fd348-f1ab27-4
fleet:worker/fd335-a71c35-1
fleet:worker/fd342-174a17-2
fleet:worker/fd345-490d0f-3
fleet:worker/fleetd-337-5ec7d4-21
fleet:worker/fleetd-341-af5a6b-24
fleet:worker/fleetd-339-5ca0a2-23
fleet:worker/fleetd-338-83a4a1-22
fleet:worker/fleetd-333-281f46-18
fleet:worker/fleetd-329-11bdbb-16
fleet:worker/fleetd-330-2770fb-17
fleet:worker/fix-326-50506e-15
fleet:worker/fix-323-b8287d-13
fleet:worker/fix-316b-bd0860-11
fleet:worker/fix-318-76ca36-9
fleet:worker/fix-317-486aec-8
fleet:worker/fix-315-ce47c5-6
fleet:worker/fix-307-275890-6
fleet:worker/fix-308-b4f664-7
fleet:worker/fix-309-ec3939-8
fleet:worker/fix-310-7a3974-9
fleet:worker/fix-302-52ad0e-9
fleet:worker/fix-298-ce1acb-8
fleet:worker/fix-297-66bd11-7
fleet:worker/fix-296-104622-6
fleet:worker/fix-293-bare-closetab-eb22b5-3
fleet:worker/fix-280-gone-ask-lapse-bca98e-2
fleet:worker/fix-290-reapidle-guard-coverage-9b0dd1-1
fleet:worker/fix-285-trust-seed-8f3565-10
fleet:worker/fix-284-backend-error-seat-85912c-11
fleet:worker/fix-282-chained-ask-e6d0bb-8
fleet:worker/fix-283-teardown-leaks-f40dfa-9
fleet:worker/fix-281-pin-handler-actions-4921ac-7
fleet:worker/audit-rendezvous-lifecycle-d072ae-2
fleet:worker/audit-health-placement-1a2476-6
fleet:worker/audit-teardown-exits-e207a5-3
fleet:worker/audit-launcher-asymmetry-27e370-4
fleet:worker/audit-rest-authz-6ca53c-5
fleet:worker/investigate-275-abandon-asking-fdef52-8
fleet:worker/fix-274-worktree-leak-b0095d-7
fleet:worker/fix-273-exhausted-pattern-9665b5-6
fleet:worker/fleetd-267-model-check-bd8068-1
fleet:worker/fleetd-131-archunit-18b834-7
fleet:worker/fleetd-266-sshagent-rename-a014ff-6
fleet:worker/fleetd-184-uid-claim-8e1f31-4
fleet:worker/fleetd-184-warn-b381ee-10
fleet:worker/fleetd-184-docs-be1d12-9
fleet:worker/fleetd-257-9bf010-7
fleet:worker/fleetd-103-23a113-6
fleet:worker/fleetd-247-342356-5
fleet:worker/fleetd-116-04dea8-4
fleet:worker/fleetd-252-a830e0-3
fleet:worker/fleetd-111-7e8673-9
fleet:worker/fleetd-155c-f8ef4b-8
fleet:worker/fleetd-176-b928ca-3
fleet:worker/fleetd-249-7a7878-2
fleet:worker/cb248-composition-root-b-9acdf7-15
fleet:worker/cb148-envrc-default-fa6c82-12
fleet:worker/cb201-unit5-wiring-6c12e6-8
fleet:worker/cb241-fallback-echo-1175e9-11
fleet:worker/cb149-trust-dialog-2392a5-9
fleet:worker/cb134-148-overlay-visible-c9b986-10
fleet:worker/cb234-session-id-keyed-04e1fc-1
fleet:worker/cb201-unit3-nudge-abdf5c-6
fleet:worker/cb201-unit2-policy-c1102c-5
fleet:worker/cb201-unit4-outcome-a13bfa-7
fleet:worker/cb201-unit1-classifier-91b9b1-4
fleet:worker/cb201-227-refine-831980-3
fleet:worker/cb175-model-readback-0f085f-1
fleet:worker/cb222-charter-tmpdir-17f013-1
fleet:worker/cb226-architect-slot-race-cd3aa8-3
fleet:worker/cb224-worktree-root-group-024523-2
fleet:worker/cb-123-role-demotion-c600f7-2
fleet:worker/cb-219-opencode-roots-1f677e-1
fleet:worker/cb214-claude-session-id-b9eab4-4
fleet:worker/cb213-zdotdir-wrong-process-dd6de4-3
fleet:worker/cb211-exhaustion-classification-9546e0-2
fleet:worker/cb137-ambiguous-task-4df3d8-4
fleet:worker/cb209-agentsessionid-4dfdb6-2
fleet:worker/cb185-hostenvnames-2692b5-3
fleet:worker/cb206-opencode-sqlite-128718-2
fleet:worker/cb185-worktree-group-fc0c99-1
fleet:worker/cb-137-ask-ticket-e7760c-2
fleet:worker/cb-172-broker-uri-d36ae4-4
fleet:worker/cb-175-model-readback-76ead6-3
fleet:worker/cb-161-pane-ancestry-293510-1
fleet:worker/cb-164-rebase-885863-8
fleet:worker/cb-164-empty-scrape-false-success-1a80af-3
fleet:fix/cb-197-ticket-ttl-from-completion
fleet:worker/cb-189-remote-url-coverage-4692f3-1
fleet:worker/cb-185-blockers-027756-4
fleet:worker/cb-192-gap-log-11b631-2
fleet:worker/cb-633-fix-5f4396-3
fleet:worker/cb185-router-d6436d-3
fleet:worker/cb185-router-routing-gaps-9e9d33-3
fleet:worker/cb185-paneids-992586-2
fleet:worker/cb-633-allow-list-union-ed374b-1
fleet:worker/cb-157-credential-in-remote-url-496e44-2
fleet:worker/cb-641-health-herdr-evidence-8f1f54-6
fleet:worker/cb-640-health-msg-evidence-99c9cd-1
fleet:worker/cb-642-fleets-status-skill-bbbc40-5
fleet:cb-634-ide-mcp
fleet:worker/lead-comms-wiring-c014b9-7
fleet:worker/lead-mailbox-c19577-6
fleet:worker/autocompact-window-82bc2f-5
fleet:worker/cb-634-probe-18056f-4
fleet:worker/cb635-broker-urienv
fleet:worker/cb-632-config-retry-8e0efa-7
fleet:lead/cb-622e-claude-md
fleet:lead/cb-622-followup
fleet:worker/cb-622a-165dff-1
fleet:lead/cb-622d-opencode-mount
fleet:worker/cb-622b-717c67-2
fleet:worker/cb-622c-ab7759-3
fleet:worker/cb-617b2-20ca4b-3
fleet:worker/cb-617a-5c2f4a-1
fleet:worker/cb596-4e49ef-3
fleet:worker/cb586-10500c-1
fleet:worker/cb-606-b9343a-25
fleet:worker/cb604-1445f8-24
fleet:worker/cb582-477374-21
fleet:worker/cb584-8c2281-22
fleet:worker/cb600-e6b9a9-20
fleet:worker/cb602-ce257f-19
fleet:worker/cb601-b42837-18
fleet:worker/cb598-6c7ba7-17
fleet:worker/cb599-740fe4-16
fleet:worker/cb597-282224-15
fleet:worker/cb590fix-185e9a-10
fleet:worker/cb528-recovery-race
fleet:worker/cb594-96bead-8
fleet:worker/cb590-916766-2
fleet:worker/cb527-997d99-3
fleet:worker/cb592-env-leak-3cbf9c-1
fleet:worker/cb588-async-ticket-nudge-3218f7-5
fleet:worker/cb578b-9dcb13-6
fleet:worker/cb581-d24826-5
fleet:worker/m2-u5-ef8c42-15
fleet:worker/cb578a-516499-2
fleet:worker/cb576-01a04b-17
fleet:worker/cb579-lead-tab-acba06-20
fleet:worker/cb580-terminal-health-ed6058-21
fleet:worker/cb577-f36fdc-18
fleet:worker/cb573b-3db06f-16
fleet:worker/cb568c-f36fdc-18
fleet:worker/cb568-drop-cause-c3ac1c
fleet:worker/cb575-cancelled-notification-c3ac1c
fleet:worker/m4-sol-a2cbec-3
fleet:worker/cb574-async-ask-c3ac1c
fleet:worker/cb573-health-model-8ca857-14
fleet:worker/cb572-unknown-target-7f2e35-13
fleet:worker/u4-700706-9
fleet:worker/u3-b9fcb6-6
fleet:worker/u2-ef5b68-4
fleet:worker/u1-469dce-1-clean
fleet:worker/u1-469dce-1
fleet:worker/cb-564-health-events-70cf7e-2
fleet:worker/cb-565-recycle-drops-role-98e58f-3
fleet:worker/cb-563-missing-reply-df2866-1
fleet:worker/cb-562-readiness-gate-silent-6c23c9-3
fleet:worker/cb-560-architect-presence-da8155-1
fleet:worker/cb-561-architect-silent-off-a71cab-2
fleet:worker/cb-548-bind-architect-slot-fe1b8c-1
fleet:worker/parity-overlay-settings-5fb711-1
fleet:secrets-central-store
fleet:cb-559-hot-key-correction
fleet:cb-557-fleet-role-pools
fleet:worker/cb-553-maxload-explicit-spawn-305ee3-6
fleet:worker/cb-551-idle-lead-heartbeat-f1633c-1
fleet:worker/cb-544-drain-preserves-worktree-925fad-3
fleet:worker/cb-552-docs-sync-1cb9cf-4
fleet:worker/cb-548-rendezvous-guard-rebased
fleet:worker/cb-548-rendezvous-guard-116b53-10
fleet:worker/cb-548-authz-v2-586df6-8
fleet:worker/cb-548-authz-264363-5
fleet:salvage/cb-528b-codex-home
fleet:salvage/cb-528a-codex-launcher
fleet:CB-518-primary-flow
fleet:feature/peer-launcher-spi
fleet:cb-103-injector
No Reviewers
Labels
Clear labels
blocked
needs-live-proof
ready-to-delegate
silent-default
Cannot start until something else lands. The body says what.
Merged and green, but never shown working on the running daemon. Not the same as done.
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: fleet/fleetd#327
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "worker/fix-324-3e9bbf-14"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fix (#324)
finishAsyncTask(Task, Reply)read the volatileTask.turnIdfield twice — once for thenull check, once as the
ConcurrentHashMap.removekey.answer()calls this while holdingsessionLocksfor the target;ask()'s own timeout path mutates the same field with nolock at all, via
clearAsyncQuestion(turnId, true).volatilemakes each read individuallyfresh, but not the pair atomic — so the field can go
nullbetween the two reads andasyncTasksByTurn.remove(null, task)throwsNullPointerExceptionon the lead's ownanswer()call, even thoughtask.future.complete(result)on the line above already ran (theanswer was in fact delivered).
Fix: capture
task.turnIdinto a local once, use that local for both the check and theremoval.
Test — mutation-proof, interleaving forced deterministically
A real race between two independent threads (
answer()'s lock-holding thread andask()'sunlocked timeout thread) can't be relied on to land in a specific few-instruction window on
every run. Calling the two methods in sequence proves nothing, since the bug is specifically
about what happens between two reads inside one method body.
So the test drives the real sequence (worker asks → primary answers → worker's real reply
arrives) and installs a package-private test hook (
finishAsyncTaskRaceHook) that fires atexactly the point between
finishAsyncTask's former two reads. The hook runsforgetTurnForTest(turnId), which is a thin wrapper that calls the identical productioncleanup
ask()'s own timeout catch block runs —clearAsyncQuestion(turnId, true)— so thetest doesn't hand-roll an approximation of the race; it forces the real mutation at the real
moment.
Both the hook field and its setter/forwarder are
null/no-ops in production and onlyreachable package-privately from the test.
What the test proves: given that exact interleaving,
answer()must not throw, and theasync ticket must still resolve to the worker's real reply.
What it does not prove: that the interleaving is reachable in production on its own
schedule — that is established by reading the code (see the analysis below), not by this
test, since forcing an interleaving via a hook is not the same as two independent threads
racing without help.
Mutation proof
Reverted the fix (restored the double read, keeping the hook call in the same relative
position so it still lands between the check and the use), ran only the new test:
Then restored the fix and re-ran the full build — green (see below).
Analysis — the four named compound actions, plus one I found
The candidate fix (local read) stops the crash but leaves the asymmetry in place. Here is the
per-instance analysis the ticket asked for, with a named path in for every one I call a
defect, and a reason for every one I call safe.
1.
finishAsyncTask(Task, Reply)(:1234 pre-fix) — DEFECT, confirmed. Path in: exactly asin the issue. Fixed above.
2.
markAskTimedOut(:1208-1213) — SAFE. It is a get-then-check-then-set(
task.askTimedOut = true), guarded byturnId.equals(task.turnId). Its only writer-sidecaller is
ask()'s own thread (including a coalesced duplicate caller on the same ticket,per the method's own doc). Two concurrent callers racing on it produce, at worst, a
redundant identical write — the guard is symmetric and the write is idempotent, so no
interleaving of concurrent calls to this method produces wrong state. Nothing under
sessionLocks(send/answer) ever reads or writesaskTimedOut, so there is nolocked-vs-unlocked asymmetry to exploit here the way there is for
turnId. No reachable pathin.
3.
clearAsyncQuestion(:1216-1230) — SAFE IN ITSELF, but it is the root cause of risk inits readers. Its own two writes (
task.question = null, and — ifforgetTurn—asyncTasksByTurn.remove(turnId, task)+task.turnId = null) all use the parameterturnId, never a fresh read oftask.turnId, so its own critical section is internallytorn-read-free regardless of interleaving with concurrent callers (the guard again makes
concurrent duplicate calls harmless). But because it runs unlocked (called only from
ask()) and nullstask.turnId/ removes theasyncTasksByTurnentry, it is exactly whatmakes items 1 and 4 unsafe.
4.
answer()'s get-then-clear-then-finish across three calls (:985, :994, :1011) — ATRISK, and only partly addressed by the local-read fix. Beyond the confirmed NPE (which
needs the removal to land inside
finishAsyncTask's own two-read window), the same raceagainst
ask()'s unlockedclearAsyncQuestion(turnId, true)has a second, quieter outcomethat the local-read fix does not touch:
finishAsyncTask(String turnId, Reply)(:1242, the overloadanswer()calls at :1011) doesits own
asyncTasksByTurn.get(turnId)lookup, independent of the earlier lookup atanswer():985. Ifask()'s unlocked cleanup removes theasyncTasksByTurnentrybefore this lookup runs (not during the two reads inside
finishAsyncTask(Task, Reply), but strictly before it is even called), the lookup returnsnull, the two-argoverload silently no-ops, and
task.future(the thingfleet_poll{ticket}watches) isnever completed by this path.
Because
ask()'s cleanup on its timeout path is just two lightweight method calls, itwill typically finish well before the worker does any further work and calls its real
fleet_reply— so this "entry already gone" outcome is, if anything, more likely thanthe narrow crash window, for the same underlying race.
The lead's own
answer()call still returns correctly in this case — the worker's realreply resolves
answer()'s own forward waiter (opened at :980) directly viarendezvous.resolve()'s fast path inreply(), which never reaches theaskAnsweredAsyncTasks()recovery designed for #137/#307 (that recovery only fires whenreply()'s fast path finds no live waiter — here there is one:answer()'s). So theexisting
askTimedOut-based recovery mechanism does not cover this case.The async ticket is left open (
question == null,futurenot done) until whatevereventually calls
abandon()on the session (an explicitfleet_stop, or the idlereaper) — which then resolves it as a misleading
Outcome.WORKER_FAILED("sessionreleased before it replied"), long after the worker actually replied successfully.
Path in: the primary answers a
fleet_askright as its own ~55s window isindependently lapsing on the worker's side (the same race the issue names), and the
worker finishes its resumed turn and calls its real
fleet_replyquickly enough afterbeing unblocked that
answer()'sfinishAsyncTask(turnId, result)call still happensbefore session teardown — which, given how little work
ask()'s cleanup does, is theordinary case for this race, not a corner of it.
5. Bonus finding (not one of the four named, same shape, found while reading the same
lines) —
reply()'s consumption ofaskAnsweredAsyncTasks(), :465-471:reads the same volatile
orphan.turnIdtwice — once for the null check, once as the removalkey — with no lock, structurally identical to the original bug. Path in: a worker whose
fleet_askis still open (turnId stamped, not yet answered) sends a stray/duplicatefleet_replythat reachesreply()'s slow path (reachable right afterresolveQuestioncloses the forward waiter and before
answer()opens a fresh one, so no live waiter existsfor the fast path to catch it) at the same moment that same
fleet_ask's own timeoutindependently fires and
ask()'s catch block callsclearAsyncQuestion(turnId, true),nulling
orphan.turnIdin betweenreply()'s two reads. I have not fixed this — it'soutside this ticket's stated scope (
finishAsyncTaskonly) and I was told to report,not build. Flagging it here since it's the same pattern with a plausible path in.
Is a local read enough, or is the unlocked
ask()side wrong?The unlocked side is wrong, in a way the local read doesn't fully close. The local read
closes the loud failure (item 1's crash). It does not close the quiet one (item 4's
silently-uncompleted ticket) or the twin instance of the loud one (item 5). Both of those
exist because a second, independent lookup by
turnId(not just a second read of thealready-fetched field) can race
ask()'s unlocked forgetting — and "read once" only helpsa caller that already holds the
Taskreference; it can't help a caller that still has tolook the task up by
turnIdin the first place.If I were building a structural fix (not doing so here, per the ticket): make "find the
task by turnId AND detach it" a single atomic operation against
asyncTasksByTurn, and stoptreating
Task.turnIdas a field a second reader can safely re-read for a decision. Concretely,something like
asyncTasksByTurn.remove(turnId)(orcomputeIfPresent) being the onlyplace a task is ever matched-and-detached by turnId, with
Task.turnIdbecomingwrite-once/informational only. Cost: touches roughly 5-6 call sites in this file
(
markAsyncQuestion,markAskTimedOut,clearAsyncQuestion, bothfinishAsyncTaskoverloads,
reply()'s recovery path,answer()), needs re-verification against everyexisting #282 (chained-ask) and #307 (
askTimedOutforgetting) test, and is exactly the kindof "different structure" the ticket says not to build in this pass. I have not built it — I'm
reporting it and leaving
ask()'s locking question (invariant 1: don't make it takesessionLockswithout proving no deadlock) untouched, since it's the same "do not build,report" instruction.
Build
Run in the worker's own worktree with
cd fleetd && mvn clean install, unpiped, full output read.Files changed
fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java— the single-local-read fix infinishAsyncTask(Task, Reply), plus the package-private test seam(
finishAsyncTaskRaceHook,setFinishAsyncTaskRaceHookForTest,forgetTurnForTest), allinert in production.
fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java— one new test,finishAsyncTaskSurvivesTurnIdGoingNullBetweenItsTwoReads.Caveat for review
Items 4 (the silent-drop variant on
answer()'s own path) and 5 (the twin bug inreply())are not fixed in this PR — they're reported per the ticket's instruction not to build a
structural fix here. The lead should decide whether either warrants its own follow-up ticket.
Pull request closed