fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

CB-603: FakeHerdr.calls was a plain ArrayList written from scheduler threads — intermittent ConcurrentModificationException #100

New Issue
Closed
opened 2026-08-16 18:14:17 +02:00 by ltms · 0 comments
ltms commented 2026-08-16 18:14:17 +02:00
Owner
Copy Link
Copy Source

Already fixed in 863d477 on main. Filed after the fact so the cause is recorded rather than living only in a commit message.

What happened

While trial-merging CB-598 (#87) onto main, the full build failed:

[ERROR] MessageServiceTest.aPrunedTicketIsReclaimedFromThePushLoopNotLeakedForever -- ERROR!
java.util.ConcurrentModificationException
	at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1695)
	...
	at java.base/java.util.stream.ReferencePipeline.anyMatch(ReferencePipeline.java:668)
	at dev.ltms.bridged.herdr.FakeHerdr.called(FakeHerdr.java:137)
	at dev.ltms.bridged.msg.MessageServiceTest.awaitNudge(MessageServiceTest.java:753)

Cause

FakeHerdr.calls was a plain ArrayList:

public final List<Call> calls = new ArrayList<>();

record(...) appends to it from whichever thread makes the call, and ReplyPushLoop and LeadHeartbeatLoop both call the fake from their own scheduler threads. Meanwhile awaitNudge polls called(...), which streams the same list from the test thread. A nudge landing mid-stream throws.

So the fake is shared across threads by design — the loops under test are asynchronous — but was never made safe for it.

Why it was not seen before

It needs a nudge to land inside the few microseconds a poll spends iterating. On an idle machine the poll almost always wins. It surfaced under a full parallel build, and CB-598 made it likelier by nudging more often. The race is on main independently of CB-598 — that change only widened the window.

Fix

calls is now a CopyOnWriteArrayList, with a comment saying which threads write it and why. Read-heavy with modest writes is exactly this collection's case, and it gives iteration a snapshot so no reader can see a concurrent append.

mvn -f bridged/pom.xml clean install → Tests run: 824, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, unpiped.

The general point

This is the second flaky test in one day (see #95) and both have the same root: a test that observes an asynchronous loop needs to be safe against that loop's thread, and neither the compiler nor a green build will tell you it is not. Worth remembering when reviewing any test that polls a fake while a scheduler runs.

Other collections in FakeHerdr (extraWorkspaces, extraAgents, extraTabs) are written only during test setup, before the loops start, so they were left alone. If a future test mutates them while a loop runs, they need the same treatment.

**Already fixed** in 863d477 on `main`. Filed after the fact so the cause is recorded rather than living only in a commit message. ## What happened While trial-merging CB-598 (#87) onto `main`, the full build failed: ``` [ERROR] MessageServiceTest.aPrunedTicketIsReclaimedFromThePushLoopNotLeakedForever -- ERROR! java.util.ConcurrentModificationException at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1695) ... at java.base/java.util.stream.ReferencePipeline.anyMatch(ReferencePipeline.java:668) at dev.ltms.bridged.herdr.FakeHerdr.called(FakeHerdr.java:137) at dev.ltms.bridged.msg.MessageServiceTest.awaitNudge(MessageServiceTest.java:753) ``` ## Cause `FakeHerdr.calls` was a plain `ArrayList`: ```java public final List<Call> calls = new ArrayList<>(); ``` `record(...)` appends to it from whichever thread makes the call, and `ReplyPushLoop` and `LeadHeartbeatLoop` both call the fake from their own scheduler threads. Meanwhile `awaitNudge` polls `called(...)`, which streams the same list from the test thread. A nudge landing mid-stream throws. So the fake is shared across threads by design — the loops under test are asynchronous — but was never made safe for it. ## Why it was not seen before It needs a nudge to land inside the few microseconds a poll spends iterating. On an idle machine the poll almost always wins. It surfaced under a full parallel build, and CB-598 made it likelier by nudging more often. **The race is on `main` independently of CB-598** — that change only widened the window. ## Fix `calls` is now a `CopyOnWriteArrayList`, with a comment saying which threads write it and why. Read-heavy with modest writes is exactly this collection's case, and it gives iteration a snapshot so no reader can see a concurrent append. `mvn -f bridged/pom.xml clean install` → `Tests run: 824, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, unpiped. ## The general point This is the second flaky test in one day (see #95) and both have the same root: **a test that observes an asynchronous loop needs to be safe against that loop's thread, and neither the compiler nor a green build will tell you it is not.** Worth remembering when reviewing any test that polls a fake while a scheduler runs. Other collections in `FakeHerdr` (`extraWorkspaces`, `extraAgents`, `extraTabs`) are written only during test setup, before the loops start, so they were left alone. If a future test mutates them while a loop runs, they need the same treatment.
ltms added this to the 1.1 — single-host close-out milestone 2026-08-16 18:14:17 +02:00
ltms closed this issue 2026-08-16 18:14:22 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone 1.1 — single-host close-out
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#100
Delete Branch "%!s()"

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?