UpdateManagerImpl.run() iterates updaters without the read lock

[egg82]

UpdateManagerImpl.run() iterates the updaters list without taking the read lock, while register() and unregister() mutate it under the write lock.

UpdateManagerImpl.java:128-131:

private void run() {
    pool.schedule(this::run, 5, TimeUnit.SECONDS);

    for (Updater u : updaters) {

That's the raw field, unsynchronized. Every other access is guarded — updaters() takes the read lock (:60), register() (:78) and unregister() (:99) take the write lock. updaters is a plain ArrayList, so a concurrent register() can throw ConcurrentModificationException in the scheduler thread or expose a partially-updated list.

The exposure is wider than startup registration, though. run() schedules its own next invocation at :129 before doing any work, and the pool is Executors.newScheduledThreadPool(Math.max(4, cores/2)) (FetcharrAPIImpl.java:22). Since u.run() blocks on HTTP to each *arr, any cycle taking longer than 5 seconds overlaps the next — so concurrent run() bodies are the normal case on a real install, not a rare race.

That matters for the fix: swapping in the existing updaters() snapshot closes the CME but not the overlap. If overlapping cycles aren't intended, :129 is the thing to look at.

Read from source; not run.

---

Found with Claude Code.

[egg82]

Correcting the severity I originally filed this with. I described the race as "small but real... during startup". That understates it: run() schedules its own next invocation at :129 before doing any work, on Executors.newScheduledThreadPool(Math.max(4, cores/2)) (FetcharrAPIImpl.java:22), and u.run() blocks on HTTP — so any cycle over 5s overlaps the next and concurrent run() bodies are normal, not rare. The snapshot fix I suggested closes the CME without touching that. Body updated.

[egg82]

Still nailing down this "bug hunting" skill. In hindsight fetcharr wasn't a good repo to throw claude at to work on this concept but here we are. At least the issue is real.