fleetd #572: pin answer()'s session-lock release across all four exits #574

Merged
ltms merged 1 commits from worker/572-answer-lock-release-46a9ae-5 into main 2026-09-12 13:30:23 +02:00
Member

Closes fleetd #572.

MessageService.answer() releases its per-session lock in an outer finally (MessageService.java:1218). Mutation testing on main at ba2f4d1 showed this line is covered-but-unasserted: removing it leaves all 1750 existing tests green, because every existing test on this path checks answer()'s return value, never that the lock it took is actually reacquirable afterward. If the lock leaks, the session is wedged forever with no exception and no log line.

Adds four tests (no production change) to MessageServiceTest, one per exit of answer(): normal REPLIED reply, TIMED_OUT_WORKING, ExecutionException rethrow, InterruptedException rethrow. Each proves the lock is reacquirable via a bounded (300ms) follow-up send on the same session, rather than only checking answer()'s own outcome.

Verification performed:

  • mvn -o clean install from fleetd/: BUILD SUCCESS, Tests run: 1754, Failures: 0, Errors: 0 (Maven summary line and an independent sum over target/surefire-reports/*.txt agree).
  • MessageService.java confirmed byte-identical before and after (sha256 b515ed0482fc7f954cd953071b3f40186688dd72fa29d777d8f12ae54b3759dc), anchor count grep -Fxc ' lock.unlock();' = 2 (pristine).
  • Re-applied mutation L exactly (sed -i '' '1218s|.*| /* mutation L: unlock removed */|') -> anchor count drops to 1, sha256 changes. Re-ran MessageServiceTest: all four new tests fail RED naming themselves (answerReleasesTheSessionLockAfterANormalReply, answerReleasesTheSessionLockAfterATimedOutWorkingReturn, answerReleasesTheSessionLockWhenTheReplyFutureFailsExceptionally, answerReleasesTheSessionLockWhenInterrupted), all 83 other MessageServiceTest tests stay green (87 total, 4 failures, 0 errors).
  • Restored file via git checkout: sha256 and anchor count back to pristine (2), git status clean on the production file.

Shape note (not fixed, per ticket instructions): the same shared 2-line cleanup (asyncTasksByWaiter.remove + rendezvous.close) is duplicated at three sites in MessageService.java -- around lines 994-995 (send()'s finally), 1134-1135 (answer()'s inline STALE_TURN early-return cleanup), and 1214-1215 (answer()'s inner finally) -- the same maintained-at-N-sites shape as the lock.unlock() finding. Not investigated further, per scope.

Closes fleetd #572. MessageService.answer() releases its per-session lock in an outer finally (MessageService.java:1218). Mutation testing on main at ba2f4d1 showed this line is covered-but-unasserted: removing it leaves all 1750 existing tests green, because every existing test on this path checks answer()'s return value, never that the lock it took is actually reacquirable afterward. If the lock leaks, the session is wedged forever with no exception and no log line. Adds four tests (no production change) to MessageServiceTest, one per exit of answer(): normal REPLIED reply, TIMED_OUT_WORKING, ExecutionException rethrow, InterruptedException rethrow. Each proves the lock is reacquirable via a bounded (300ms) follow-up send on the same session, rather than only checking answer()'s own outcome. Verification performed: - mvn -o clean install from fleetd/: BUILD SUCCESS, Tests run: 1754, Failures: 0, Errors: 0 (Maven summary line and an independent sum over target/surefire-reports/*.txt agree). - MessageService.java confirmed byte-identical before and after (sha256 b515ed0482fc7f954cd953071b3f40186688dd72fa29d777d8f12ae54b3759dc), anchor count `grep -Fxc ' lock.unlock();'` = 2 (pristine). - Re-applied mutation L exactly (`sed -i '' '1218s|.*| /* mutation L: unlock removed */|'`) -> anchor count drops to 1, sha256 changes. Re-ran MessageServiceTest: all four new tests fail RED naming themselves (answerReleasesTheSessionLockAfterANormalReply, answerReleasesTheSessionLockAfterATimedOutWorkingReturn, answerReleasesTheSessionLockWhenTheReplyFutureFailsExceptionally, answerReleasesTheSessionLockWhenInterrupted), all 83 other MessageServiceTest tests stay green (87 total, 4 failures, 0 errors). - Restored file via `git checkout`: sha256 and anchor count back to pristine (2), git status clean on the production file. Shape note (not fixed, per ticket instructions): the same shared 2-line cleanup (asyncTasksByWaiter.remove + rendezvous.close) is duplicated at three sites in MessageService.java -- around lines 994-995 (send()'s finally), 1134-1135 (answer()'s inline STALE_TURN early-return cleanup), and 1214-1215 (answer()'s inner finally) -- the same maintained-at-N-sites shape as the lock.unlock() finding. Not investigated further, per scope.
agent added 1 commit 2026-09-12 13:24:54 +02:00
fleetd #572: pin answer()'s session-lock release across all four exits
CI / shell-tests (pull_request) Successful in 7s
CI / contract (pull_request) Successful in 47s
CI / build (pull_request) Successful in 2m10s
a4dbc8f8b7
MessageService.answer() releases its per-session lock in an outer finally
(MessageService.java:1218) that mutation testing showed was covered but
unasserted: removing that line left all 1750 existing tests green, because
every existing test on this path checks answer()'s return value, never that
the lock it took is actually reacquirable afterward. If it leaked, a session
would be wedged forever with no exception and no log line.

Adds four tests, one per exit of answer() (normal REPLIED reply,
TIMED_OUT_WORKING, ExecutionException rethrow, InterruptedException
rethrow), each proving the lock is reacquirable via a bounded (300ms)
follow-up send on the same session rather than merely checking answer()'s
own outcome. No production change.
ltms merged commit 84d631b030 into main 2026-09-12 13:30:23 +02:00
Sign in to join this conversation.